Bug 181084 – Special list-item counter starts from an incorrect number for ::before and ::after

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Roman Komarov]

## Steps to reproduce the problem:

1. Go to https://codepen.io/kizu/pen/QaKjmJ
2. Look at the custom counters

Or

1. Create an ordered list, optionally with a `start` attribute.
2. Add custom styles for the list items by using `counter(list-item)`.

## What is the expected behavior?

This counter should start from either the `1` or from the number set by `start` attribute.

## What went wrong?

The number the counters start from are wrong by one.

However, when the `value` attribute is used, the value is correct.

Edge browser renders everything in a correct way.

There is a hacky fix/workaround for this issue:

    ol.fix:not(*:root) > li:first-child:not([value]) {
      counter-increment: list-item 0;
    }

[Darin Adler]

Specification language for this is quite unclear. Nothing seems to indicate that the "list-item" counter should be affected by the start attribute on the <ul> element or the value attribute on the <li> element. Nothing clearly defines what the behavior should be if someone specifies counter-increment or counter-reset on the list item.

The "fix/workaround" is not compatible with the fix I made for WebKit. I could probably make a better fix if I had a more extensive test suite, demonstrating how counter-increment and counter-reset would affect things.

Has the Edge team contributed a test case for this to the CSS working group?

[Darin Adler]

There is some "sample HTML style sheet" that helps demonstrate the behavior they want.

[Darin Adler]

Created attachment 330668 [details]
Patch

[alan]

Comment on attachment 330668 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=330668&action=review

> Source/WebCore/rendering/RenderListItem.cpp:203
> +    auto* list = enclosingList(*this);
> +    auto* orderedList = is<HTMLOListElement>(list) ? downcast<HTMLOListElement>(list) : nullptr;
>  
>      // FIXME: This recurses to a possible depth of the length of the list.
> -    // That's not good -- we need to change this to an iterative algorithm.
> -    if (RenderListItem* previousItem = previousListItem(list, *this))
> -        return previousItem->value() + valueStep;
> -
> -    if (oListElement)
> -        return oListElement->start();
> +    // That's not good, and we can and should change this to an iterative algorithm.
> +    if (auto* previousItem = previousListItem(list, *this)) {
> +        m_value = previousItem->value() + (orderedList && orderedList->isReversed() ? -1 : 1);

Couldn't you just use isInReversedOrderedList() here?

> Source/WebCore/rendering/style/CounterDirectives.h:37
> +struct CounterDirectives {
> +    std::optional<int> resetValue;
> +    std::optional<int> incrementValue;

This is a really nice cleanup (class -> struct).

[Darin Adler]

Comment on attachment 330668 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=330668&action=review

>> Source/WebCore/rendering/RenderListItem.cpp:203
>> +        m_value = previousItem->value() + (orderedList && orderedList->isReversed() ? -1 : 1);
> 
> Couldn't you just use isInReversedOrderedList() here?

I’ll do that, or something like it, in my next patch, which fixes the "iterative algorithm" comment here too.

[Darin Adler]

Comment on attachment 330668 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=330668&action=review

>>> Source/WebCore/rendering/RenderListItem.cpp:203
>>> +        m_value = previousItem->value() + (orderedList && orderedList->isReversed() ? -1 : 1);
>> 
>> Couldn't you just use isInReversedOrderedList() here?
> 
> I’ll do that, or something like it, in my next patch, which fixes the "iterative algorithm" comment here too.

I could do that, but it would re-do the work of calling enclosingList, which climbs the parent tree, and I don’t want to do that.

[WebKit Commit Bot]

Comment on attachment 330668 [details]
Patch

Clearing flags on attachment: 330668

Committed r226613: <https://trac.webkit.org/changeset/226613>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/36369157>