1964745 - :not(:has(~ .item > :nth-child(2)) not getting invalidated correctly (was: CSS not applying correctly when element added/removed)

Status: RESOLVED FIXED

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

Description

[Ben Ashton]

Attachment: firefox-bug.html

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:138.0) Gecko/20100101 Firefox/138.0

Steps to reproduce:

1. Use the following CSS to give an "item" a blue background whenever it is not followed by another item that has at least two children:

.item:not(:has(~ .item > :nth-child(2))) {
background: blue;
}

2. Add/remove items with two children

Actual results:

When adding/removing items, the styling of dependent items does not update until some other change is made. Various actions can cause the styling to update like inspecting the element, resizing the browser window (only works sometimes), or adding/removing stylesheets.

Please see the attached minimal reproduction which includes buttons to add/remove an element, and refresh the styles.

Expected results:

When adding/removing items, the styling of dependent items should update immediately

Comment 1

[Ben Ashton]

Forgot to mention, if you uncomment the empty CSS rule in the attached file, that magically fixes the problem.

Comment 2

[Alice0775 White]

Regression window:
https://hg-edge.mozilla.org/integration/autoland/pushloghtml?fromchange=eccda51a6f68b20cfe9af1e94b9336b9856b103e&tochange=02f729ed8438c8611f8352364030fd59e24bd244

Suspect: Bug 1853258

Comment 3

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

Set release status flags based on info from the regressing bug 1853258

:dshin, since you are the author of the regressor, bug 1853258, could you take a look?

For more information, please visit BugBot documentation.

Comment 4

[David Shin[:dshin]]

Ah, the :has optimization path should not be taken if we find any unknown match.

Comment 5

[David Shin[:dshin]]

Attachment: Bug 1964745: Tighten down when relative selector invalidation is "pulled up" for optimization. r=#style

Previous behaviour where we did not do compound matching is incorrect, but the
order invalidations are added bailed us out (Which makes it fragile).
Add a way to skip this "pull-up" behaviour for indexing selectors, since
element's poisition matters.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9a06e070b01f
Tighten down when relative selector invalidation is "pulled up" for optimization. r=firefox-style-system-reviewers,emilio

Comment 7

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

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

Comment 8

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/9a06e070b01f

Comment 9

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

Upstream PR merged by moz-wptsync-bot

Comment 10

[Acasandrei Beatrice (needinfo me)]

Perfherder has detected the following talos performance change. Do you think push 9a06e070b01f1b1f12a028220a940563efb6de75 could have caused it? At the moment some jobs are failing to run and it looks like either this bug or Bug 1966190 could have caused these regressions:

Regressions:

Ratio	Test	Platform	Options	Absolute values (old vs new)
28%	perf_reftest_singletons append-nth-edge-has-pseudo.html	windows11-64-24h2-shippable	e10s fission stylo webrender	4.41 -> 5.63
27%	perf_reftest_singletons append-nth-edge-has-pseudo.html	linux1804-64-shippable-qr	e10s fission stylo webrender	15.14 -> 19.24
27%	perf_reftest_singletons append-nth-edge-has-pseudo.html	linux1804-64-shippable-qr	e10s fission stylo webrender	14.93 -> 18.91
25%	perf_reftest_singletons append-nth-edge-has-pseudo.html	macosx1470-64-shippable	e10s fission stylo webrender	8.40 -> 10.53
22%	perf_reftest_singletons remove-front-has-pseudo.html	linux1804-64-shippable-qr	e10s fission stylo webrender	1.51 -> 1.84
22%	perf_reftest_singletons remove-front-has-pseudo.html	macosx1470-64-shippable	e10s fission stylo webrender	1.12 -> 1.37
18%	perf_reftest_singletons remove-front-has-pseudo.html	windows11-64-24h2-shippable	e10s fission stylo webrender	0.56 -> 0.66

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

Comment 11

[David Shin[:dshin]]

Hmm. I'll have to investigate.

That said, this was an issue of correctness, so it may be something we have to take as a loss.
Further, bug 1875081 and bug 1874066 added these tests as part of bugs that caused delays in orders of seconds - I don't see such delay on today's Nightly, so I don't think it'll be a huge issue.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/c23d3bffa761
https://hg.mozilla.org/releases/mozilla-beta/rev/7b3263ac6138
Revert "Bug 1964745: Tighten down when relative selector invalidation is "pulled up" for optimization. r=firefox-style-system-reviewers,emilio" for causing Bug 1971486