Summary: | Web Inspector: persist DOM breakpoints between page reloads | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Pavel Podivilov <podivilov> | ||||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | commit-queue, joepeck, yurys | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | OS X 10.5 | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 45209 | ||||||||||||
Attachments: |
|
Description
Pavel Podivilov
2010-08-29 06:32:50 PDT
Created attachment 65902 [details]
Proposed patch.
Comment on attachment 65902 [details] Proposed patch. > +++ b/LayoutTests/inspector/dom-breakpoints.html > @@ -35,7 +35,7 @@ var test = function() > function step2(node) > { > InspectorTest.addResult("Found dom node d0."); > - WebInspector.domBreakpointManager.setBreakpoint(node, WebInspector.DOMBreakpoint.Types.SubtreeModified); > + node.setBreakpoint("SubtreeModified"); Use constants for such arguments. > +++ b/WebCore/inspector/front-end/DOMAgent.js > @@ -143,6 +143,40 @@ WebInspector.DOMNode.prototype = { > this.ownerDocument._domAgent.removeAttributeAsync(this, name, callback); > }, > > + path: function() > + { > + var path = []; > + var node = this; > + while (node && "index" in node && node.nodeName && node.nodeName.length) { Is it possible that there is node.index but but no node.nodeName? If we restore breakpoints from the front-end we may miss some early events, is it intentional or should we move the logic to the backend? Pavel, please use the http://webkit.org/new-inspector-bug link, or the "Inspector" link from within Bugzilla <https://bugs.webkit.org/enter_bug.cgi>. I would like to get CC'd on your bugs. That automatically applies the proper template and a CC list of interested people (including reviewers to get extra eyes on patches). Thanks! Created attachment 66047 [details]
All comments addressed.
(In reply to comment #4) > Pavel, please use the http://webkit.org/new-inspector-bug link, or the "Inspector" > link from within Bugzilla <https://bugs.webkit.org/enter_bug.cgi>. I would like to > get CC'd on your bugs. That automatically applies the proper template and a CC > list of interested people (including reviewers to get extra eyes on patches). Thanks! ok Comment on attachment 66047 [details] All comments addressed. View in context: https://bugs.webkit.org/attachment.cgi?id=66047&action=prettypatch > WebCore/inspector/front-end/DOMAgent.js:804 > + WebInspector.DOMBreakpoint._ids = {}; Why do we need both integer and string type value? We should either use strings or intergers everywhere, r- for this. Given that the backend uses integers and we have named constants in the frontend it seems like we may well get rid of string values. > WebCore/inspector/front-end/DOMAgent.js:842 > + WebInspector.DOMBreakpoint._contextMenuLabels[WebInspector.DOMBreakpoint.Types.NodeRemoved] = WebInspector.UIString("Break on Node Removal"); Revert this please. Created attachment 66353 [details]
Proposed patch.
Comments addressed. Added test for restoring DOM breakpoints on page reload.
Created attachment 66359 [details]
Proposed patch.
Comments addressed. Added test for restoring DOM breakpoints on page reload.
Comment on attachment 66359 [details] Proposed patch. Clearing flags on attachment: 66359 Committed r66712: <http://trac.webkit.org/changeset/66712> All reviewed patches have been landed. Closing bug. |