Bug 169479

Summary: Web Inspector: Show additional pause reason details for DOM "subtree modified" breakpoint
Product: WebKit Reporter: Matt Baker <mattbaker>
Component: Web InspectorAssignee: Matt Baker <mattbaker>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, inspector-bugzilla-changes, joepeck, timothy
Priority: P2    
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
[Image] pause reason UI
none
[Image] pause reason v1
none
[Image] pause reason v2
none
Patch none

Matt Baker
Reported 2017-03-10 12:01:07 PST
Created attachment 304061 [details] [Image] pause reason UI Summary: Show additional pause reason details for DOM "subtree modified" breakpoint. Make use of the `insertion` and `targetNode` info sent from the backend. insertion (boolean) - disambiguates a subtree modification (node inserted or removed) taretNode (object) - if insertion = true, remote object payload for the DOM node owning the breakpoint (how is this helpful?) - if insertion = false, remote object payload for the DOM node being removed Notes: Attached is a WIP UI for the insertion = false case.
Attachments
[Image] pause reason UI (344.09 KB, image/png)
2017-03-10 12:01 PST, Matt Baker 		no flags
Details
[Image] pause reason v1 (638.84 KB, image/png)
2017-03-17 19:56 PDT, Matt Baker 		no flags
Details
[Image] pause reason v2 (104.60 KB, image/png)
2017-03-17 19:57 PDT, Matt Baker 		no flags
Details
Patch (6.21 KB, patch)
2017-03-18 02:17 PDT, Matt Baker 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Joseph Pecoraro
Comment 1 2017-03-10 12:54:41 PST
So when we pause for exceptions we expose a `$exception` value in the console. Something similar might be neat here as well, but I don't think any name will be obvious. We could show it in the sidebar kind of like your image. I agree the targetNode should probably always be the node being inserted/removed. We should already know the node owning the breakpoint.
Matt Baker
Comment 2 2017-03-12 12:33:06 PDT
(In reply to comment #0) > Created attachment 304061 [details] > [Image] pause reason UI > > Summary: > Show additional pause reason details for DOM "subtree modified" breakpoint. > Make use of the `insertion` and `targetNode` info sent from the backend. > > insertion (boolean) > - disambiguates a subtree modification (node inserted or removed) > > taretNode (object) > - if insertion = true, remote object payload for the DOM node owning the > breakpoint (how is this helpful?) I spoke too soon. The comment on InspectorDOMDebuggerAgent.cpp:307 explains: "For inheritable breakpoint types, target node isn't always the same as the node that owns a breakpoint." The following scenario would be an example: <body> <-- set a 'Subtree Modified' breakpoint here <div> <p>...</p> <--- Insert a new <p> here </div> </body> target node: "<div>" owner node: "<body>"
Matt Baker
Comment 3 2017-03-12 12:41:52 PDT
For the following DOM tree with a 'Subtree Modified' breakpoint set on body: > <body> <div> <p>...</p> </div> </body> Removing the paragraph should break and show: [P] Triggered DOM Breakpoint Subtree Modified Remove descendant <p> Inserting a new paragraph at the same spot should break and show: [P] Triggered DOM Breakpoint Subtree Modified Append child to <div>
Matt Baker
Comment 4 2017-03-17 19:56:57 PDT
Created attachment 304849 [details] [Image] pause reason v1
Matt Baker
Comment 5 2017-03-17 19:57:33 PDT
Created attachment 304850 [details] [Image] pause reason v2
Matt Baker
Comment 6 2017-03-18 02:17:43 PDT
Created attachment 304866 [details] Patch
WebKit Commit Bot
Comment 7 2017-04-03 20:19:12 PDT
Comment on attachment 304866 [details] Patch Clearing flags on attachment: 304866 Committed r214861: <http://trac.webkit.org/changeset/214861>
WebKit Commit Bot
Comment 8 2017-04-03 20:19:14 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.