Closed Bug 1505489 (shadow-parts) Opened Last year Closed 5 months ago

Implement CSS Shadow Parts (and enable it for chrome callers)

Categories

(Core :: CSS Parsing and Computation, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox69 --- fixed

People

(Reporter: bgrins, Assigned: emilio)

References

(Depends on 1 open bug, Blocks 3 open bugs, )

Details

(Keywords: dev-doc-needed, Whiteboard: [layout:backlog:2019q3:69])

Attachments

(11 files)

36 bytes, text/x-github-pull-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
52 bytes, text/x-github-pull-request
Details | Review
One of the difficulties about migrating from XBL->Shadow DOM is that we can't style anonymous content from document sheets in the latter, and we do this extensively in the browser chrome. This spec appears to allow us to do so.
Component: DOM → CSS Parsing and Computation
Flags: needinfo?(emilio)
Priority: -- → P3
Blocks: 1397876
See Also: → 1531870
Depends on: 1543808
Depends on: 1545425
Alias: shadow-parts
Flags: needinfo?(emilio)
Summary: Implement CSS Shadow Parts for chrome callers → Implement CSS Shadow Parts (and enable it for chrome callers)
Depends on: 1545430
Duplicate of this bug: 1551113

I've created a project with a demo that looks great in Chrome: https://stevenvachon.github.io/scrolling-menu

Whiteboard: [layout:backlog:2019q3:69]
OS: Unspecified → All
Hardware: Unspecified → All
Depends on: 1554497
Assignee: nobody → emilio

I need this to make style invalidation work with Shadow Parts in a way that's
fast. If something in the ancestor shadow root or any of its ancestor changes,
that make a ::part() selector change, I don't want to walk the whole shadow tree
over and over in order to find the parts that I need to restyle.

Unfortunately we cannot just use the mutation observer setup from ShadowRoot
since, unlike for slotted elements, there's no restriction of where a part can
appear in the shadow tree.

That means that the regular nsIMutationObserver notifications don't quite cut
it, since we'd need notified only of subtree roots and we'd need to tree-walk
around in order to figure out if we have any new part.

I thought that I was going to be able to share more code with other bits if I
moved them away from nsIMutationObserver, thus bug 1554498, but in the end it
was not the case, so here's the "without bug 1554498" version of the patch.

The patch on top of that bug (that as I mention in the commit message I'm
ambivalent about whether we should land or not) should be pretty similar either
way.

Still does nothing, since we still do not collect part rules, but this is all
the plumbing that should allow us to invalidate parts when attributes or state
change on their ancestors.

Depends on D32641

I think that, given ::part() right now (without forwarding) cannot affect
descendants (and early pseudo-elements are handled as part of the normal element
restyling process), it is not worth the effort to add more complex invalidation.

But we can always re-evaluate.

Depends on D32642

This uses the bit added for tracking part attributes in order to avoid doing
wasted work.

Depends on D32643

This grows the selector struct, but only in 32-bit, since in 64-bit we take
space from the alignment padding that we're paying due to having the size of the
slice as a word.

Depends on D32644

Unlike for :host() or ::slotted(), or regular rules, we don't need a whole
SelectorMap<>, so gotta make the code a bit more generic.

Depends on D32645

I still haven't implemented each_part(), so this will do nothing yet.

The cascade order stuff is fishy, I know, and I'll fix in a followup if it's
fine with you. I moved the sorting of the rules to rule_collector, since it
seemed to me it was better that way that duplicating the code, and those
SelectorMap functions only have a single caller anyway.

Depends on D32646

This should make all the pieces come together.

Note that we don't need to look at the snapshot for ::part() for now (other than
when selector-matching normally) because I decided to just restyle the element
for now when the part attribute changes.

::part() can't affect descendants anyway (as long as we don't do the forwarding
stuff), and eager pseudo-elements are handled during the normal element restyle,
so it seems to me that adding all the complexity that we have for classes to
part may not be worth it at least yet.

Depends on D32647

I want to enable in Nightly to evaluate (in the medium term) shipping it without
the part forwarding, once the cascade order and importance issues are fixed, and
that we pass all the tests that don't involve forwarding.

