1738356 - WPT /css/css-counter-styles/japanese-formal/css3-counter-styles-049.html fails?

Status: RESOLVED FIXED

(Core :: Layout: Generated Content, Lists, and Counters, task, P4)

Description

[Mats Palmgren (inactive)]

Attachment: screenshot

The following test is marked as failing in Firefox:
https://wpt.fyi/results/css/css-counter-styles/japanese-formal/css3-counter-styles-049.html

The documentation in the test seems to indicate that we do pass it though:
" test passes only if the left column of the 2nd and 3rd lines is NOT decimal digits and is NOT the same as the right side"
Attached is a screenshot of how it renders for me in Nightly.
So it appears we do pass the test? (since that's not decimal digits, and not the same as the right side)

The explanation goes on:
"Score as Partial if the columns of the 2nd and 3rd lines are the same (ie. fallback was used)"
So it appears that the reference is expecting the Partial result:
http://wpt.live/css/css-counter-styles/japanese-formal/css3-counter-styles-049-ref.html

This seems like a bogus test to me - shouldn't the reference reflect the "full pass" result rather than the "partial pass" result?

I don't know Japanese though, so I have no clue what these characters mean...
Hiro, could you take a look if our result here indeed looks correct?

Comment 1

[Mats Palmgren (inactive)]

Attachment: screenshot of reference (css3-counter-styles-049-ref.html)

For the record, this is what the reference looks like for me.
IIUC, this the "partial pass" result, since the left and right sides are the same on line 2 and 3.

Comment 2

[Mats Palmgren (inactive)]

Here's the spec: https://drafts.csswg.org/css-counter-styles/#limited-japanese
I see that the range there is range: -9999 9999 so perhaps I'm just confused and the intent of this test is to check that we do indeed use the fallback since the second line has the value 10000 and the third line 10001?

Comment 3

[Mats Palmgren (inactive)]

Attachment: wip

FWIW, this makes the test render as the reference.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Yeah, I think your suspicious is totally correct. The test seems to intend to test the outside of the range. (then, the comment in the test looks bogus) But it seems it's originally intend to test the suffix 、 as per https://chromium-review.googlesource.com/c/chromium/src/+/2676274/ .

That said, the range in the spec doesn't make sense to me in the first place. Honestly it's first time to see this 阡 (representing "thousand") in my entire life (I thought at first look it's a Chinese character), whereas I've seen 萬 quite often. I wonder how they decide the range.

Comment 5

[Mats Palmgren (inactive)]

OK, this test is for japanese-formal so perhaps that's why it's using somewhat unusual characters?
There's a japanese-informal test here: http://wpt.live/css/css-counter-styles/japanese-informal/css3-counter-styles-044.html
(which we also fail for the same reason it seems)
Our internal encoding for these counter styles are here:
https://searchfox.org/mozilla-central/rev/a9e0a3f5e5f7cde941d419db967997aaa1f06b0f/layout/style/CounterStyleManager.cpp#206-221
(it looks like we're using the correct codepoints according to the spec)

Anyway, do you think we should just add the range from the spec to our code, as I did in the WIP above, to make us pass these tests?

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Yep, we should add the range as per the spec text. (if there were no spec text, I think our current behavior is preferable)

Comment 7

[Mats Palmgren (inactive)]

Actually, the spec says:

Because opinions differ on how best to represent numbers 10k or greater using the longhand CJK styles, all of the counter styles defined in this section are defined to have a range of -9999 to 9999, but implementations may support a larger range.

https://drafts.csswg.org/css-counter-styles/#complex-cjk

So I guess our current rendering is technically permitted. (It does seem a little odd that we switch from 阡 (in nine thousand) to 萬 (in ten thousand) but I guess that's an effect of going from the enumerated range we have encoded for japanese-formal to the generic code that synthesizes any number (which I assume is shared for all Japanese counter styles).)

If you think our current rendering is preferable, then I guess we could add an alternative reference (I think WPT supports "pass if you match any of these references").

Comment 8

[Mats Palmgren (inactive)]

OK, I didn't see your last answer before adding my reply :-)
so I guess you prefer to keep the code as is and add an alternative reference?

Comment 9

[Hiroyuki Ikezoe (:hiro)]

Oh yeah, good to know! Then, hmm, I don't know how to tweak the test, we don't need to change anything in our implementation.

Comment 10

[Mats Palmgren (inactive)]

Great, I'll try to find how to specify an alternative reference for the test. Thanks for your help!

Comment 11

[Mats Palmgren (inactive)]

Turns out it's trivial to add an alternative reference - just add another <link rel='match' href=...> :
https://web-platform-tests.org/writing-tests/reftests.html

I added alternative reference files for the tests that I think fail due to this issue:
testing/web-platform/tests/css/css-counter-styles/japanese-formal/css3-counter-styles-049.html
testing/web-platform/tests/css/css-counter-styles/japanese-informal/css3-counter-styles-044.html
testing/web-platform/tests/css/css-counter-styles/korean-hangul-formal/css3-counter-styles-054.html
testing/web-platform/tests/css/css-counter-styles/korean-hanja-formal/css3-counter-styles-064.html
testing/web-platform/tests/css/css-counter-styles/korean-hanja-informal/css3-counter-styles-059.html
testing/web-platform/tests/css/css-counter-styles/simp-chinese-formal/css3-counter-styles-078.html
testing/web-platform/tests/css/css-counter-styles/simp-chinese-informal/css3-counter-styles-073.html
testing/web-platform/tests/css/css-counter-styles/trad-chinese-informal/css3-counter-styles-083.html

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6bf8d3a195e1add9bf0852c2ada4d180f1a75ebc

Comment 12

[Mats Palmgren (inactive)]

... and one more:
testing/web-platform/tests/css/css-counter-styles/trad-chinese-formal/css3-counter-styles-088.html

https://treeherder.mozilla.org/#/jobs?repo=try&revision=19ee106a7ff128a05111a168c55a60c5dee4bea1

Comment 13

[Mats Palmgren (inactive)]

BTW, I checked a few of these test in Chrome and they have the same rendering as we do...
except the two Japanese tests, where they use the fallback rendering for some reason,
but the other ones appears to be the same.

Comment 14

[Mats Palmgren (inactive)]

Actually there's a similar note in the spec about Hebrew:
https://drafts.csswg.org/css-counter-styles/#ref-for-hebrew%E2%91%A0

Implementations must implement hebrew at least to the range specified in the @counter-style rule above, but may implement it to a higher range.

and both Firefox and Chrome do that so I'll add an alternative reference for this one too while I'm here...

Comment 15

[Mats Palmgren (inactive)]

Attachment: Bug 1738356 - Update various counter style tests to not assume that a range is applied. r=hiro

In particular, for 'hebrew', the spec says: "Implementations must implement
hebrew at least to the range specified in the @counter-style rule above,
but may implement it to a higher range." at
https://drafts.csswg.org/css-counter-styles/#ref-for-hebrew%E2%91%A0
and for CJK: "Because opinions differ on how best to represent numbers 10k
or greater using the longhand CJK styles, all of the counter styles defined
in this section are defined to have a range of -9999 to 9999, but
implementations may support a larger range." at
https://drafts.csswg.org/css-counter-styles/#complex-cjk

Firefox and Chrome (at least) do implement a larger range than the spec
recommends, so there needs to be an alternative reference for these tests
to make them pass (which is OK since the range isn't normative).

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2be753dcd925
Update various counter style tests to not assume that a range is applied.  r=hiro,TYLin,saschanaz DONTBUILD

Comment 17

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/31455 for changes under testing/web-platform/tests

Comment 18

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/2be753dcd925

Comment 19

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot