[css-lists] Algorithm for initial counter value in reversed list should repeat the last increment instead of the 1st one

[Loirooriol]

https://drafts.csswg.org/css-lists/#instantiating-counters

1. Let num be 0.
2. Let first be true.
3. For each element or pseudo-element el that increments or sets the same counter in the same scope:
   1. Let incrementNegated be el’s counter-increment integer value for this counter, multiplied by -1.
   2. If first is true, then add incrementNegated to num and set first to false.
   3. If el sets this counter with counter-set, then add that integer value to num and break this loop.
   4. Add incrementNegated to num.
4. Return num.

Problem 1: the value of the last item is the increment of the 1st one.

Let's consider this basic list:

<ol reversed><!-- 5 -->
  <li style="counter-increment: list-item -1"><!-- 4 --></li>
  <li style="counter-increment: list-item -1"><!-- 3 --></li>
  <li style="counter-increment: list-item -1"><!-- 2 --></li>
  <li style="counter-increment: list-item -1"><!-- 1 --></li>
</ol>

The initial value is 5 which is the sum of the counter-increments, with the 1st one counted twice. The result looks good.
Then, let's change one increment in the middle:

<ol reversed><!-- 6 -->
  <li style="counter-increment: list-item -1"><!-- 5 --></li>
  <li style="counter-increment: list-item -1"><!-- 4 --></li>
  <li style="counter-increment: list-item -2"><!-- 2 --></li>
  <li style="counter-increment: list-item -1"><!-- 1 --></li>
</ol>

Only the preceding elements are affected, as expected. But now let's undo and change the first increment instead:

<ol reversed><!-- 7 -->
  <li style="counter-increment: list-item -2"><!-- 5 --></li>
  <li style="counter-increment: list-item -1"><!-- 4 --></li>
  <li style="counter-increment: list-item -1"><!-- 3 --></li>
  <li style="counter-increment: list-item -1"><!-- 2 --></li>
</ol>

Now all the values changed! That's because, by counting the 1st increment twice, the value of the last item will precisely be the increment of the 1st item (assuming there is no counter-set). This doesn't seem to make much sense.

Problem 2: counter-set to the current value affects preceding values

Let's consider, again,

<ol reversed><!-- 6 -->
  <li style="counter-increment: list-item -1"><!-- 5 --></li>
  <li style="counter-increment: list-item -1"><!-- 4 --></li>
  <li style="counter-increment: list-item -2"><!-- 2 --></li>
  <li style="counter-increment: list-item -1"><!-- 1 --></li>
</ol>

and then add a counter-set: list-item 2 to the item that already had value 2:

<ol reversed><!-- 5 -->
  <li style="counter-increment: list-item -1"><!-- 4 --></li>
  <li style="counter-increment: list-item -1"><!-- 3 --></li>
  <li style="counter-increment: list-item -2; counter-set: list-item 2"><!-- 2 --></li>
  <li style="counter-increment: list-item -1"><!-- 1 --></li>
</ol>

Seems unexpected that when setting the counter to the same value that it would have without counter-set, the values of the preceding items change. Basically, instead of using the counter-increment of the 3rd item as the difference between the 2nd and 3rd items, it's using the counter-increment of the 1st item!

Solution: use the last increment twice, instead of the 1st one

The algorithm should probably be more like:

1. Let num be 0.
2. Let incrementNegated be 0.
3. For each element or pseudo-element el that increments or sets the same counter in the same scope:
   1. Set incrementNegated to el’s counter-increment integer value for this counter, multiplied by -1.
   2. If el sets this counter with counter-set, then add that integer value to num and break this loop.
   3. Add incrementNegated to num.
4. Add incrementNegated to num.
5. Return num.

Or, taking #6738 into account, repeat the last non-zero increment:

1. Let num be 0.
2. Let lastNonZeroIncrementNegated be 0.
3. For each element or pseudo-element el that increments or sets the same counter in the same scope:
   1. Let incrementNegated to el’s counter-increment integer value for this counter, multiplied by -1.
   2. If incrementNegated is not zero, set lastNonZeroIncrementNegated to incrementNegated.
   3. If el sets this counter with counter-set, then add that integer value to num and break this loop.
   4. Add incrementNegated to num.
4. Add lastNonZeroIncrementNegated to num.
5. Return num.

Then we would have:

<ol reversed><!-- 6 -->
  <li style="counter-increment: list-item -2"><!-- 4 --></li>
  <li style="counter-increment: list-item -1"><!-- 3 --></li>
  <li style="counter-increment: list-item -1"><!-- 2 --></li>
  <li style="counter-increment: list-item -1"><!-- 1 --></li>
</ol>

