[webkit-reviews] review denied: [Bug 64283] Web Inspector: Focusing on another node while editing CSS property does not update styles : [Attachment 100814] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jul 15 06:25:26 PDT 2011


Yury Semikhatsky <yurys at chromium.org> has denied Pavel Feldman
<pfeldman at chromium.org>'s request for review:
Bug 64283: Web Inspector: Focusing on another node while editing CSS property
does not update styles
https://bugs.webkit.org/show_bug.cgi?id=64283

Attachment 100814: Patch
https://bugs.webkit.org/attachment.cgi?id=100814&action=review

------- Additional Comments from Yury Semikhatsky <yurys at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=100814&action=review


Is there a test that covers this change?

> LayoutTests/http/tests/inspector/elements-test.js:87
> +	   InspectorTest.addSniffer(WebInspector.StylesSidebarPane.prototype,
"_nodeStylesUpdatedForTest", sniff);

Since the method is for tests only it makes sense to convert it into a property
and don't use addSniffer.

> Source/WebCore/inspector/front-end/CSSStyleModel.js:516
> +		  
WebInspector.cssModel._fireStyleSheetChanged(style.id.styleSheetId,
majorChange, userCallback ? userCallback.bind(this, style) : null);

It is not clear which object is going to be 'this' at the moment of binding.

> Source/WebCore/inspector/front-end/DOMAgent.js:432
> +    _inlineStyleInvalidated: function(nodeIds)

Inline this?

> Source/WebCore/inspector/front-end/StylesSidebarPane.js:2106
> +	       this._parentPane._userOperation = true;

Is intentional that _userOperation is never reset to false?


More information about the webkit-reviews mailing list