Extend 'Presentational Hints' from 'LI' (list-item) to 'OL'

[Ahmad Saleem]

Hi Team,

While exploring failures which are not related to 'counter-rest' etc. in following WPT Testcase:

WPT Testcase: https://wpt.fyi/results/html/rendering/non-replaced-elements/lists/lists-styles.html?label=master&label=experimental&aligned&q=safari%3Afail

We fail following tests:

<ol type="none"> - list-style-type	
<ol type="NONE"> - list-style-type	
<ol type="disc"> - list-style-type	
<ol type="DISC"> - list-style-type	
<ol type="circle"> - list-style-type	
<ol type="CIRCLE"> - list-style-type	
etc.

I looked into Web-Spec and came across following: https://html.spec.whatwg.org/#lists & https://drafts.csswg.org/css-lists/#ua-stylesheet

In following, I noticed that (in HTML Spec), we have presentational hints for 'LI' but not for 'OL':

ul[type=none i], li[type=none i] { list-style-type: none; }
ul[type=disc i], li[type=disc i] { list-style-type: disc; }
ul[type=circle i], li[type=circle i] { list-style-type: circle; }
ul[type=square i], li[type=square i] { list-style-type: square; }

___

Now when I looked into 'HTMLOListElement::collectPresentationalHintsForAttribute', I noticed that we don't account for failing test related presentational hints and when I update similar to 'HTMLLIElement'. We do progress all above failing tests.


I just wanted to check whether I should do PR to fix the bug or we wait for 'Web Spec' to reflect this?

CCing - Anne & Tim - for their input. If I came to wrong conclusion, it is still fine, it would be good to learn something from this bug for me.

Thanks!

[Tim Nguyen (:ntim)]

We do have the correct presentational hints here: https://searchfox.org/wubkat/rev/1531d41a4ef64103835343c7ca448db7defee9de/Source/WebCore/html/HTMLUListElement.cpp#59

[Ahmad Saleem]

(In reply to Tim Nguyen (:ntim) from comment #1)
> We do have the correct presentational hints here:
> https://searchfox.org/wubkat/rev/1531d41a4ef64103835343c7ca448db7defee9de/
> Source/WebCore/html/HTMLUListElement.cpp#59

Yes, but we don't have for HTMLOListElement here: https://searchfox.org/wubkat/rev/1531d41a4ef64103835343c7ca448db7defee9de/Source/WebCore/html/HTMLOListElement.cpp#70

and if you see, web-spec don't mention any presentation hints like we have in UL and LI for 'OL' but WPT test expect it.

[Ahmad Saleem]

With this:

void HTMLOListElement::collectPresentationalHintsForAttribute(const QualifiedName& name, const AtomString& value, MutableStyleProperties& style)
{
    if (name == typeAttr) {
        if (value == "a"_s)
            addPropertyToPresentationalHintStyle(style, CSSPropertyListStyleType, CSSValueLowerAlpha);
        else if (value == "A"_s)
            addPropertyToPresentationalHintStyle(style, CSSPropertyListStyleType, CSSValueUpperAlpha);
        else if (value == "i"_s)
            addPropertyToPresentationalHintStyle(style, CSSPropertyListStyleType, CSSValueLowerRoman);
        else if (value == "I"_s)
            addPropertyToPresentationalHintStyle(style, CSSPropertyListStyleType, CSSValueUpperRoman);
        else if (value == "1"_s)
            addPropertyToPresentationalHintStyle(style, CSSPropertyListStyleType, CSSValueDecimal);
    else {
        auto valueLowerCase = value.convertToASCIILowercase();
        if (valueLowerCase == "disc"_s)
            addPropertyToPresentationalHintStyle(style, CSSPropertyListStyleType, CSSValueDisc);
        else if (valueLowerCase == "circle"_s)
            addPropertyToPresentationalHintStyle(style, CSSPropertyListStyleType, CSSValueCircle);
        else if (valueLowerCase == "round"_s)
            addPropertyToPresentationalHintStyle(style, CSSPropertyListStyleType, CSSValueRound);
        else if (valueLowerCase == "square"_s)
            addPropertyToPresentationalHintStyle(style, CSSPropertyListStyleType, CSSValueSquare);
        else if (valueLowerCase == "none"_s)
            addPropertyToPresentationalHintStyle(style, CSSPropertyListStyleType, CSSValueNone);
    } 
    } else
        HTMLElement::collectPresentationalHintsForAttribute(name, value, style);
}

___

We progress WPT test cases.

[Tim Nguyen (:ntim)]

I think this test is actually incorrect and not conformant to the spec. It tests that ol supports the `none` / `disc` / `circle` / `square` list style types, but the spec says otherwise (those types are restricted to `ul` and `li` in the spec).

[Ahmad Saleem]

(In reply to Tim Nguyen (:ntim) from comment #4)
> I think this test is actually incorrect and not conformant to the spec. It
> tests that ol supports the `none` / `disc` / `circle` / `square` list style
> types, but the spec says otherwise (those types are restricted to `ul` and
> `li` in the spec).

Yep:

In test: https://github.com/web-platform-tests/wpt/blob/0caf8c7c02/html/rendering/non-replaced-elements/lists/lists-styles.html#L38

We have:

ol[class=type-a], li[class=type-a] { list-style-type: lower-alpha; }
ol[class=type-A], li[class=type-A] { list-style-type: upper-alpha; }
ol[class=type-i], li[class=type-i] { list-style-type: lower-roman; }
ol[class=type-I], li[class=type-I] { list-style-type: upper-roman; }
ol[type=none i], li[type=none i] { list-style-type: none; }
ol[type=disc i], li[type=disc i] { list-style-type: disc; }
ol[type=circle i], li[type=circle i] { list-style-type: circle; }
ol[type=square i], li[type=square i] { list-style-type: square; }

but web-spec is:

ol[type="1"], li[type="1"] { list-style-type: decimal; }
ol[type=a s], li[type=a s] { list-style-type: lower-alpha; }
ol[type=A s], li[type=A s] { list-style-type: upper-alpha; }
ol[type=i s], li[type=i s] { list-style-type: lower-roman; }
ol[type=I s], li[type=I s] { list-style-type: upper-roman; }
ul[type=none i], li[type=none i] { list-style-type: none; }
ul[type=disc i], li[type=disc i] { list-style-type: disc; }
ul[type=circle i], li[type=circle i] { list-style-type: circle; }
ul[type=square i], li[type=square i] { list-style-type: square; }

__

NOTE: Firefox / Gecko do support 'presentational' hints on 'OL' but WebKit / Safari don't.

[Tim Nguyen (:ntim)]

https://github.com/web-platform-tests/wpt/pull/43463