Bug 236631 – Change the canonical unit for time category from ms to s.

• Component: CSS
• Status: RESOLVED FIXED

[Joonghun Park]

According to the spec, https://drafts.csswg.org/css-values-4/#time, the canonical unit for <time> is Seconds.

[Joonghun Park]

Created attachment 451990 [details]
Patch

[Joonghun Park]

Created attachment 451999 [details]
Patch

[Darin Adler]

Comment on attachment 451999 [details]
Patch

I am really surprised that this behavior is not tested by any WPT test. Assuming this can affect browser interoperability, we need to get something into WPT. Or maybe this only affects the legacy CSS object model and that is not well tested in WPT?

If this does only affect the legacy CSS object model then I wonder what the odds are that this breaks some existing content accidentally relying on this. How does this behave in other web browsers?

[Joonghun Park]

In case of Blink, I landed a change, https://chromium-review.googlesource.com/c/chromium/src/+/3460189, which has the same change to this patch. So it lowers interoperablity risk to some extent.

And in wpt aspects,
css/css-values/minmax-time-computed.html
css/css-values/minmax-time-serialize.html
should check the behavior(The change in Blink changed the result of the 2 wpt tests from fail to pass as expected.)

After I read your comment, I found that I missed out to change

case CalculationCategory::Time: return CSSUnitType::CSS_MS;

canonicalUnitTypeForCalculationCategory() in CSSCalcCategoryMapping.cpp.

I will apply this change too.

Thank you for your review!

[Joonghun Park]

Created attachment 452189 [details]
Patch

[Joonghun Park]

Created attachment 452190 [details]
Patch

[Joonghun Park]

Created attachment 452194 [details]
Patch

[Joonghun Park]

Created attachment 452254 [details]
Patch

[EWS]

Committed r289997 (247383@main): <https://commits.webkit.org/247383@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 452254 [details].

[Radar WebKit Bug Importer]

<rdar://problem/89068282>