[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