[webkit-reviews] review canceled: [Bug 44424] Web Inspector: show DOM breakpoints in sidebar pane : [Attachment 65252] Add check boxes.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Aug 24 05:09:48 PDT 2010
Pavel Feldman <pfeldman at chromium.org> has canceled Pavel Podivilov
<podivilov at chromium.org>'s request for review:
Bug 44424: Web Inspector: show DOM breakpoints in sidebar pane
https://bugs.webkit.org/show_bug.cgi?id=44424
Attachment 65252: Add check boxes.
https://bugs.webkit.org/attachment.cgi?id=65252&action=review
------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
Great job! Please fix style nits and we can land this.
General nit: Instead of setting listeners to the individual breakpoints, you
should add single one to the breakpoint
manager.WebCore/inspector/front-end/BreakpointsSidebarPane.js:202
(can be done later)
+
WebInspector.DOMBreakpointItem.Labels[WebInspector.DOMBreakpoint.Types.SubtreeM
odified] = "Subtree Modified";
It is better to move UIString call here.
WebCore/inspector/front-end/BreakpointsSidebarPane.js:212
+ element: WebInspector.JSBreakpointItem.prototype.element,
You should either inherit or aggregate.
WebCore/inspector/front-end/inspector.js:203
+ WebInspector.breakpointManager.addEventListener("breakpoint-added",
function(event)
Please follow style guidelines when declaring functions.
WebCore/inspector/front-end/inspector.js:213
+
WebInspector.domBreakpointManager.addEventListener("dom-breakpoint-added",
function(event)
ditto
More information about the webkit-reviews
mailing list