1818151 - :nth-child(An+B of [attribute]) and :nth-child(An+B of .class) invalidation

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement)

Description

[Zach Hoffman [:zrhoffman]]

Reftests that should pass:

css/selectors/invalidation/nth-child-of-attr.html
css/selectors/invalidation/nth-child-of-class.html
css/selectors/invalidation/nth-last-child-of-attr.html
css/selectors/invalidation/nth-last-child-of-class.html

Comment 1

[Zach Hoffman [:zrhoffman]]

Attachment: Bug 1818151 - Add plumbing for nth-of invalidation r?#style,#layout

This includes:

• InvalidationProcessor::invalidated_nth_of()
• DependencyInvalidationKind::NthOf
• collecting nth-of invalidations
• TreeStyleInvalidator::invalidate_nth_of()

Notably absent so far is an NthOf InvalidationKind. Although it's not
needed yet, InvalidationKind::NthOf will be necessary for
:nth-child(An+B of sibling) invalidation in bug 1818152 and
:nth-child(An+B of selector list)-as-ancestor invalidation in
bug 1818153.

Comment 2

[Zach Hoffman [:zrhoffman]]

Attachment: Bug 1818151 - Use a separate nth-index cache key if the element is in a snapshot r?#style,#layout

Performing nth-of invalidation will include calling
InvalidationProcessor.check_outer_dependency(..., nth_sibling). For any
siblings whose nth-child index has changed since the snapshot, trying to
use the cached nth-child index will result in an assertion failure when
matches_generic_nth_child() compares the cached index against an
uncached nth_child_index() result.

For that reason, an element should use a different hash in the nth-index
cache if it is part of the snapshot.

Depends on D170988

Depends on D170988

Comment 3

[Zach Hoffman [:zrhoffman]]

Attachment: Bug 1818151 - Implement :nth-child(An+B of [attribute]) and :nth-child(An+B of .class) invalidation r?#style,#layout

We perform NthOf invalidation when a dependency's selector includes
NthOf between the dependency's selector offset and the next combinator.

An alternative would be to perform NthOf invalidation when a
dependency's ancestor's selector contains NthOf. But because the parent
dependency's selector offset is always either 0 or the index of the last
combinator + 1, rather than the specific index of the parent dependency
selector's NthOf component, that approach would not give us additional
information about which NthOf component contains the dependency, in the
case of 2 adjacent NthOf components, without some additional work.

Depends on D170989

Comment 4

[Zach Hoffman [:zrhoffman]]

Attachment: Bug 1818151 - Reiterate over an element's traveral children if nth-of invalidation occurred r?#style,#layout

As of this change, :nth-last-child(An+B of [attribute]) and
:nth-last-child(An+B of .class) invalidation succeeds.

Depends on D170990

Depends on D170990

Comment 5

[Zach Hoffman [:zrhoffman]]

Attachment: Bug 1818151 - Invalidate siblings if NODE_HAS_SLOW_SELECTOR_NTH_OF is set r?#style,#layout

Because nothing sets NODE_HAS_SLOW_SELECTOR_NTH_OF yet, this patch alone
doesn't change any behavior.

Comment 6

[Zach Hoffman [:zrhoffman]]

Attachment: Bug 1818151 - Record attribute dependencies within the selector list of :nth-child(... of <selector list>) r?#style,#layout

There are separate filters for attribute local names, classes, and IDs.

Also, we apply NODE_HAS_SLOW_SELECTOR_NTH_OF to elements matched against
the selector list of :nth-child(... of <selector list>), since doing so
will no longer result in broad over-invalidation.

There are 2 invalidation cases still uncovered:

• Attribute selectors for the class or id attributes. Examples:
  • :nth-child(even of [class^="class-prefix"])
  • nth-child(even of [id^="id-prefix"])
• :nth-child(An+B of :has()) (bug 1818155)

The reftests for :nth-child(... of <selector list>) within a
shadow root still fail because polyfill_declarative_shadow_dom() is
included but not called, which is fixed in the next patch.

Depends on D171935

Comment 7

[Zach Hoffman [:zrhoffman]]

Attachment: Bug 1818151 - Use declarative shadow DOM polyfill for :nth-child(... of <selector>) in shadow root r?#style,#layout

Although the polyfill script was already included, it was not used (and
we still need it, pending bug 1712140), which made these tests still
fail after :nth-child(... of <selector>) invalidation was implemented.

Depends on D171936

Comment 8

[Phabricator Automation]

Comment on attachment 9321745 [details]
Bug 1818151 - Use declarative shadow DOM polyfill for :nth-child(... of <selector>) in shadow root r?#style,#layout

Revision D171937 was moved to bug 1821269. Setting attachment 9321745 [details] to obsolete.

Comment 9

[Zach Hoffman [:zrhoffman]]

https://treeherder.mozilla.org/jobs?repo=try&revision=576cc15f196caee95efea29d5b25922cfd4570ca

Comment 10

[Zach Hoffman [:zrhoffman]]

A couple variables used only for MOZ_ASSERTs needed DebugOnly. Optimized builds look okay now: https://treeherder.mozilla.org/jobs?repo=try&revision=bae766f2b1f9e8be0dad142f763cb314feb135f6

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/660539cf0c3f
Record attribute dependencies within the selector list of :nth-child(... of <selector list>) r=emilio

Comment 12

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

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

Comment 16

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/660539cf0c3f

Comment 17

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

Upstream PR merged by moz-wptsync-bot