Closed Bug 1539171 Opened 9 months ago Closed 8 months ago

Automatic list-item increment shouldn't be visible from getComputedStyle().

Categories

(Core :: Layout, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: emilio, Assigned: emilio)

References

()

Details

Attachments

(1 file, 1 obsolete file)

No description provided.

This is per https://drafts.csswg.org/css-lists/#declaring-a-list-item.

I intentionally kept <li value> defined using attribute mapping, I think that's
saner than special-casing it in layout.

The HTML restriction doesn't match any browser.

This matches Edge, though I filed
https://github.com/w3c/csswg-drafts/issues/3766 about the pseudo-element
condition.

Depends on D24935

Hmm, I think you're misinterpreting the spec. I'm assuming you refer to
the "(This has no effect on the values of the counter-* properties.)".
As I read that, it's saying that the incremented final value shouldn't
be reflected in the computed values.

It's not about the "static" 'counter-increment: list-item 1' that all
list items have.
The "Sample Style Sheet For HTML" section says:
"/* counter-increment: list-item; (implied by display: list-item) */"
which I read as "magically set the computed value of 'counter-increment'
to 'list-item 1' when 'display' is 'list-item'", which is exactly what we do.
There's also: "ol[reversed] > li { counter-increment: list-item -1; }".

So, I tend to think that we should have the spec clarified instead if
there's some confusion over this. It's much better that the computed
value reflects the actual increment IMHO.

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

This matches Edge

Do any other UA actually support the built-in 'list-item' counter at all?
I seem to recall that I tested EdgeHTML/WebKit/Blink and all
of them did not. If it's not supported then obviously
'counter-increment: list-item 1' wouldn't show up there
since it'd have no effect.

(In reply to Mats Palmgren (:mats) from comment #4)

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

This matches Edge

Do any other UA actually support the built-in 'list-item' counter at all?
I seem to recall that I tested EdgeHTML/WebKit/Blink and all
of them did not. If it's not supported then obviously
'counter-increment: list-item 1' wouldn't show up there
since it'd have no effect.

I mean that edge treats non-HTML elements as a list item.

Also, this was discussed by the CSSWG in csswg-drafts#3686 and the they resolved on what we're currently doing:
"RESOLVED: go with fantasai proposal (a) for initial value of counter increment"

I interpret "initial value" there to refer to the computed value.
I interpret "counter increment" there as referring to the 'counter-increment' property.
IOW, I'm reading the resolution as: "when the computed value of 'display' is 'list-item' then
'list-item 1' is magically added to the computed value of 'counter-increment' unless
it already contains a value for 'list-item'."

(The test in the second patch doesn't use the counter for anything really.)

Regarding the first patch, I do indeed interpret the spec differently from you, and this was one of the issues that Domenic raised on dev-platform. No browser reflects this in the counter-increment computed value (so I tend to agree with Domenic that this is a slight loss in interop), but if you disagree strongly please do file an issue against the spec...

The second patch is exclusively about the is_html_element / IsHTMLElement check. We shouldn't restrict the counter increment to HTML elements, the spec just defines it in terms of display.

Ok, I'll move the second patch to a different bug, it's trivial to write it on top of the existing code anyway.

Note in particular @tabatkins comment:
"the code you write under (A) is identical to what you'd write if list-item was just set up in the UA sheet, or in an author sheet."
which is precisely what we're doing.

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

Ok, I'll move the second patch to a different bug, it's trivial to write it on top of the existing code anyway.

OK, sounds good.

Comment on attachment 9053670 [details]
Bug 1539171 - Follow the list-item definition from the spec a bit more closely. r=mats

Revision D24936 was moved to bug 1539267. Setting attachment 9053670 [details] to obsolete.

Attachment #9053670 - Attachment is obsolete: true
Priority: -- → P3

Need to rebase this, and also fix our implementation based on https://github.com/w3c/csswg-drafts/issues/3766.

Flags: needinfo?(emilio)

Rebased, will change the behavior of ::before / ::after in a separate bug.

Flags: needinfo?(emilio)
Blocks: 1543328
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4ec02d2be99e
Make the list-item increment not visible from the computed style. r=mats
Status: ASSIGNED → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/16485 for changes under testing/web-platform/tests
Upstream PR merged
You need to log in before you can comment on or make changes to this bug.