Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[css-lists] CSS counter scope/inheritance is incompatible with HTML ordinals #5477

Open
MatsPalmgren opened this issue Aug 27, 2020 · 18 comments

Comments

@MatsPalmgren
Copy link

The rules for counter scope/inheritance in CSS Lists is incompatible with HTML <ol><ul><li> ordinals and is thus not web-compatible.
https://drafts.csswg.org/css-lists/#nested-counters
https://drafts.csswg.org/css-lists/#inheriting-counters

Gecko has implemented HTML <ol><ul><li> ordinals using the built-in list-item counter as per the CSS Lists spec. However, we've got many reports that this breaks existing web content to the extent that we need to ship a (non spec-compliant) fix.
Consider this simple HTML example:

<style>
  li::before { content: "(list-item:" counters(list-item,'.') ")"; }
</style>
<ol>
  <li></li>
  <li></li>
  <ol>
    <li></li>
  </ol>
  <li></li>
</ol>

According to CSS Lists, the correct rendering is:
image
This is clearly not how HTML lists should work - the last item should have the ordinal 3.
Granted, the above HTML is invalid - <ol><ul> aren't allowed as a direct child of <ol><ul>, but this kind of bogus markup is rather common on the web unfortunately. It's also the DOM that execCommand('indent') produce (sigh), which affects HTML editors. So telling authors to fix their markup isn't a tenable solution.

The alternative fixes we considered were:

  1. change CSS Lists to make all CSS counters behave differently for this case
    Con: breaks existing (author-specified) nested CSS counters of the same name
  2. change the behavior for the built-in list-item counter only
    Con: makes the built-in list-item and other CSS counters behave differently which is a more complicated model to implement, specify, and teach to authors
  3. extend CSS Lists to allow either behavior
    (e.g. something like ol, ul, menu { counter-reset: strict-scope(list-item); } in the UA sheet)
    Con: a more complicated model to implement, specify, and teach to authors

We concluded that 1 is the least bad solution, so we propose to include a new step in finding the counter scope that searches an element's ancestors first, before falling back to the current steps in the spec that searches previous siblings in reverse DOM order.
This also makes author-defined counters behave more sensible - consider the analogous example:

<style>
  list, item { display: block; }
  list { counter-reset: foo; margin-left: 40px; }
  item { counter-increment: foo; }
  item::before { content: counters(foo,'.'); }
</style>
<list>
  <item></item>
  <item></item>
  <list>
    <item></item>
  </list>
  <item></item>
</list>

which currently renders as:
image
to instead render as:
image
which makes more sense IMO.

CC @fantasai @emilio

@faceless2
Copy link

faceless2 commented Aug 27, 2020

Slightly nervous about this one. Can I propose a fourth option which might be lower impact?

  1. Change the counter rules so that counter-reset is applied after counter-increment and counter-set (it's currently applied before). Then you can cater for the invalid markup with this in a UA sheet.
ol > ol { counter-increment: list-item; counter-reset: list-item }

The outer-scoped list-item will be incremented, then a new scope is created with the counter value of 1.

The only impact that would have is on elements that have both a counter-reset and counter-increment - a pointless combination as the rules are currently defined (and also one that can be searched for to see how often its used). It feels like this approach might have less unintended side-effects (and be easier to explain too).

@emilio emilio added the css-lists-3 Current Work label Aug 27, 2020
moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 27, 2020
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Aug 28, 2020
moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 28, 2020
@Loirooriol
Copy link
Contributor

The outer-scoped list-item will be incremented, then a new scope is created with the counter value of 1.

@faceless2 If counter-reset: list-item is applied afterwards, then the inner scope will have value 0. And if counter inheritance prefers the previous sibling over the parent, the next element will still get the value from the inner scope? I don't get how your proposal solves the problem.

@faceless2
Copy link

I've just worked through it again. I'm wrong and you're correct. Scratch that idea, sorry!

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-lists] CSS counter scope/inheritance is incompatible with HTML ordinals, and agreed to the following:

  • RESOLVED: change CSS Lists to make all CSS counters behave differently for this case
