1079185 - [markup view] Shadow DOM shown instead of light DOM

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Wilson Page [:wilsonpage]]

Attachment: 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.

Comment 1

[Wilson Page [:wilsonpage]]

Attachment: previous.png

Comment 2

[Wilson Page [:wilsonpage]]

Until we get a proper way to inspect shadow-dom, I would expect to see light-dom content.

Comment 3

[Brian Grinstead [:bgrins]]

Thanks for the report, what is the URL of the page that you are seeing this on?

Comment 4

[Wilson Page [:wilsonpage]]

(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.

Comment 5

[Gabor Krizsanits (INACTIVE)]

(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...

Comment 6

[Alexandre Poirot [:ochameau]]

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")

Comment 7

[Brian Grinstead [:bgrins]]

(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

Comment 8

[Brian Grinstead [:bgrins]]

> > 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.

Comment 9

[Brian Grinstead [:bgrins]]

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:

Comment 10

[Gabor Krizsanits (INACTIVE)]

(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?

Comment 11

[William Chen [: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().

Comment 12

[Gabor Krizsanits (INACTIVE)]

https://tbpl.mozilla.org/?tree=Try&rev=44087187b7e5

Comment 13

[Gabor Krizsanits (INACTIVE)]

Attachment: Using GetParentForNode in GetFlattenedTreeParent. v1

(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...

Comment 14

[Boris Zbarsky [: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.

Comment 15

[Gabor Krizsanits (INACTIVE)]

(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

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8ea35244fcfd