Closed Bug 1079185 Opened 7 years ago Closed 7 years ago

[markup view] Shadow DOM shown instead of light DOM

Categories

(DevTools :: Inspector, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 36

People

(Reporter: wilsonpage, Assigned: gkrizsanits)

References

(Depends on 1 open bug)

Details

Attachments

(3 files)

Attached image current.png
Previously inspector used to show the light-dom content within a shadow-host. Now the light-dom content is hidden and the inspector shows the shadow-dom.
Attached image previous.png
Until we get a proper way to inspect shadow-dom, I would expect to see light-dom content.
Thanks for the report, what is the URL of the page that you are seeing this on?
Component: Developer Tools → Developer Tools: Inspector
OS: Mac OS X → All
Hardware: x86 → All
Summary: [inspector] Shadow DOM shown instead of light DOM → [markup view] Shadow DOM shown instead of light DOM
(In reply to Brian Grinstead [:bgrins] from comment #3)
> Thanks for the report, what is the URL of the page that you are seeing this
> on?

You can use http://gaia-components.github.io/gaia-components as a reference, but I'm pretty sure this will recreate on all shadow-dom examples.
See Also: → 1053898
(In reply to Wilson Page [:wilsonpage] from comment #2)
> Until we get a proper way to inspect shadow-dom, I would expect to see
> light-dom content.

What are we missing right now for "proper way to inspect shadow-dom"? Is there any platform work that needed to be done here? I think all the necessary APIs for this are either there or should be trivial to add, but of course on devtools side I expect more work than that...
OK so I looked at the current state of shadow dom support and various things are missing or broken.
Take http://gaia-components.github.io/gaia-components <gaia-list> element for example:
(See http://gaia-components.github.io/gaia-components/bower_components/gaia-list/gaia-list.js and the `var template` definition)

1) Shadow dom is partial, it looks like you can only see the direct children of the shadow root and that's all.
For <gaia-list> we only see <style> and <div class="inner" />
2) When selecting one of those shadow dom element, the inspector panels seems broken. Nothing appear in Rules nor computed styles
3) <content> nodes don't appear. It may be on purpose? I don't know if it is important to see them or not, but we really need to see the non shadow dom element selected in <content> node! But may be that's broken because of 1)?

At the end I don't know if we really care that much about all the discussion from bug 1053898.
I think we would mostly need some bugfixes in order to get something that just work, even if the presentation is not perfect. But clearly, today, we have something somewhat broken when hitting a web component.

One obvious improvement would be to be able to distinguish shadow dom from regular dom.
Another would be to be able to ignore shadow dom completely and only display regular dom nodes on demand.
(Both features were available in the good old "DOM Inspector")
(In reply to Alexandre Poirot [:ochameau] from comment #6)
> OK so I looked at the current state of shadow dom support and various things
> are missing or broken.
> Take http://gaia-components.github.io/gaia-components <gaia-list> element
> for example:
> (See
> http://gaia-components.github.io/gaia-components/bower_components/gaia-list/
> gaia-list.js and the `var template` definition)
> 
> 1) Shadow dom is partial, it looks like you can only see the direct children
> of the shadow root and that's all.
> For <gaia-list> we only see <style> and <div class="inner" />

It looks like there are errors in the console when trying to set current node on the walker [0].  This seems also related to Bug 1079195.  Just setting up a very basic test case I've not yet been able to reproduce, so I'll have to try to expand that case until I bump into the problem.

> 2) When selecting one of those shadow dom element, the inspector panels
> seems broken. Nothing appear in Rules nor computed styles

The rule view seems to generally work.  I see some UA styles on the elements, and if I add a new inline style in the rule view it does seem to apply (like some padding on <div class="inner">, for instance).

> 3) <content> nodes don't appear. It may be on purpose? I don't know if it is
> important to see them or not, but we really need to see the non shadow dom
> element selected in <content> node! But may be that's broken because of 1)?
> 
> At the end I don't know if we really care that much about all the discussion
> from bug 1053898.

<content> nodes are not removed on purpose, I'm guessing that they just aren't enumerated by the deep tree walker.  Where / how <content> nodes should show up seems to be related to bug 1053898, where we are discussing how to best display insertion points.

[0]:  Message: [Exception... "Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [inIDeepTreeWalker.currentNode]"  nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/inspector.js :: DocumentWalker :: line 3159"  data: no]
    DocumentWalker@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/inspector.js:3159:2
WalkerActor<.children</getFilteredWalker@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/inspector.js:1516:13
WalkerActor<.children<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/inspector.js:1541:25
actorProto/</handler@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:987:18
DSC_onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/main.js:1407:14
LocalDebuggerTransport.prototype.send/<@resource://gre/modules/devtools/dbg-client.jsm -> resource://gre/modules/devtools/transport/transport.js:545:10
makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/DevToolsUtils.js:83:13
makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/DevToolsUtils.js:83:13
Depends on: 1079195
> > 1) Shadow dom is partial, it looks like you can only see the direct children
> > of the shadow root and that's all.
> > For <gaia-list> we only see <style> and <div class="inner" />
> 
> It looks like there are errors in the console when trying to set current
> node on the walker [0].  This seems also related to Bug 1079195.  Just
> setting up a very basic test case I've not yet been able to reproduce, so
> I'll have to try to expand that case until I bump into the problem.

