1836755 - CSS variables set on the shadow dom host element are shown as not set in the inherited dom

Status: RESOLVED FIXED

(DevTools :: Inspector: Rules, defect)

Description

[Abhijeet Chawla [:ff2400t]]

Actual results:

CSS variables set on the Custom Element Root with the :host selector are show as being not set in the Rules Panel i.e. their color is grey. Example

Comment 1

[Julian Descottes [:jdescottes]]

Thanks for the report, will investigate.

Comment 2

[Julian Descottes [:jdescottes]]

The issue here is around our logic to decide if a specific selector matches an element, which relies on _buildMatchedRules and selectorMatchesElement (https://searchfox.org/mozilla-central/rev/aec3a901e6f6b3041b5ec457c9111a042cef1fb1/devtools/server/actors/inspector/css-logic.js#471-483)

They both loop on

   while (
      (element = element.parentNode) &&
      element.nodeType === nodeConstants.ELEMENT_NODE
    );

which makes them miss selectors which apply to the shadow root.

I tried updating the code to loop on shadow hosts when encountering a shadow root, but I am blocked on InspectorUtils.selectorMatchesElement(element, domRule, idx) which only seems to accept elements, and I'm not sure how this could be used to match the :host pseudo selector.

Emilio: do you have a suggestion to match :host selectors using selectorMatchesElement or should we have some custom logic?

Comment 3

[Emilio Cobos Álvarez [:emilio]]

So if the element is a shadow root, you should pass it's .host, which is an element.

But I'm a bit confused about what that code is trying to do. Is it building rules from inheritance ancestors? Because looking at .parentNode is not the right thing to check there, you want .flattenedTreeParentNode.

But all those rules come from the style engine, so at least one of them necessarily match, can't you skip all that work when the selector list length is 1?

Comment 4

[Julian Descottes [:jdescottes]]

Thanks for flattenedTreeParentNode, I didn't know about that.

I guess I got confused when debugging, the problem is actually that when looping on flattenedTreeParentNode, selectorMatchesElement always returns false for the :host rule.

Now for the reason why we have this logic is that for each rule we return for an element, we keep track of which selectors are matching it. We render "matching" selectors differently from non-matching selectors. And if users update the selector so that it no longer matches the element, we keep the rule displayed as the long as the user doesn't change the selected element (because otherwise it would very hard for them to edit selectors). So basically we try to track quite finely which selectors of a dom rule match the element.

And finally our logic to display the value of css variables is fully client side and relies on this. We check the rules displayed for the element which have a matching selector, and we see if we can find the css variable defined in there.

Not against revisiting this, but do you think selectorMatchesElement could be updated to return true when used with a :host rule on the corresponding host element?

Comment 5

[Emilio Cobos Álvarez [:emilio]]

So part of the issue is that whether :host matches depends on where the rule is.

E.g., if I have:

<style>
#div, :host {
  color: red;
}
</style>
<div id="div"></div>
<script>
  div.attachShadow({ mode: "open" }).innerHTML = `
    <style>#div, :host { background-color: red }</style>
  `;
</script>

Ideally the result should be that #div matches on the first one and :host on the second one.

Comment 6

[Emilio Cobos Álvarez [:emilio]]

I sent a patch to improve the behavior of :host. DevTools should still use .flattenedTreeParentNode, and fixing the second #div case there is a bit harder, but I don't think it matters much in practice.

Comment 7

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1836755 - Improve behavior of :host in InspectorUtils.selectorMatchesElement. r=jdescottes

For non-adopted sheets, use the owner tree. Those are always good to go.

For constructed stylesheets, try to find the most likely candidate. It's
not perfect but it's likely to always be correct in practice.

We could ask for the specific host instead, though that can get rather
annoying in practice.

Comment 8

[Julian Descottes [:jdescottes]]

Thanks Emilio! I'll take a look at the patch and will provide the devtools changes and test.

Comment 9

[Julian Descottes [:jdescottes]]

Attachment: Bug 1836755 - [devtools] Use flattenedTreeParentNode to build matching selectors

Depends on D180347

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0e2078b102ff
Improve behavior of :host in InspectorUtils.selectorMatchesElement. r=jdescottes,layout-reviewers,dshin

Comment 11

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/0e2078b102ff

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/05f4ed5e1027
[devtools] Use flattenedTreeParentNode to build matching selectors r=emilio,devtools-reviewers,nchevobbe

Comment 13

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/05f4ed5e1027

Comment 14

[Intermittent Failures Robot]

199 failures in 779 pushes (0.255 failures/push) were associated with this bug yesterday.

** This test has failed more than 150 times in the last 21 days. It should be disabled until it can be fixed. **

Repository breakdown:

• autoland: 199

Platform and build breakdown:

• gecko-decision: 2
  • opt: 2
• linux64: 5
  • asan: 1
  • opt: 2
  • tsan: 1
  • debug: 1
• windows2012-64: 3
  • debug: 1
  • opt: 1
  • asan: 1
• android-5-0-x86_64: 4
  • debug: 1
  • opt: 2
  • debug-isolated-process: 1
• lint: 7
  • opt: 7
• osx-cross: 3
  • opt: 1
  • debug: 1
  • plain: 1
• linux1804-64: 2
  • opt: 2
• windows2012-32: 2
  • opt: 1
  • debug: 1
• linux1804-64-qr: 85
  • debug: 65
  • opt: 20
• macosx1015-64-qr: 6
  • debug: 3
  • opt: 3
• windows11-32-2009-qr: 3
  • debug: 2
  • opt: 1
• windows11-64-2009-qr: 5
  • debug: 3
  • opt: 2
• android-em-7-0-x86_64-qr: 3
  • debug-isolated-process: 1
  • debug: 1
  • opt: 1
• linux1804-64-tsan-qr: 5
  • opt: 5
• android-em-7-0-x86_64-lite-qr: 1
  • opt: 1
• linux1804-64-asan-qr: 62
  • opt: 62
• windows11-64-2009-asan-qr: 1
  • opt: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1836755&startday=2023-06-14&endday=2023-06-14&tree=all

Comment 15

[Natalia Csoregi [:nataliaCs]]

Removing the [stockwell disable-recommended] tag as it was caused by a spam user starring jobs against this bug.
Sorry for the inconvenience!