Summary: | [ECMA-402] Extension values should default to true, canonicalize without "-true" | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ross Kirsling <ross.kirsling> | ||||||||
Component: | New Bugs | Assignee: | Ross Kirsling <ross.kirsling> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | darin, ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer, ysuzuki | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Ross Kirsling
2020-04-13 14:38:44 PDT
Created attachment 396338 [details]
Patch
Created attachment 396345 [details]
Patch
Comment on attachment 396345 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=396345&action=review > JSTests/stress/intl-collator.js:85 > -shouldBe(testCollator(Intl.Collator('en-u-kn'), [{locale: 'en', numeric: true}]), true); > -shouldBe(testCollator(Intl.Collator('en-u-kn-true'), [{locale: 'en-u-kn-true', numeric: true}]), true); > +shouldBe(testCollator(Intl.Collator('en-u-kn'), [{locale: 'en-u-kn', numeric: true}]), true); > +shouldBe(testCollator(Intl.Collator('en-u-kn-true'), [{locale: 'en-u-kn', numeric: true}]), true); These two test cases are basically the change in a nutshell. Created attachment 396564 [details]
Patch
Make ChangeLog clearer, fix string literal
Comment on attachment 396564 [details]
Patch
r=me
Committed r260151: <https://trac.webkit.org/changeset/260151> All reviewed patches have been landed. Closing bug and clearing flags on attachment 396564 [details]. Comment on attachment 396564 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=396564&action=review > Source/JavaScriptCore/runtime/IntlObject.cpp:539 > + if (lowercase != "true"_s) { There is no benefit of using _s here, but it also does no harm. Generates exactly the same code, I believe. I suppose the rules for when ASCIILiteral is helpful are too confusing. > Source/JavaScriptCore/runtime/IntlObject.cpp:541 > + extension.append('-'); > + extension.append(lowercase); This one-liner is more efficient than two append calls. extension = makeString(extension, '-', lowercase); (In reply to Darin Adler from comment #8) > Comment on attachment 396564 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=396564&action=review > > > Source/JavaScriptCore/runtime/IntlObject.cpp:539 > > + if (lowercase != "true"_s) { > > There is no benefit of using _s here, but it also does no harm. Generates > exactly the same code, I believe. I suppose the rules for when ASCIILiteral > is helpful are too confusing. This one was Yusuke's suggestion. :) > > Source/JavaScriptCore/runtime/IntlObject.cpp:541 > > + extension.append('-'); > > + extension.append(lowercase); > > This one-liner is more efficient than two append calls. > > extension = makeString(extension, '-', lowercase); Oh, that's good to know. I wasn't sure whether `foo = makeString(foo, ...)` was a good pattern or not. Will update. (In reply to Ross Kirsling from comment #9) > Oh, that's good to know. I wasn't sure whether `foo = makeString(foo, ...)` > was a good pattern or not. Will update. Committed r260161: <https://trac.webkit.org/changeset/260161> |