[webkit-reviews] review denied: [Bug 40069] Web Inspector: Eliminate direct dependency of StylesSidebarPane on InspectorBackend : [Attachment 57863] [PATCH] Comments addressed

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jun 4 06:12:19 PDT 2010


Pavel Feldman <pfeldman at chromium.org> has denied Alexander Pavlov (apavlov)
<apavlov at chromium.org>'s request for review:
Bug 40069: Web Inspector: Eliminate direct dependency of StylesSidebarPane on
InspectorBackend
https://bugs.webkit.org/show_bug.cgi?id=40069

Attachment 57863: [PATCH] Comments addressed
https://bugs.webkit.org/attachment.cgi?id=57863&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
Overall great first step. Few nits below:

WebCore/inspector/front-end/CSSStyleModel.js:36
 +	styles: function(nodeId, authOnly, userCallback)
getStylesAsync

WebCore/inspector/front-end/CSSStyleModel.js:48
 +	computedStyle: function(nodeId, userCallback)
getComputedStyleAsync

WebCore/inspector/front-end/CSSStyleModel.js:39
 +	    function callback(styles)
why wrapping it?

WebCore/inspector/front-end/CSSStyleModel.js:50
 +	    function callback(style)
ditto

WebCore/inspector/front-end/CSSStyleModel.js:95
 +	toggleStyleEnablement: function(styleId, propertyName, disabled,
userCallback)
toggleStyleEnabled

WebCore/inspector/front-end/CSSStyleModel.js:117
 +	    function callback(success)
no need to wrap

WebCore/inspector/front-end/CSSStyleModel.js:115
 +	setStyleText: function(styleId, cssText, userCallback)
setCSSText

WebCore/inspector/front-end/CSSStyleModel.js:140
 +	   
InspectorBackend.applyStyleText(WebInspector.Callback.wrap(callback), styleId,
styleText, propertyName);
callback.bind(this)

WebCore/inspector/front-end/CSSStyleModel.js:127
 +	    var self = this;
no 'this' here

WebCore/inspector/front-end/StylesSidebarPane.js:1567
 +	    WebInspector.styleModel.applyStyleText(this.style.id, styleText,
this.name, successCallback, failureCallback);
cssModel


More information about the webkit-reviews mailing list