1809505 - Style containment not working well with counters on pseudo-elements

Status: RESOLVED FIXED

(Core :: Layout: Generated Content, Lists, and Counters, defect)

Description

[Oriol Brufau [:Oriol]]

Attachment: style-containment.html

Load attached testcase.

Expected:

[::before=0]
  1. [::before=1][::after=1]
  [::before=0]    <-- Here
    1. [::before=1][::after=1]
  [::after=1]
  2. [::before=2][::after=2]
[::after=2]

Actual:

[::before=0]
  1. [::before=1][::after=1]
  [::before=1]    <-- Here
    1. [::before=1][::after=1]
  [::after=1]
  2. [::before=2][::after=2]
[::after=2]

Also, layout is not stable. if then I run

var div = document.querySelector("div");
div.style.display = "none";
div.offsetLeft;
div.style.display = "";

it becomes

[::before=0]
  1. [::before=1][::after=1]
  [::before=2]    <-- Here
    1. [::before=1][::after=1]
  [::after=1]
  2. [::before=2][::after=2]
[::after=2]

Comment 1

[Oriol Brufau [:Oriol]]

Attachment: style-containment.html

I forgot about https://drafts.csswg.org/css-contain/#containment-style

the counter() and counters() value of the content property is not itself scoped, and can refer to counters established outside of the subtree.

So the initial result is actually good. I'm attaching a better testcase with counters().

[::before=0]
  1. [::before=1][::after=1]
  [::before=1]
    1. [::before=1.1][::after=1.1]
  [::after=1.1]
  2. [::before=2][::after=2]
[::after=2]

But when I run the JS, it wrongly becomes

[::before=0]
  1. [::before=1][::after=1]
  [::before=2]
    1. [::before=2.1][::after=2.1]
  [::after=2.1]
  2. [::before=2][::after=2]
[::after=2]

Comment 2

[Tiaan Louw [:tlouw]]

Reproduced on nightly 110

Comment 3

[Emilio Cobos Álvarez [:emilio]]

Martin, do you have cycles to take a look at this? No big deal if not but it seems it might not be too hard to address.

Comment 4

[Martin Robinson (email:[e-mail])]

Sure. I can take a look at this failure.

Comment 5

[Martin Robinson (email:[e-mail])]

Please accept my apologies for dropping the ball on this issue. I think that I may not have time to tackle it in the short term. :(

Comment 7

[Daniel Holbert [:dholbert]]

emilio, maybe you can pick this up if/when you have cycles? Sounded like in comment 3 like it might be a relatively straightforward fix. (We just got what-seems-to-be-a-dupe filed in bug 1934755, so it seems to be something that folks have stumbled over in the wild in at least that one instance.)

Comment 8

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1809505 - Fix ContainStyleScope::GetPrecedingElementInGenConList. r=mrobinson,#layout

The binary search gives us the first node after the insertion point.

We want the preceding one.

Comment 9

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1809505 - Extract counter list dumping. r=#layout

This was useful for debugging.

Comment 10

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

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

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/453067fa6fa0
Extract counter list dumping. r=jwatt

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/8bca57a930cc
Fix ContainStyleScope::GetPrecedingElementInGenConList. r=jwatt

Comment 13

[amarc]

https://hg.mozilla.org/mozilla-central/rev/8bca57a930cc
https://hg.mozilla.org/mozilla-central/rev/453067fa6fa0

Comment 14

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

Upstream PR merged by moz-wptsync-bot