RESOLVED FIXED 44424
Web Inspector: show DOM breakpoints in sidebar pane
https://bugs.webkit.org/show_bug.cgi?id=44424
Summary Web Inspector: show DOM breakpoints in sidebar pane
Pavel Podivilov
Reported 2010-08-23 07:52:33 PDT
Web Inspector: show DOM breakpoints in sidebar pane
Attachments
Proposed patch. (29.28 KB, patch)
2010-08-23 08:50 PDT, Pavel Podivilov
no flags
Screenshot. (107.31 KB, image/png)
2010-08-23 09:11 PDT, Pavel Podivilov
no flags
Add check boxes. (31.88 KB, patch)
2010-08-24 02:36 PDT, Pavel Podivilov
no flags
Screenshot. (85.13 KB, image/png)
2010-08-24 02:41 PDT, Pavel Podivilov
no flags
Add check boxes. (32.27 KB, patch)
2010-08-24 03:17 PDT, Pavel Podivilov
no flags
Proposed patch. (35.06 KB, patch)
2010-08-24 06:34 PDT, Pavel Podivilov
no flags
What's changed from previous patch. (13.66 KB, patch)
2010-08-24 06:36 PDT, Pavel Podivilov
no flags
Pavel Podivilov
Comment 1 2010-08-23 08:50:49 PDT
Created attachment 65123 [details] Proposed patch.
Pavel Podivilov
Comment 2 2010-08-23 09:11:52 PDT
Created attachment 65129 [details] Screenshot.
Pavel Podivilov
Comment 3 2010-08-24 02:36:06 PDT
Created attachment 65246 [details] Add check boxes.
Pavel Podivilov
Comment 4 2010-08-24 02:41:04 PDT
Created attachment 65247 [details] Screenshot.
Pavel Podivilov
Comment 5 2010-08-24 03:17:55 PDT
Created attachment 65252 [details] Add check boxes.
Pavel Feldman
Comment 6 2010-08-24 05:09:48 PDT
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
Pavel Podivilov
Comment 7 2010-08-24 06:34:31 PDT
Created attachment 65264 [details] Proposed patch.
Pavel Podivilov
Comment 8 2010-08-24 06:36:08 PDT
Created attachment 65265 [details] What's changed from previous patch.
Yury Semikhatsky
Comment 9 2010-08-24 07:58:40 PDT
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.
WebKit Commit Bot
Comment 10 2010-08-24 15:17:35 PDT
Comment on attachment 65264 [details] Proposed patch. Clearing flags on attachment: 65264 Committed r65939: <http://trac.webkit.org/changeset/65939>
WebKit Commit Bot
Comment 11 2010-08-24 15:17:40 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.