OK, I've come up with a reduced case that shows the error here: http://jsfiddle.net/bgrins/e8m8f4ed/.  In this case I see the same problems and errors that happen on http://gaia-components.github.io/gaia-components when there is a content insertion point that is wrapped within an element but it doesn't fail when the insertion point is a direct child of the shadow root.  So: <header><content select='h2'></content></header> fails but <content select='h2'></content> is OK.
When loading http://fiddle.jshell.net/bgrins/e8m8f4ed/show/ and navigating the inspector to '#broken' > 'header' this is what I see with extra logging around the call to set currentNode [0]:

console.log: About to set current node ShadowRoot {}
console.log: Done setting current node
console.log: About to set current node <HEADER> 
console.log: Done setting current node
console.log: About to set current node <HEADER> 
console.log: Done setting current node
console.log: About to set current node <H2>
console.error: 
  Message: [Exception... "Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [inIDeepTreeWalker.currentNode]"  nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/inspector.js :: DocumentWalker :: line 3159"  data: no]
  Stack:
    DocumentWalker@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/inspector.js:3159:2
WalkerActor<.children</getFilteredWalker@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/inspector.js:1516:13
WalkerActor<.children<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/inspector.js:1541:25
actorProto/</handler@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:987:18
DSC_onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/main.js:1407:14
LocalDebuggerTransport.prototype.send/<@resource://gre/modules/devtools/dbg-client.jsm -> resource://gre/modules/devtools/transport/transport.js:545:10
makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/DevToolsUtils.js:83:13
makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/DevToolsUtils.js:83:13

Gabor, any idea why setting currentNode on the element resulting from a content insertion point would throw an NS_ERROR_ILLEGAL_VALUE exception?

[0]: http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/inspector.js#3158, this is what I see: console.log:
Flags: needinfo?(gkrizsanits)
(In reply to Brian Grinstead [:bgrins] from comment #9)
> Gabor, any idea why setting currentNode on the element resulting from a
> content insertion point would throw an NS_ERROR_ILLEGAL_VALUE exception?

Yes. After debugging, the problem is that the DOMUtils::GetParentForNode when ShowAnonymousContent is on, does not return the parent in the distributed tree, but the light DOM parent instead.

For XBL I think this part http://mxr.mozilla.org/mozilla-central/source/layout/inspector/inDOMUtils.cpp#159 ensures what we need here, shouldn't this just work for shadow DOM as well?

William, is there a way to get the parent in the distributed tree of a node? Shouldn't DOMUtils::GetParentForNode do that like it does for XBL?
Flags: needinfo?(gkrizsanits) → needinfo?(wchen)
You should be able to use nsIContent::GetFlattenedTreeParent() to get the parent in the "distributed tree" for both XBL and Shadow DOM. If we want inDOMUtils::GetParentForNode to get the parent in the composed tree, then we should use GetFlattenedTreeParent() instead of GetXBLInsertionParent().
Flags: needinfo?(wchen)
(In reply to William Chen [:wchen] from comment #11)
> You should be able to use nsIContent::GetFlattenedTreeParent() to get the
> parent in the "distributed tree" for both XBL and Shadow DOM. If we want
> inDOMUtils::GetParentForNode to get the parent in the composed tree, then we
> should use GetFlattenedTreeParent() instead of GetXBLInsertionParent().

As far as I can tell that is the intention there... If I'm wrong I could still add another flag to the function like aShowingShadowContent, but that would be quite redundant and would be great to avoid it...
Attachment #8512706 - Flags: review?(bzbarsky)
Comment on attachment 8512706 [details] [diff] [review]
Using GetParentForNode in GetFlattenedTreeParent. v1

This seems fine as long as the APIs for going down the tree consider shadow DOM.
Attachment #8512706 - Flags: review?(bzbarsky) → review+
(In reply to Boris Zbarsky [:bz] from comment #14)
> This seems fine as long as the APIs for going down the tree consider shadow
> DOM.

Yes, it seems to be using GetChildren(nsIContent::eAllChildren) for that.

https://hg.mozilla.org/integration/mozilla-inbound/rev/8ea35244fcfd
https://hg.mozilla.org/mozilla-central/rev/8ea35244fcfd
Assignee: nobody → gkrizsanits
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 36
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.