1554009 - InspectorUtils.selectorMatchesElement is a bit broken in presence of :host and ::slotted rules.

Status: NEW

(DevTools :: Inspector, defect, P3)

Description

[Emilio Cobos Álvarez (:emilio)]

In particular, since we never pass it the shadow host the style rule is in, it'll never match these selectors.

This was caught by an assertion in bug 1544242.

This is fixable, CssStyleRule should just get the AssociatedDocumentOrShadowRoot() and pass it down or something.

I'm removing that assertion since even if we did that we could trigger it: The style system should never itself try to match a :host or ::slotted outside of a shadow tree, but the OM and inspector can try, and we'd correctly return false.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Julian, how do we use this API? I assume it's desirable to match these rules correctly? Or is this API not used for anything important? (I haven't seen any bug about this...)

Comment 2

[Julian Descottes [:jdescottes]]

Thanks for filing Emilio!

I think it is used in various places. Basically we are checking it in various places the check that a given rule actually applies to the current element.

So I think I have STRs for this bug in DevTools:

• go to https://juliandescottes.github.io/webcomponents-playground/slotted-selector/
• inspect the "green" container

Expected result: In the rule view, the rule background: rgb(255, 0, 0); should be marked as overridden (greyed + strikethrough)
Actual result: In the rule view, the rule background: rgb(255, 0, 0); has the default style.

This is related to the following code https://searchfox.org/mozilla-central/rev/662de518b1686c4769320d6b8825ce4864c4eda0/devtools/client/inspector/rules/models/element-style.js#285 . Basically we don't update properly this rule because we think it doesn't really apply to the element.

So it's probably not used in critical paths, that's why we didn't get any bug report so far. But it would be great to fix it!
Emilio do you have more pointers to fix this by any chance?

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Yes, this function: https://searchfox.org/mozilla-central/rev/662de518b1686c4769320d6b8825ce4864c4eda0/layout/style/CSSStyleRule.cpp#201

Should do something like:

Element* containingHost = nullptr;
if (StyleSheet* sheet = GetStyleSheet()) {
  if (ShadowRoot* shadow = sheet->GetContainingShadow()) {
    containingHost = shadow->Host();
  }
}

(Need to make GetContainingShadow() public).

Then you need to pass that as an argument to the style system, so like: https://searchfox.org/mozilla-central/rev/662de518b1686c4769320d6b8825ce4864c4eda0/servo/ports/geckolib/glue.rs#2147

But optional (so containing_host: Option<&RawGeckoElement>).

Then you need to use that in the MatchingContext, so:

ctx.current_host = containing_host.map(|e| GeckoElement(e).opaque())

In that function. That should do (And tests, of course :)).

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Alternatively, we could take the current host as an argument from devtools if that'd be useful.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:dholbert, could you have a look please?

For more information, please visit auto_nag documentation.