Bug 65918 – Web Inspector: [REGRESSION] Editor lost after committing a CSS property value for inline style

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Alexander Pavlov (apavlov)]

1. Bring up Web Inspector for any element
2. Start editing new/existing CSS property value in the Styles pane (Elements panel)
3. Press Enter or Tab to commit the property.

What is the expected output?
A new property editor appears

What do you see instead?
Oftentimes, no property editor appears. This works fine for non-inline styles and seems to be a regression introduced in http://trac.webkit.org/changeset/91070 (essentially, a race condition mostly seen with large stylesheets and/or Debug builds).

Upstreaming http://code.google.com/p/chromium/issues/detail?id=91822

[Alexander Pavlov (apavlov)]

Created attachment 103366 [details]
[PATCH] Suggested fix

[Pavel Feldman]

Comment on attachment 103366 [details]
[PATCH] Suggested fix

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

Looks promising!

> Source/WebCore/inspector/front-end/StylesSidebarPane.js:936
> +    update: function(full, callback)

Is this change needed?

> Source/WebCore/inspector/front-end/StylesSidebarPane.js:1623
> +    _updateAll: function(userCallback)

What does updateAll actually do?

> Source/WebCore/inspector/front-end/StylesSidebarPane.js:2100
> +            --parentPane._userOperation;

You should keep userOperation's logging + invent a new flag for this "transitioning between the values".

[Alexander Pavlov (apavlov)]

(In reply to comment #2)
> (From update of attachment 103366 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=103366&action=review
> 
> Looks promising!
> 
> > Source/WebCore/inspector/front-end/StylesSidebarPane.js:936
> > +    update: function(full, callback)
> 
> Is this change needed?

Good point, rewrote the call site.

> > Source/WebCore/inspector/front-end/StylesSidebarPane.js:1623
> > +    _updateAll: function(userCallback)
> 
> What does updateAll actually do?

It attempts to update the biggest possible scope accessible from this tree element (i.e., the Styles pane (the parent section has the "pane" field set), the parent section (no "pane" field), or the element's title (no parent section)). After some experimenting, I failed to hit the second and the third cases, so I've cleaned them up and renamed the method to _updatePane().

> > Source/WebCore/inspector/front-end/StylesSidebarPane.js:2100
> > +            --parentPane._userOperation;
> 
> You should keep userOperation's logging + invent a new flag for this "transitioning between the values".

logging -> logic. I've managed to get rid of this counting merely by avoiding the flag modification in the !updateInterface case (since in the !updateInterface case the _isEditingStyle flag is true, and we are safe).

Attaching an updated patch shortly.

[Alexander Pavlov (apavlov)]

Created attachment 103466 [details]
[PATCH] Comments addressed

[Alexander Pavlov (apavlov)]

Committed r92764: <http://trac.webkit.org/changeset/92764>