Bug 169086 - Web Inspector: Should be able to push nodes in an iframe subtree to the frontend
Summary: Web Inspector: Should be able to push nodes in an iframe subtree to the frontend
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Matt Baker
URL:
Keywords:
Depends on:
Blocks: 168101
  Show dependency tree / graph
 
Reported: 2017-03-02 11:17 PST by Matt Baker
Modified: 2017-03-02 21:52 PST (History)
2 users (show)

See Also:

Attachments
[WebArchive] Test page for iframes/frameset (1.18 KB, application/x-webarchive)
2017-03-02 13:38 PST, Matt Baker 		no flags Details
Patch (9.26 KB, patch)
2017-03-02 20:48 PST, Matt Baker 		no flags Details | Formatted Diff | Diff
[Video] Restoring selection from iframe DOM node (312.51 KB, video/mp4)
2017-03-02 20:57 PST, Matt Baker 		no flags Details
Patch for landing (9.71 KB, patch)
2017-03-02 21:11 PST, Matt Baker 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) 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 Matt Baker 2017-03-02 11:17:11 PST 
Summary:
Should be able to push nodes in an iframe subtree to the frontend.

Steps to Reproduce:
1. Inspect theverge.com > goto Elements tab
2. Locate an iframe element (Cmd+F helps)
3. Expand the iframe's #document node, select the <html> element
4. Reload page
=> Expected: iframe's <html> element selected after DOM tree refresh
=> Actual: the default element (main frame <body>) is selected

Note:
It looks like InspectorDOMAgent::nodeForPath is halting when it encounters the iframe element, because InspectorDOMAgent::innerChildNodeCount returns 0.
Comment 1 Matt Baker 2017-03-02 11:57:14 PST 
(In reply to comment #0)
> Note:
> It looks like InspectorDOMAgent::nodeForPath is halting when it encounters
> the iframe element, because InspectorDOMAgent::innerChildNodeCount returns 0.

Due to the check for matching URLs on DOMTreeContentView.js:365, elements in an iframe's subtree are never requested via InspectorDOMAgent::pushNodeByPathToFrontend. Commenting out the check results in the protocol method being called, but the agent still fails to push the node for the reason above.
Comment 2 Matt Baker 2017-03-02 13:38:34 PST 
Created attachment 303230 [details]
[WebArchive] Test page for iframes/frameset

Test page includes:

-iframe with src="about:blank"
-iframe with <frameset> source*
-iframe with srcdoc content

*Although <frameset> is deprecated in HTML5, the fix for this bug should just work for all nodes derived from WebCore::HTMLFrameElementBase.
Comment 3 Matt Baker 2017-03-02 20:48:55 PST 
Created attachment 303296 [details]
Patch
Comment 4 Matt Baker 2017-03-02 20:57:54 PST 
Created attachment 303297 [details]
[Video] Restoring selection from iframe DOM node
Comment 5 Joseph Pecoraro 2017-03-02 20:59:38 PST 
Comment on attachment 303296 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=303296&action=review

Neat! r=me

> Source/WebCore/inspector/InspectorDOMAgent.cpp:2170
> +                return 0;

Style: You may want to change all of these `return 0` to `return nullptr`. I thought we had already updated our code to use nullptr, but clearly not.

> LayoutTests/inspector/dom/push-node-by-path-to-frontend.html:13
> +            name,
> +            description,

Style: I normally do `name, description` on one line since multiline these don't add much.
Comment 6 Matt Baker 2017-03-02 21:11:31 PST 
Created attachment 303299 [details]
Patch for landing
Comment 7 WebKit Commit Bot 2017-03-02 21:52:30 PST 
Comment on attachment 303299 [details]
Patch for landing

Clearing flags on attachment: 303299

Committed r213353: <http://trac.webkit.org/changeset/213353>
Comment 8 WebKit Commit Bot 2017-03-02 21:52:35 PST 
All reviewed patches have been landed.  Closing bug.