Bug 227832

Summary: [JSC] Intl.DisplayNames v2
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: JavaScriptCoreAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, mark.lam, msaboff, ross.kirsling, saam, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=229608
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
ross.kirsling: review+
Patch none

Description Yusuke Suzuki 2021-07-09 01:37:43 PDT 
...
Comment 1 Yusuke Suzuki 2021-07-10 17:09:57 PDT 
https://github.com/tc39/intl-displaynames-v2
Comment 2 Yusuke Suzuki 2021-07-10 18:46:58 PDT 
Created attachment 433283 [details]
Patch
Comment 3 Yusuke Suzuki 2021-07-11 00:50:40 PDT 
Created attachment 433286 [details]
Patch
Comment 4 Radar WebKit Bug Importer 2021-07-16 01:38:15 PDT 
<rdar://problem/80674704>
Comment 5 Yusuke Suzuki 2021-08-07 03:20:35 PDT 
Created attachment 435125 [details]
Patch
Comment 6 Yusuke Suzuki 2021-08-20 01:25:34 PDT 
Created attachment 435951 [details]
Patch
Comment 7 Ross Kirsling 2021-08-20 15:43:36 PDT 
Comment on attachment 435951 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=435951&action=review

r=me with comments

> Source/JavaScriptCore/runtime/IntlDisplayNames.cpp:210
> +        // If code does not matches the unicode_language_id production, throw a RangeError exception
> +        if (!isUnicodeLanguageId(code)) {
> +            throwRangeError(globalObject, scope, "argument is not a language id"_s);

Hmm, I feel like this comment doesn't add any information. (Ditto for the region and script ones.)

> Source/JavaScriptCore/runtime/IntlDisplayNames.cpp:295
> +        // Convert code to legacy names since ICU requires legacy names.

Ooh, this could use the helper function I suggested in the Locale Info patch too.
Comment 8 Yusuke Suzuki 2021-08-21 08:08:44 PDT 
Comment on attachment 435951 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=435951&action=review

>> Source/JavaScriptCore/runtime/IntlDisplayNames.cpp:210
>> +            throwRangeError(globalObject, scope, "argument is not a language id"_s);
> 
> Hmm, I feel like this comment doesn't add any information. (Ditto for the region and script ones.)

Removed.

>> Source/JavaScriptCore/runtime/IntlDisplayNames.cpp:295
>> +        // Convert code to legacy names since ICU requires legacy names.
> 
> Ooh, this could use the helper function I suggested in the Locale Info patch too.

Changed.
Comment 9 Yusuke Suzuki 2021-08-21 08:21:21 PDT 
Created attachment 436073 [details]
Patch
Comment 10 EWS 2021-08-21 09:17:08 PDT 
Committed r281375 (240790@main): <https://commits.webkit.org/240790@main>

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