[css-counter-styles] Additive algorithm shouldn't divide by 0

[Loirooriol]

Not sure if I'm missing something, but consider

<style>
@counter-style foo {
  system: additive;
  additive-symbols: 0 ⚀;
}
</style>
<li value="1" style="list-style: foo inside"></li>

From https://drafts.csswg.org/css-counter-styles/#additive-system

1. If value is initially 0

It's 1, so no-op.

2. While value is greater than 0 and there are elements left in the symbol list:

Fine, value = 1 > 0 and the symbol list contains one additive tuple.

2.1. Pop the first additive tuple from the symbol list. This is the current tuple.

The current tuple is 0 ⚀.

2.2. Append the current tuple’s counter symbol to S floor( value / current tuple’s weight ) times (this may be 0).

floor(1 / 0) = floor(∞) = ∞. So the algorithm is asking to append ⚀ infinite times??

2.3 Decrement value by the current tuple’s weight multiplied by the number of times the current tuple was appended to S in the previous step.

value = 1 - 0*∞ = 1 - NaN = NaN, I guess?

2. While value is greater than 0

NaN is not greater than 0, presumably.

3. If the loop ended because value is 0, return S. Otherwise, the given counter value cannot be represented by this counter style, and must instead be represented by the fallback counter style.

So I guess it may end up being fine with the fallback counter style. But it just seems pretty wrong for the algorithm to have divisions by 0, infinite insertions, etc.

Both Firefox and Chromium stop iterating if the weight is 0 . I think the algorithm should have that check too.

[tabatkins]

Should we just skip over 0-weight entries, or call them invalid? This is an integer, so it's fine for us to define the range as [1,Inf]. I'm inclined to do the latter, unless there's a great reason to do otherwise (and unless there's web compat or impl difficulties, 'we're currently doing the first one' isn't a sufficient reason, imo).

[Loirooriol]

It may make sense to make a lone 0-weight entry invalid. But not in general, I think it's reasonable to be able to specify a custom symbol for zero in:

<style>
@counter-style dice {
  system: additive;
  additive-symbols: 6 ⚅, 5 ⚄, 4 ⚃, 3 ⚂, 2 ⚁, 1 ⚀, 0 ∅;
  suffix: " ";
  range: infinite infinite;
}
</style>
<ol start="-12" style="list-style: dice"></ol>
<script>
for (let i = -12; i <= 12; ++i)
  document.querySelector("ol").append(document.createElement("li"));
</script>

[tabatkins]

...sigh, yes, apologies, I forget how my own spec works. A zero entry is indeed necessary, just not by itself.

Okay, I'll fix the spec, thanks for bearing with me. ^_^

[tabatkins]

All right, rewrote the algo a little for style purposes, and fixed the bug. A zero-weight tuple is now just skipped over. (And I now early-exit when 'value' hits 0, so since a 0-weight tuple must be the last one encountered in a valid 'additive-symbols', if you hit it you're in fallback territory, and now properly fall into that step.)

[Loirooriol]

Thanks, mostly it looks good, but the final assertion may not hold:

<style>
@counter-style foo {
  system: additive;
  additive-symbols: 1 ⚀;
}
</style>
<li value="0" style="list-style: foo inside"></li>

1. Let value initially be the counter value

It's 0.

2. If value is 0, and there is an additive tuple with a weight of 0, append that tuple’s counter symbol to S and return S.

The condition is false because there is no tuple with a weight of 0. So we start the loop

3.2. If weight is zero, or weight is greater than value, continue.

weight = 1 > 0 = value, so we continue. But there is no other tuple, so we exit the loop.

4. Assertion: value is still non-zero.

Assert failed, the value is zero.

[Loirooriol]

I guess you can remove the "or weight is greater than value" condition. If weight > value, then reps = 0, so S and value are not actually altered anyways. And the algorithm returns at 3.5, avoiding the assert.

[tabatkins]

Fair, I'd hoped it would make it clearer, but it does indeed trip the assert. ^_^

[Loirooriol]

Ah, no, sorry, my suggestion was wrong! Because in the example above, we want to use the fallback style to display zero, instead of using an empty string.

[tabatkins]

Ah, indeed, my brain isn't working great this morning either.

Okay, I'm now just catching the zero-value case right away and either rendering it or triggering fallback. Then the non-zero values get to fall thru to the rest of the algo.

[Loirooriol]

Seems good now, thanks

[Loirooriol]

Actually, maybe it's not crystal clear that the steps are aborted at 2.2?

Consider something like

Otherwise, the given counter value cannot be represented by this counter style.
Return the result of constructing the representation with the fallback counter style.