1441535 - Figure out what devtools wants document.getAnonymousNodes to do wrt shadow DOM.

Status: RESOLVED FIXED

(DevTools :: General, enhancement, P3)

Description

[Emilio Cobos Álvarez (:emilio)]

Bug 1425759 makes nsIDocument::GetAnonymousNodes not return Shadow DOM nodes, since they're not part of the XBL stuff.

I haven't fixed right away because ideally we wouldn't mix the concepts, but I can keep working as it was if you want.

There's a todo_is about the children of a shadow host in getChildren not returning them.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Julian, any opinion on this since you're working on bug 1053898?

Comment 2

[Emilio Cobos Álvarez (:emilio)]

This makes getChildren not get the children of the shadow host. My point is that you can check whether the node is a shadow host anyway explicitly, instead of relying on getAnonymousNodes.

Comment 3

[Julian Descottes [:jdescottes]]

Thanks for the ping. This should not have a big impact on our implementation. 

The only two sites relying on this are:
- https://searchfox.org/mozilla-central/rev/14d933246211b02f5be21d2e730a57cf087c6606/devtools/shared/layout/utils.js#558
  which already special cases shadowdom right before calling getAnonymous
- https://searchfox.org/mozilla-central/rev/14d933246211b02f5be21d2e730a57cf087c6606/devtools/server/actors/inspector/node.js#189-190
  this one needs updating, and it's most likely why browser_markup_anonymous_03.js needed to be disabled in the parent bug

I'll fix it either in this bug or in bug 1053898.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

(In reply to Julian Descottes [:jdescottes][:julian] from comment #3)
> Thanks for the ping. This should not have a big impact on our
> implementation. 
> 
> The only two sites relying on this are:
> -
> https://searchfox.org/mozilla-central/rev/
> 14d933246211b02f5be21d2e730a57cf087c6606/devtools/shared/layout/utils.js#558
>   which already special cases shadowdom right before calling getAnonymous

Actually that special-case can go now, nice :)

> https://searchfox.org/mozilla-central/rev/
> 14d933246211b02f5be21d2e730a57cf087c6606/devtools/server/actors/inspector/
> node.js#189-190
>   this one needs updating, and it's most likely why
> browser_markup_anonymous_03.js needed to be disabled in the parent bug

Right.
 
> I'll fix it either in this bug or in bug 1053898.

Thanks a lot!

Comment 5

[Julian Descottes [:jdescottes]]

Attachment: Bug 1441535 - Remove unnecessary shadowdom check in isXBLAnonymous;

doc.getAnonymousNodes no longer returns shadow dom nodes, so no need
to filter them out in isXBLAnonymous.

This feature is tested in browser_markup_anonymous_02.js so reenabling
this test.

toolbarbutton has 4 children now, but since our tests are no longer
running in non-e10s, the regression was missed!

Review commit: https://reviewboard.mozilla.org/r/259062/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/259062/

Comment 6

[Brian Grinstead [:bgrins]]

Comment on attachment 8994514 [details]
Bug 1441535 - Remove unnecessary shadowdom check in isXBLAnonymous;

https://reviewboard.mozilla.org/r/259062/#review266094

Thanks

Comment 7

[Julian Descottes [:jdescottes]]

Thanks for the review Brian! Try at https://treeherder.mozilla.org/#/jobs?repo=try&revision=1679a13e3bbb6bf3ac43bfb98f8ce46023f698ad

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/26cf4424046e
Remove unnecessary shadowdom check in isXBLAnonymous;r=bgrins

Comment 9

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/26cf4424046e