[webkit-reviews] review denied: [Bug 183097] Web Inspector: Styles: Newly added unsupported properties sometimes don't have warnings : [Attachment 335965] Patch

[bugzilla-daemon at webkit.org]

Matt Baker <mattbaker at apple.com> has denied Nikita Vasilyev
<nvasilyev at apple.com>'s request for review:
Bug 183097: Web Inspector: Styles: Newly added unsupported properties sometimes
don't have warnings
https://bugs.webkit.org/show_bug.cgi?id=183097

Attachment 335965: Patch

https://bugs.webkit.org/attachment.cgi?id=335965&action=review




--- Comment #3 from Matt Baker <mattbaker at apple.com> ---
Comment on attachment 335965
  --> https://bugs.webkit.org/attachment.cgi?id=335965
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=335965&action=review

This fixes the issue, but I'm not sure it is the right approach so r- for now.
While debugging I noticed the following:

1. Click after "Style Attribute {" to add new property
2. Type "foo: b", then hit backspace to clear "b" from the value field.
3. Type "b" in value field again
  => Warning appears

So the first time text is entered in the new property's value field, it isn't
validated. But clearing the field and starting to type again triggers an
update. This seems wrong.

> Source/WebInspectorUI/ChangeLog:9
> +	   Update properties warnings every time focus moves.

I'd reword this, since SpreadsheetStyleProperty.updateStatus does more than
update warnings:

"Update status of properties warnings every time focus moves.

>
Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.
js:232
> +	   // Focus moved by clicking outside of the currently focused
property.

This comment is inaccurate. The direction will also be null when this is called
in response to a property text field "blur" event.