The full IRC log of that discussion <emilio> Topic: [css-lists] CSS counter scope/inheritance is incompatible with HTML ordinals
<emilio> github: https://github.com//issues/5477
<emilio> TabAtkins: when I was writing the counter rules I was working from css 2.1 and my testing
<emilio> ... I knew HTML <ol> didn't match CSS counters in invalid markup
<emilio> ... Gecko tried to switch to counters for CSS lists
<emilio> ... and found out that they were not web compatible
<emilio> ... see the example in the issue with a nested <ol> directly under another
<emilio> ... html renders that as you'd probably expect
<emilio> ... that's also what editors generate for some reason
<emilio> ... In order to fix it we can specify that either html is magic
<emilio> ... that the list-item counter is magic
<emilio> ... or switch counter behavior
<emilio> ... mats is going for the later
<Rossen_> q
<emilio> ... and is proposing to do an ancestor-only check and only if that fails we look for a previous sibling counter
<emilio> ... that's probably fine and would still work with the main sibling use case which is naming headers
<emilio> ... so I'm ok with making that change
<emilio> Rossen_: you're referring to the third approach in the comment?
<emilio> TabAtkins: no, number 1
<dbaron> seems like a reasonable way to fix this without breaking use of counter numbering for headers
<emilio> fantasai: if you look at the example and replace list with section and item with h1, it will end up giving a better result
<emilio> ... I'm in favor of this change
<Rossen_> ack fantasai
<emilio> ... if we can pull it of
<emilio> off
<emilio> q+
<astearns> and you’re OK with this, faceless2?
<fantasai> emilio: We actually have made this change, unsure if released yet
<fantasai> emilio: but haven't heard of any breakage
<fantasai> Rossen_: cool
<faceless2> yes. everything I had to say on the topic before was wrong :-)
<emilio> Rossen_: objections?
<fantasai> strong +1 given emilio's report :)
<emilio> RESOLVED: change CSS Lists to make all CSS counters behave differently for this case
<emilio> [... discussion about republishing CSS2 ...]
<oriol> The change is in Firefox 82, which seems it was released on 2020-10-20 (but I still don't have it)

@tabatkins
Copy link
Member

All right, edits are in; did some editorial cleanup of the algorithm while I was in there, as the structure of the algo didn't make much sense any more.

@MatsPalmgren Mind reviewing to make sure we've caught the right nuances?

@fantasai fantasai added this to Awaiting Commenter / Reviewer in Tab and fantasai Await Nov 9, 2020
@tabatkins
Copy link
Member

Fun fact: I raised this (on the mailing list) back in 2016! https://lists.w3.org/Archives/Public/www-style/2016Apr/0364.html

@triple-underscore
Copy link

The updated inherit counters algorithm by this commit would not produce correct result in certain cases, because it makes elements not inherit a nested counter instantiated by any preceding sibling, due to step 3 of the algorithm:

Let sibling counters be the CSS counters set of element’s preceding sibling (if it has one), or an empty CSS counters set otherwise.
For each counter of sibling counters, if element counters does not already contain a counter with the same name, append a copy of counter to element counters.

So, for the following case (no increment), for example:

div::before {
    content: "(" counters(foo,'.') ")";
}

<div style="counter-reset: foo">
    <div style="counter-reset: foo"></div>
    <div></div>
</div>

According to the algorithm, If my interpretation is correct, the second inner div will only inherit it's parent's counters named foo, but ignores the same named nested counter instantiated by the first inner div, and it would produce:

(0)
  (0.0)
  (0)

But browsers renders this as:

(0)
  (0.0)
  (0.0)

@Loirooriol
Copy link
Contributor

@triple-underscore That was precisely the point of the change. Sure, it's not backwards compatible, but hopefully it will be web compatible. Though it seems that Firefox already got a complaint: https://bugzilla.mozilla.org/show_bug.cgi?id=1672569

@triple-underscore
Copy link

@Loirooriol

not backwards compatible, but hopefully it will be web compatible.

Ah, that is what I wanted to know.

Another way to make both backwards compatible and web compatible might be special casing for such mis-parented <ol> mentioned in the first comment to have wrapped with “anonymous list item” for the purpose of counters (effectively, the same as option 3 in the first comment), since the author's intention is clear to do so in such cases.

(Or even more, make indeed generate a list item box for such “anonymous list item” for layout purpose, similar to generate anonymous internal table boxes for table layout; of course, it might introduce another compatibility issues.)

@fantasai fantasai removed this from Awaiting Commenter / Reviewer in Tab and fantasai Await Dec 10, 2020
@Loirooriol
Copy link
Contributor

@MatsPalmgren Can you clarify why Firefox still renders

<style>
  list, item { display: block; }
  list { counter-set: foo; margin-left: 40px; }
  list list { counter-reset: foo; }
  item { counter-increment: foo; }
  item::before { content: counters(foo,'.'); }
</style>
<list>
  <item></item>
  <item></item>
  <list>
    <item></item>
  </list>
  <item></item>
</list>

like this
image

The top-level <list> instantiates the foo counter, so why does the last <item> fall back to inheriting from the previous sibling? Is this a bug or intentional?

@MatsPalmgren
Copy link
Author

@Loirooriol That's intentional. We only prefer the ancestor's scope when it is an explicit counter-reset scope, not an implicit scope created from counter-set/increment as in your example (the outer <list> only has a counter-set, not counter-reset). The reason is that it's the minimal change for making CSS counters compatible with HTML ordinals.

I guess we could include implicit scopes too for consistency perhaps, but that also risks breaking existing content that depends on that behavior. But yeah, it looks like the spec currently doesn't make that distinction, so I guess it's technically a bug in Gecko. But the intent of our proposal was to only include explicit counter-reset scopes, since that was the minimal required change for HTML lists.

I don't feel strongly either way, but I haven't looked at the details of the fallout that change would create... (I can look into that after I'm back from my vacation in case people want to go that route - filed a bug for now).

@Loirooriol
Copy link
Contributor

@MatsPalmgren Some sites that were broken by the change switched to counter-set as a workaround. But implementing the updated spec would break them again. Like https://digital.library.upenn.edu/women/stylesheets/book-2020.css

/* create a page counter for the book */
/* previously used counter-reset in this call */
BODY {
 counter-set: page-counter; 
}

It doesn't seem good to do different things for counter-set and counter-reset when both instantiate a counter, though.

This is getting quite messy. I'm starting to think that a new syntax would have been better (like the recently introduced reversed()).

@Loirooriol
Copy link
Contributor

Loirooriol commented Sep 15, 2023

Agenda+ to revert the previous resolution, which broke websites and didn't even match the behavior implemented by Firefox, and instead use narrow counter scopes from #9076

@tabatkins
Copy link
Member

Agreed, my edits do solve the problematic markup given in the OP, but don't solve the closely-related (and totally valid, unlike this issue's) markup given in #9076. So I support reverting and instead introducing a narrow-scoped counter, and using that in HTML's UA stylesheet.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-lists] CSS counter scope/inheritance is incompatible with HTML ordinals, and agreed to the following:

  • RESOLVED: Revert the previous resolution and the associated edits
  • RESOLVED: The working group is interested in defining an opt-in narrow scope for counter resets. Exact syntax tbd
The full IRC log of that discussion <dael> TabAtkins: A little while ago...several years...we talked about what changes to make to the counter scoping algo to make it workable as a mech to explain html lists
<dael> TabAtkins: html lists have weird behaviors for legacy reasons. I tried to write a fix that fixed what's in the thread but there's close behavior it did not solve. A related change that would fix both is define a counter has an associated scope, wide or narro. Default is wide where it's visible to children and following siblings until scope of same name is declared. Narrow is only visible to children
<dael> TabAtkins: That matched how counters. You have an ol and outside of that you have an li, today b/c wide scoping the li sees the counter scope from the ol and will increment that.
<dael> TabAtkins: There's some reasonable cases that are less intuitive due to wide.
<dael> TabAtkins: Ability to declare scope to be narrow fixes this properly. It will be somewhat magic same way list item counter is. Will be slightly more reasonable for authors where they intent it to number a list among children
<dael> TabAtkins: Concept also carried to when thinking about toggles. Having wide and narrow was necessary there. I think it's proved out that is useful.
<dael> TabAtkins: Precise mechanics I have not figured out. happy to take a resolution to figure out
<dael> florian: Wide is default and narrow is opt-in to match html?
<dael> TabAtkins: Correct
<dael> astearns: Other questions or opinions?
<dael> astearns: First thing would be revert previous?
<dael> astearns: Prop: Revert the previous resolution and the associated edits
<dael> astearns: Obj?
<dael> RESOLVED: Revert the previous resolution and the associated edits
<TabAtkins> valid markup that gets weird currently is in https://github.com//issues/9076
<dael> astearns: Prop: The working group is interested in defining an opt-in narrow scope for counter resets. Exact syntax tbd
<dael> TabAtkins: I expect it'll work same as reverse counter specs, but I haven't dug in enough to be certain
<dael> astearns: Obj?
<dael> florian: Question- you mentioned html has weird edge case. narrow scope seems too clean to cover. Do we end up matching html or is there a bajillion corner cases where we're different?
<dael> TabAtkins: I think this brings us all the way or very very close enough that browsers could call it good. I know wide behavior is web breaking if applied to html counters
<dael> florian: We'll discover it soon enough. We create narrow and if it doesn't work people will tell us
<dael> TabAtkins: I think remaining weird was list item counter and we resolved how to handle that. Where if you don't declare on an ol it happens. I'm not completely loaded on this and it's complex
<dael> RESOLVED: The working group is interested in defining an opt-in narrow scope for counter resets. Exact syntax tbd
<dael> TabAtkins: 9076 was also this as well. It was the same case

@Rolf-B
Copy link

Rolf-B commented Jan 31, 2024

Now Firefox follows the spec change, but but not consistently. I declared a nested counter on a span, and it was inherited to a subsequent ::after Element. When I declare the nested counter on a li element, Firefox isolates it, but Chrome inherits it to the next item. So Chrome doesn't respect the Spec change at all, and Firefox does it partially.

Compare Chrome and Firefox in this JSFiddle.

That's really confusing.

@Loirooriol
Copy link
Contributor

The last resolution was to revert the previous spec change. So Chrome is right, and if you want to isolate the nested counter you will need #9076

@josepharhar
Copy link
Contributor

A WPT change regarding this behavior was proposed here: https://chromium-review.googlesource.com/c/chromium/src/+/5661343

The WPTs here are not yet interoperable: https://wpt.fyi/results/html/semantics/grouping-content/the-li-element?label=master&label=experimental&aligned&q=html%2Fsemantics%2Fgrouping-content

Can someone confirm whether the chromium/webkit or firefox rendering of this is correct? I'm not sure based on the thread of reversing the resolution here, and this doesn't seem to match up perfectly to the example in the OP.

<style>
  body { margin-left: 40px }
  li { list-style-type: decimal }
</style>
<li></li>
<li></li>
<li></li>
<li></li>
<div>
  <li></li>
</div>
<div>
  <div>
    <li></li>
  </div>
</div>

Chromium/webkit:
Screenshot 2024-06-28 at 5 50 27 AM

Firefox:
Screenshot 2024-06-28 at 5 50 41 AM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests