Summary: | REGRESSION(r259480): Two new failing i18n tests | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Michael Catanzaro <mcatanzaro> | ||||||||||
Component: | JavaScriptCore | Assignee: | Ross Kirsling <ross.kirsling> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | clopez, darin, ews-watchlist, keith_miller, mark.lam, mcatanzaro, msaboff, ross.kirsling, saam, tpopela, tzagallo, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | Linux | ||||||||||||
Attachments: |
|
Description
Michael Catanzaro
2020-04-16 07:00:11 PDT
Ah, this test was probably failing for Linux before, but it was easier to set more granular expectations on the LayoutTests side. I think this is a code bug -- the spec says the default time zone needs to be canonicalized too but we're only performing step 3 (https://tc39.es/ecma402/#sec-canonicalizetimezonename) for the non-default path. Will fix. *** Bug 210604 has been marked as a duplicate of this bug. *** Created attachment 396676 [details]
Patch
Comment on attachment 396676 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=396676&action=review > Source/JavaScriptCore/runtime/IntlDateTimeFormat.cpp:134 > + if (canonical.isNull() || canonical == "Etc/UTC"_s || canonical == "Etc/GMT"_s) > + canonical = "UTC"_s; Nicer if we found a way to share code with the below instead of repeating twice. Created attachment 396685 [details]
Patch for landing
Thanks Ross! Comment on attachment 396685 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=396685&action=review > Source/JavaScriptCore/runtime/IntlDateTimeFormat.cpp:107 > +static ALWAYS_INLINE bool isUTCEquivalent(StringView timeZone) I think ALWAYS_INLINE is overkill here. Not incorrect, but not necessary. > Source/JavaScriptCore/runtime/IntlDateTimeFormat.cpp:109 > + return timeZone == "Etc/UTC"_s || timeZone == "Etc/GMT"_s; Using _s here makes the code a tiny bit larger and a tiny bit slower. I suggest just leaving it off. Alternatively we could overload operator== to take ASCIILiteral, but I don’t really want to do that. Created attachment 396736 [details]
Patch for landing
ChangeLog entry in JSTests/ChangeLog contains OOPS!. Created attachment 396738 [details]
Patch for landing
Committed r260237: <https://trac.webkit.org/changeset/260237> All reviewed patches have been landed. Closing bug and clearing flags on attachment 396738 [details]. |