That is, I want to monitor whether having ::part() causes compat issues or not.

Depends on D32648

Attachment #9067664 - Attachment description: Bug 1505489 - Add a pref and enable Shadow Parts in Nightly for chrome stylesheets. r=heycam → Bug 1505489 - Add a pref and enable Shadow Parts in Nightly and for chrome stylesheets. r=heycam
Attachment #9067664 - Attachment description: Bug 1505489 - Add a pref and enable Shadow Parts in Nightly and for chrome stylesheets. r=heycam → Bug 1505489 - Add a pref and enable Shadow Parts in Nightly for chrome stylesheets. r=heycam
Attachment #9067664 - Attachment description: Bug 1505489 - Add a pref and enable Shadow Parts in Nightly for chrome stylesheets. r=heycam → Bug 1505489 - Add a pref and enable Shadow Parts in Nightly and for chrome stylesheets. r=heycam
Keywords: dev-doc-needed
Type: enhancement → task

According to MozillaWiki, this is an enhancement.

Type: task → enhancement
Blocks: 1558576
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4ae51e24b0f3
Keep track of elements with part attributes in a shadow tree. r=bzbarsky
https://hg.mozilla.org/integration/autoland/rev/612537692024
Add plumbing code to invalidate shadow parts. r=heycam
https://hg.mozilla.org/integration/autoland/rev/c92e78a509e6
Invalidate the style of the element when the part attribute changes. r=heycam
https://hg.mozilla.org/integration/autoland/rev/6d1ee416a39d
Don't go through all the part names if not there. r=jwatt
https://hg.mozilla.org/integration/autoland/rev/c81cb058e6c1
Add an extra flag to flag ::part() to selectors. r=heycam
https://hg.mozilla.org/integration/autoland/rev/559549b1004d
Collect ::part() rules during CascadeData rebuilds. r=heycam
https://hg.mozilla.org/integration/autoland/rev/a22e7f4bb90f
Add code to make part rules affect the style of the elements. r=heycam
https://hg.mozilla.org/integration/autoland/rev/39daaeb848f4
Implement GeckoElement::each_part. r=heycam
https://hg.mozilla.org/integration/autoland/rev/c47966c0cbde
Add a pref and enable Shadow Parts in Nightly and for chrome stylesheets. r=heycam
Blocks: 1514926
Blocks: 1560556
Blocks: 1424418

Emilio, sorry for asking here. Let me know if there is a better place to ask questions about this.

Does this only work for the browser stylesheets? I tried to use ::part in userChrome.css but without success. Before I invest too much time, I wanted to ask if using ::part in userChrome.css is supported at all. Thanks!

Flags: needinfo?(emilio)
Depends on: 1575507

So it indeed doesn't work in userChrome.css. The issue is that userChrome.css is a user stylesheet.

We only look at author-origin part rules right now.

Other shadow-dom-ish selectors have the same issues. It is unclear to me what the cascade order should be for that, and whether they should apply as if they appeared in all shadow trees or just in the top-level document. But I'm happy to think a bit through and fix that.

Comments with what would be most useful are welcome, I filed bug 1575507 for this.

Flags: needinfo?(emilio)
Depends on: 1577592

Should custom-button::part(foo)[disabled=true] work? Adding that extra attribute selector at the end makes it not match, and it's not clear to me reading the draft if it should (but since it's exposing only information on the local element itself it seems it could). I'm wanting to do this for the arrowscrollbox Custom Element conversion (bug 1514926). I can work around this if it's not intended to work for some reason but would prefer if it did to make the migration easier.

Flags: needinfo?(emilio)

No it should not. Pseudo-elements are the last part of a selector, generally, modulo other pseudos and some blessed pseudo-classes.

I think the right way to do something like that is having a foo foo-disabled part attribute rather than a disabled attribute on the part or such.

Flags: needinfo?(emilio)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #20)

No it should not. Pseudo-elements are the last part of a selector, generally, modulo other pseudos and some blessed pseudo-classes.

I think the right way to do something like that is having a foo foo-disabled part attribute rather than a disabled attribute on the part or such.

OK makes sense, thanks.

You need to log in before you can comment on or make changes to this bug.