Bug 44424 - Web Inspector: show DOM breakpoints in sidebar pane
Summary: Web Inspector: show DOM breakpoints in sidebar pane
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-23 07:52 PDT by Pavel Podivilov
Modified: 2010-08-24 15:17 PDT (History)
3 users (show)

See Also:

Attachments
Proposed patch. (29.28 KB, patch)
2010-08-23 08:50 PDT, Pavel Podivilov 		no flags Details | Formatted Diff | Diff
Screenshot. (107.31 KB, image/png)
2010-08-23 09:11 PDT, Pavel Podivilov 		no flags Details
Add check boxes. (31.88 KB, patch)
2010-08-24 02:36 PDT, Pavel Podivilov 		no flags Details | Formatted Diff | Diff
Screenshot. (85.13 KB, image/png)
2010-08-24 02:41 PDT, Pavel Podivilov 		no flags Details
Add check boxes. (32.27 KB, patch)
2010-08-24 03:17 PDT, Pavel Podivilov 		no flags Details | Formatted Diff | Diff
Proposed patch. (35.06 KB, patch)
2010-08-24 06:34 PDT, Pavel Podivilov 		no flags Details | Formatted Diff | Diff
What's changed from previous patch. (13.66 KB, patch)
2010-08-24 06:36 PDT, Pavel Podivilov 		no flags Details | Formatted Diff | Diff
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Podivilov 2010-08-23 07:52:33 PDT 
Web Inspector: show DOM breakpoints in sidebar pane
Comment 1 Pavel Podivilov 2010-08-23 08:50:49 PDT 
Created attachment 65123 [details]
Proposed patch.
Comment 2 Pavel Podivilov 2010-08-23 09:11:52 PDT 
Created attachment 65129 [details]
Screenshot.
Comment 3 Pavel Podivilov 2010-08-24 02:36:06 PDT 
Created attachment 65246 [details]
Add check boxes.
Comment 4 Pavel Podivilov 2010-08-24 02:41:04 PDT 
Created attachment 65247 [details]
Screenshot.
Comment 5 Pavel Podivilov 2010-08-24 03:17:55 PDT 
Created attachment 65252 [details]
Add check boxes.
Comment 6 Pavel Feldman 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
Comment 7 Pavel Podivilov 2010-08-24 06:34:31 PDT 
Created attachment 65264 [details]
Proposed patch.
Comment 8 Pavel Podivilov 2010-08-24 06:36:08 PDT 
Created attachment 65265 [details]
What's changed from previous patch.
Comment 9 Yury Semikhatsky 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.
Comment 10 WebKit Commit Bot 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>
Comment 11 WebKit Commit Bot 2010-08-24 15:17:40 PDT 
All reviewed patches have been landed.  Closing bug.