Summary: | Web Inspector: show DOM breakpoints in sidebar pane | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Pavel Podivilov <podivilov> | ||||||||||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||
Severity: | Normal | CC: | commit-queue, pfeldman, yurys | ||||||||||||||||
Priority: | P2 | ||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||
Hardware: | PC | ||||||||||||||||||
OS: | OS X 10.5 | ||||||||||||||||||
Attachments: |
|
Description
Pavel Podivilov
2010-08-23 07:52:33 PDT
Created attachment 65123 [details]
Proposed patch.
Created attachment 65129 [details]
Screenshot.
Created attachment 65246 [details]
Add check boxes.
Created attachment 65247 [details]
Screenshot.
Created attachment 65252 [details]
Add check boxes.
Comment on attachment 65252 [details]
Add check boxes.
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.SubtreeModified] = "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
Created attachment 65264 [details]
Proposed patch.
Created attachment 65265 [details]
What's changed from previous patch.
Comment on attachment 65264 [details]
Proposed patch.
WebCore/inspector/front-end/BreakpointsSidebarPane.js:203
+ return this._breakpoint.type < other._breakpoint.type ? -1 : 1;
You may replace this with
return this._breakpoint.type - other._breakpoint.type;
WebCore/inspector/front-end/DOMAgent.js:703
+ for (var type in nodeBreakpoints)
This will fail if nodeBreakpoints is undefined.
Comment on attachment 65264 [details] Proposed patch. Clearing flags on attachment: 65264 Committed r65939: <http://trac.webkit.org/changeset/65939> All reviewed patches have been landed. Closing bug. |