<ol reversed><!-- 6 -->
  <li style="counter-increment: list-item -1"><!-- 5 --></li>
  <li style="counter-increment: list-item -1"><!-- 4 --></li>
  <li style="counter-increment: list-item -2; counter-set: list-item 2"><!-- 2 --></li>
  <li style="counter-increment: list-item -1"><!-- 1 --></li>
</ol>

[MatsPalmgren]

I'm in favor of this change. I think it makes slightly more sense to use the last (non-zero) increment than the first.
The last algo above looks good to me. I tested the last two examples in a local build which implements this, and I get the results as indicated in the comments.

FYI, I'm correcting the existing tests (as if the proposal here is adopted) and adding a ton of new WPTs in:
https://phabricator.services.mozilla.com/D129958

Here's the paths to the new tests in case you want to reference them in the spec:
/css/css-lists/foo-counter-reversed-006a.html
/css/css-lists/foo-counter-reversed-006b.html
/css/css-lists/foo-counter-reversed-006c.html
/css/css-lists/foo-counter-reversed-006d.html
/css/css-lists/foo-counter-reversed-006e.html
/css/css-lists/foo-counter-reversed-007a.html
/css/css-lists/foo-counter-reversed-007b.html
/css/css-lists/foo-counter-reversed-008a.html
/css/css-lists/foo-counter-reversed-008b.html
/css/css-lists/foo-counter-reversed-009a.html
/css/css-lists/foo-counter-reversed-009b.html
/css/css-lists/li-value-reversed-006a.html
/css/css-lists/li-value-reversed-006b.html
/css/css-lists/li-value-reversed-006c.html
/css/css-lists/li-value-reversed-006d.html
/css/css-lists/li-value-reversed-006e.html
/css/css-lists/li-value-reversed-007a.html
/css/css-lists/li-value-reversed-007b.html
/css/css-lists/li-value-reversed-008a.html
/css/css-lists/li-value-reversed-008b.html
/css/css-lists/li-value-reversed-009a.html
/css/css-lists/li-value-reversed-009b.html

I'm also amending these tests to check the syntax:
/css/css-lists/parsing/counter-increment-invalid.html
/css/css-lists/parsing/counter-reset-invalid.html
/css/css-lists/parsing/counter-reset-valid.html
/css/css-lists/parsing/counter-set-invalid.html

I'll wait for a resolution before landing that...

[css-meeting-bot]

The CSS Working Group just discussed Initial Counter Value of reversed list and increments, and agreed to the following:

• RESOLVED: Accept proposal

The full IRC log of that discussion

<fantasai> Topic: Initial Counter Value of reversed list and increments
<fantasai> github: https://github.com//issues/6797
<fantasai> oriol: Define reversed counters
<fantasai> oriol: either specify start explicitly or calculate automatically
<fantasai> oriol: I think algorithm doesn't make sents
<fantasai> oriol: if don't have any counter-set, some of the increments of the items for the counter
<fantasai> oriol: but first increment is counted twice, not once
<fantasai> oriol: and then sum is adjusted by -1 to get start value
<fantasai> oriol: counting the first increment twice, the reason might be otherwise last item will have value of ??
<fantasai> oriol: but in case of -1, we want the last item to get a value of 1
<fantasai> oriol: if all increments are the same
<fantasai> oriol: when they are different, I think we should actually repeat the last increment
<fantasai> oriol: I have some examples in the issue
<fantasai> oriol: if list with all increments -1
<fantasai> oriol: and take one item in middle of list to -2, this only affects preceding items
<fantasai> oriol: but if we change first item, this will affect the value of the last item
<fantasai> oriol: and modify all values in the list
<fantasai> oriol: which seems unexpected
<fantasai> oriol: another issue with counter-set, you have some increment there and the with counter-set
<fantasai> oriol: start without counter-set, and item with value 2
<fantasai> oriol: and then we assign counter-set: 2
<fantasai> oriol: this should have no effect
<fantasai> oriol: probably it's what the author expects
<fantasai> oriol: with current spec this can have an effect
<fantasai> oriol: in issue itself I proposed how to update the spec
<fantasai> oriol: Also variant of spec text taking into account resolution from 6738
<fantasai> oriol: where we decided to skip elements that are hidden
<fantasai> oriol: so only non-zero increments
<fantasai> oriol: Mats said it makes sense
<fantasai> oriol: and he already has an implementation
<Rossen_> q?
<fantasai> oriol: so suggest to take this change
<fantasai> Rossen_: sounds like a reasonable change, any others with an opposing opinion?
<fantasai> [silence]
<TabAtkins> +1 btw
<fantasai> Rossen_: objections?
<fantasai> RESOLVED: Accept proposal
<TabAtkins> (i assumed this was gonna be included in the issue we talked about during the f2f last week)