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

Description Matt Baker 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.
Comment 1 Joseph Pecoraro 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.
Comment 2 Matt Baker 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>"
Comment 3 Matt Baker 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>
Comment 4 Matt Baker 2017-03-17 19:56:57 PDT
Created attachment 304849 [details]
[Image] pause reason v1
Comment 5 Matt Baker 2017-03-17 19:57:33 PDT
Created attachment 304850 [details]
[Image] pause reason v2
Comment 6 Matt Baker 2017-03-18 02:17:43 PDT
Created attachment 304866 [details]
Patch
Comment 7 WebKit Commit Bot 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>
Comment 8 WebKit Commit Bot 2017-04-03 20:19:14 PDT
All reviewed patches have been landed.  Closing bug.