1995943 - InspectorUtils.selectorMatchesElement returns invalid result for nested shadow host with shared stylesheet

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Nicolas Chevobbe [:nchevobbe]]

Attachment: nested-shadow-host-matches-selector.html

Steps to reproduce

1. Open the attached page
2. Inspect the "nested shadow dom section" element
3. Hover the --a variable in the section { color: var(--a); } rule

Expected results

The popup indicates that --a is red

Actual results

The popup says that --a is not defined

---

This is caused by CSSStyleRule::selectorMatchesElement not property working for the :host rule on the nested shadow host for the shared stylesheet.

D269727 highlights the issue.
CSSStyleRule::selectorMatchesElement was fixed to handle :host in Bug 1836755 , but I guess we missed this specific case

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Attachment: rules view with the variable popup indicating that --a is not set, when we can clearly see it in the inherited section

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1995943 - Add test for CSSStyleRule::selectorMatchesElement on :host rules. r=emilio.

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Emilio, since you took care of Bug 1836755, do you know what's happening here?

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

Emilio, since you took care of Bug 1836755, do you know what's happening here?

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1995943 - [devtools] Add rules view test for CSS variables from shared stylesheet used in nested shadow dom element. r=#devtools.

Comment 6

[Nicolas Chevobbe [:nchevobbe]]

Added rules view test checking the variable in D269733

Comment 7

[Emilio Cobos Álvarez [:emilio]]

When you call .selectorMatchesElement on a rule from an adopted stylesheet, both answers are technically correct depending on which scope you're matching from. In the regressing bug I added this comment which is still true.

I guess we could try to match all the potentially-relevant hosts? So basically changing that GetHost(..) call by a ForEachHost? I guess that wouldn't be too bad, you're likely to want that answer...

Do you want to give that a try? might not be hard to fix :)

Comment 8

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1995943 - Check all potentially related hosts in CSSStyleRule::SelectorMatchesElement. r=emilio.

Comment 9

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #7)

When you call .selectorMatchesElement on a rule from an adopted stylesheet, both answers are technically correct depending on which scope you're matching from. In the regressing bug I added this comment which is still true.

I guess we could try to match all the potentially-relevant hosts? So basically changing that GetHost(..) call by a ForEachHost? I guess that wouldn't be too bad, you're likely to want that answer...

Do you want to give that a try? might not be hard to fix :)

Not sure I understand everything, but I gave it a try: D269931 , we can discuss the details on phab :)

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/5d0f81407228
https://hg.mozilla.org/integration/autoland/rev/683392c38072
Check all potentially related hosts in CSSStyleRule::SelectorMatchesElement. r=emilio,firefox-style-system-reviewers.
https://github.com/mozilla-firefox/firefox/commit/e999124172a9
https://hg.mozilla.org/integration/autoland/rev/f78c58f49a96
Add test for CSSStyleRule::selectorMatchesElement on :host rules. r=layout-reviewers,dshin.

Comment 11

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/683392c38072
https://hg.mozilla.org/mozilla-central/rev/f78c58f49a96

Comment 12

[Phabricator Automation]

Comment on attachment 9521916 [details]
Bug 1995943 - [devtools] Add rules view test for CSS variables from shared stylesheet used in nested shadow dom element. r=#devtools.

Revision D269733 was moved to bug 1996508. Setting attachment 9521916 [details] to obsolete.