search

w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.51k stars 669 forks source link

[css-counter-styles-3] system for cjk-earthly-branch and cjk-heavenly-stem #8596

Closed vitorroriz closed 1 year ago

vitorroriz commented 1 year ago

Hi everybody, https://github.com/w3c/csswg-drafts/pull/4570 changed both styles's systems from alphabetic to fixed, however WPT tests like http://wpt.live/css/css-counter-styles/cjk-heavenly-stem/css3-counter-styles-205.html still expect it to behave as alphabetic and blink/gecko/webkit also behave as alphabetic.

Should the WPT related tests be updated and should engines change behavior or is it something still up to debate?

vitorroriz commented 1 year ago

It seems that WPT tests were modified by blink here to match legacy behavior: https://github.com/web-platform-tests/wpt/commit/2f9d75ef9570f1b55842aa15820f855468c69d77

tabatkins commented 1 year ago

Ugh, that was an extremely bad test change. Explicitly testing for spec-violating behavior, while leaving the assert unchanged so it now claims the test is showing something entirely different, and all without any report to the WG asking for a change.

I'll get this fixed.

tabatkins commented 1 year ago

Oh and for context, https://github.com/w3c/csswg-drafts/issues/2096 changed the spec from the legacy alphabetic behavior to the current fixed behavior at the explicitly request of the CLREQ people. (https://www.w3.org/2019/12/06-clreq-minutes.html#x03 is the minutes where they discussed this.)

dbaron commented 1 year ago

It seems that WPT tests were modified by blink here to match legacy behavior:

For what it's worth, I think it looks more accurate to say that that change converted tests from manual tests (which are rarely if ever run) into reftests. I don't see any sign that anything was intentionally changed from one behavior to another, but it's possible the creation of the references was done incorrectly given the existing tests (although I don't see where, given that the references can be internally verified without consulting the tests).

vitorroriz commented 1 year ago

Hi @dbaron I don't oppose to this change and WebKit has adopted the same behavior. I was just referring the commit message that says:

The two counter styles are officially specified as 'fixed', supporting only a fixed range. However, the legacy implementation has been supporting them as alphabetic counter styles. So this patch extends their ranges to match the existing behavior.

tabatkins commented 1 year ago

Yeah, the manual tests clearly expected the spec behavior (as documented by the meta-assert message, which still maintains that the spec is correct and contradicts the refs). The refs were generated off of Chrome's incorrect behavior, and the commit message clearly indicates that this was an intentional change.

(That said, I talked to Xiaocheng and he doesn't remember why he did this. ^_^ The tests just need to be fixed.)

dbaron commented 1 year ago

I'm not disagreeing about whether the tests are currently right or wrong, but I don't see how anything has changed the expectations of these tests since they were originally created in https://github.com/web-platform-tests/wpt/commit/798952374ea5cca5062adf7cef86edd18354826f (at least for -201 and -202 and -205, which I followed the full history of). I think Xiaocheng's change was just creation of references for existing tests that was consistent with what those tests already expected. (The manual tests already had internal references within the tests for manual comparison, they just weren't set up as reftests.)

vitorroriz commented 1 year ago

Maybe another issue would be better for this but, if the system is to be fixed, is the default fallback (decimal) the best option we have? I'm not an expert, so ignore me if I'm totally wrong, but if I had to guess I'd say cjk-decimal would make more sense as a fallback (?).

nt1m commented 1 year ago

@vitorroriz Can you file a new issue for the fallback?

vitorroriz commented 1 year ago

@vitorroriz Can you file a new issue for the fallback?

Sure! https://github.com/w3c/csswg-drafts/issues/8975

css-meeting-bot commented 1 year ago

The CSS Working Group just discussed [css-counter-styles-3] system for cjk-earthly-branch and cjk-heavenly-stem.

The full IRC log of that discussion <emilio> vitorroriz: some years ago the spec changed alphabetic to fixed
<emilio> ... so just wanted to make sure it's fixed indeed
<emilio> TabAtkins: yeah tests were just wrong
<emilio> vitorroriz: I fixed them
<emilio> TabAtkins: great, nothing on the spec needs change