1604989 - CSS Shadow Parts rules only apply to first of type

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect, P2)

Description

[Alexander Marks]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/79.0.3945.79 Safari/537.36

Steps to reproduce:

It seems that in Firefox Nightly 73.0a1 (2019-12-18) with layout.css.shadow-parts.enabled=true, a ::part rule is only applied to elements that are the first of their type within their shadow root. (I might not have fully pinned down the behavior, but this seems to match what I'm seeing).

For example, given the rule:

<style>
::part(redpart) { color: red; }
</style>

Then this part will correctly be red:

<shadow-host>
#shadow-root
<div part="redpart">correctly red</div>

But this one will not be red:

<shadow-host>
#shadow-root
<div></div>
<div part="redpart">incorrectly black</div>

And if I add another node with the same part attribute, but that is a different element, then that new node does match:

<shadow-host>
#shadow-root
<div></div>
<div part="redpart">incorrectly black</div>
<span part="redpart">correctly red</span>

Simple repro: https://jsbin.com/pohakiveqo/1/edit?html,output

More complex repro that also uses exportparts: https://jsbin.com/yuyigopoyo/1/edit?html,output

Note the above two jsbin repros look correct in Chrome.

Firefox Nightly 73.0a1 (2019-12-18) (64-bit)
layout.css.shadow-parts.enabled = true
Linux 5.2.17-1

Actual results:

Only when a part node is the first of its type in its shadow root does it seem to receive style from a ::part rule.

Expected results:

A ::part rule should apply to all nodes in the shadow root of a matching shadow host without regard to sibling structure.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Could you attach a test-case that reproduces the issue?

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Ah, sorry, missed the jsbin link.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Simplified test-case.

The fact that dynamically adding and removing the attribute works makes it sound like somehow we don't set the "has part attr" bit correctly... Weird, will take a look.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1604989 - Do not incorrectly share style across elements with different part names. r=#style

Do the same we do for classes for now. We could be more precise and achieve a
bit more sharing with some more effort (left a comment there), but it seems
unlikely to matter in practice (and if we did that, we'd probably want to do the
same for classes).

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1907a23a8d6e
Do not incorrectly share style across elements with different part names. r=nordzilla

Comment 6

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/20972 for changes under testing/web-platform/tests

Comment 7

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 8

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/1907a23a8d6e

Comment 9

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 10

[Alexander Marks]

It looks like the patch for this bug wasn't included in Firefox 72, which was just released and has layout.css.shadow-parts.enabled true by default. Shadow parts are not very usable with this bug present. Will there be an off-cycle release before 73 that includes this patch?

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9118200 [details]
Bug 1604989 - Do not incorrectly share style across elements with different part names. r=#style

Beta/Release Uplift Approval Request

• User impact if declined: New feature shipped in 72 has a hard-to-workaround bug. Sites using it may behave erratically.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: comment 0 / test-case attached to the bug
• List of other uplifts needed: none
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Share less styles to avoid correctness issues.
• String changes made/needed: none

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Ugh, looks like I forgot to request uplift, sorry for that :(

Comment 13

[Emilio Cobos Álvarez (:emilio)]

To be clear (for release managers): Not sure this is dot-release worthy, so feel free to approval-, but then we should probably disable the feature on release or something.

Comment 14

[Emilio Cobos Álvarez (:emilio)]

(In reply to Alexander Marks from comment #10)

It looks like the patch for this bug wasn't included in Firefox 72, which was just released and has layout.css.shadow-parts.enabled true by default. Shadow parts are not very usable with this bug present. Will there be an off-cycle release before 73 that includes this patch?

Adding a [part] selector to the shadow root can work as a workaround, fwiw.

Comment 15

[Cristian Fogel [:cfogel]]

Confirmed issue with 72.0 on Windows 10 using the test cases.
Fix verified with 73.0b2 on Windows 10, macOS 10.15, Ubuntu 18.04.

Waiting for the decision regarding the dot-release worthyness with updating the other flags.

Comment 17

[Justin Fagnani]

With Edge release ::part() support this wednesday, it would be really great if we had it working across three browsers as soon as possible.

Comment 18

[Julien Cristau [:jcristau]]

Comment on attachment 9118200 [details]
Bug 1604989 - Do not incorrectly share style across elements with different part names. r=#style

fix for css shadow parts (new in 72); approved for 72.0.2

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/f77c2fbe34a8

Comment 20

[Cristian Fogel [:cfogel]]

Verified with 72.0.2 - Windows 10, macOS 10.15, Ubuntu 19.04.