New issue
Advanced search Search tips

Issue 766650 link

Starred by 7 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 657748



Sign in to add a comment

Counters are incorrectly reset by display:contents

Reported by r...@opera.com, Sep 19 2017

Issue description

According to CSS22 [1], an element with display:none does not increment or reset counters.

According to the current draft of the CSS Lists module [2], elements which do not generate boxes do not reset nor increment counters.

Having display:contents reset/increment counters may make sense and it's not clear to me if the spec in CSS Lists module was intentionally written to include display:contents as elements which do not reset/increment. Gecko currently behaves as display:none for display:contents for counters.

What's clearly a bug is that Blink currently forces counters to 0 on display:contents elements.

See attached demo.

[1] https://www.w3.org/TR/CSS22/generate.html#undisplayed-counters
[2] https://drafts.csswg.org/css-lists-3/#counters-without-boxes

 
counter.html
287 bytes View Download
Labels: Update-Monthly
Labels: ApproachableBug
Owner: futhark@chromium.org
Status: Assigned (was: Available)
Status: Started (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6ddbf96264d611b883d034ed674e61031ab4151a

commit 6ddbf96264d611b883d034ed674e61031ab4151a
Author: Rune Lillesveen <futhark@chromium.org>
Date: Thu Nov 16 19:07:06 2017

Style containment applies to display:content elements.

The CSS Containment spec explicitly states that containment has no
effect on elements with display:contents element for paint, layout and
others. Not stated for contain:style.

Check for style containment for counters even when LayoutObject is null.

Note that due to the bug reported in 766650, the added test passes even
without this code change.

Bug:  766650 
Change-Id: I64082f52255d5d810ba803148a418edff5da66c8
Reviewed-on: https://chromium-review.googlesource.com/771830
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517140}
[add] https://crrev.com/6ddbf96264d611b883d034ed674e61031ab4151a/third_party/WebKit/LayoutTests/external/wpt/css-contain/contain-style-counters-ref.html
[add] https://crrev.com/6ddbf96264d611b883d034ed674e61031ab4151a/third_party/WebKit/LayoutTests/external/wpt/css-contain/contain-style-counters.html
[modify] https://crrev.com/6ddbf96264d611b883d034ed674e61031ab4151a/third_party/WebKit/Source/core/layout/LayoutCounter.cpp

Project Member

Comment 6 by bugdroid1@chromium.org, Nov 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7012b129bab4b5693a22ded64ac8e8cc9914c043

commit 7012b129bab4b5693a22ded64ac8e8cc9914c043
Author: Philip Jägenstedt <foolip@chromium.org>
Date: Mon Nov 20 12:42:44 2017

Move css-contain tests into css/ (web-platform-tests)

See https://github.com/w3c/web-platform-tests/issues/7503

The incorrect css/README.md was updated here:
https://github.com/w3c/web-platform-tests/pull/8306

Bug:  766650 
Change-Id: I28c3a0d683b18ec59cfa8d2f65f71296eb521b13
Reviewed-on: https://chromium-review.googlesource.com/778884
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Philip Jägenstedt <foolip@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517802}
[rename] https://crrev.com/7012b129bab4b5693a22ded64ac8e8cc9914c043/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-style-counters-ref.html
[rename] https://crrev.com/7012b129bab4b5693a22ded64ac8e8cc9914c043/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-style-counters.html

Status: Assigned (was: Started)
Status: Started (was: Assigned)
Labels: -Update-Monthly
Project Member

Comment 10 by bugdroid1@chromium.org, Dec 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d857d94a8417ca19e240eff6cec2985feb770ee1

commit d857d94a8417ca19e240eff6cec2985feb770ee1
Author: Rune Lillesveen <futhark@chromium.org>
Date: Mon Dec 11 19:13:14 2017

Handle display:contents while creating counter nodes.

In order to support CSS counters in a display:contents context, we need
to track current element instead of LayoutObject in FindPlaceForCounter
since display:contents do not have LayoutObjects.

Also corrected documentation and variable/method names according to
coding style.

Bug:  766650 
Change-Id: I399511184f3a22a19b6279a6c921787b96b84cef
Reviewed-on: https://chromium-review.googlesource.com/818341
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523158}
[add] https://crrev.com/d857d94a8417ca19e240eff6cec2985feb770ee1/third_party/WebKit/LayoutTests/external/wpt/css/css-lists/counter-7-ref.html
[add] https://crrev.com/d857d94a8417ca19e240eff6cec2985feb770ee1/third_party/WebKit/LayoutTests/external/wpt/css/css-lists/counter-increment-inside-display-contents.html
[add] https://crrev.com/d857d94a8417ca19e240eff6cec2985feb770ee1/third_party/WebKit/LayoutTests/external/wpt/css/css-lists/counter-reset-increment-display-contents.html
[add] https://crrev.com/d857d94a8417ca19e240eff6cec2985feb770ee1/third_party/WebKit/LayoutTests/external/wpt/css/css-lists/counter-reset-increment-display-none.html
[add] https://crrev.com/d857d94a8417ca19e240eff6cec2985feb770ee1/third_party/WebKit/LayoutTests/external/wpt/css/css-lists/counter-reset-inside-display-contents.html
[modify] https://crrev.com/d857d94a8417ca19e240eff6cec2985feb770ee1/third_party/WebKit/Source/core/layout/LayoutCounter.cpp

Status: Fixed (was: Started)

Sign in to add a comment