[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