Bug 159084

Summary: [Win] Re-enable INTL
Product: WebKit Reporter: Per Arne Vollan <pvollan>
Component: JavaScriptCoreAssignee: Per Arne Vollan <pvollan>
Status: RESOLVED CONFIGURATION CHANGED    
Severity: Normal CC: achristensen, benjamin, bfulgham, commit-queue, ross.kirsling
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch achristensen: review-

Per Arne Vollan
Reported 2016-06-24 02:53:09 PDT
INTL is curretly not enabled on Windows.
Attachments
Patch (8.53 KB, patch)
2016-06-24 03:03 PDT, Per Arne Vollan 		no flags
DetailsFormatted DiffDiff
Patch (8.80 KB, patch)
2016-06-24 04:04 PDT, Per Arne Vollan 		no flags
DetailsFormatted DiffDiff
Patch (6.77 KB, patch)
2016-06-29 13:49 PDT, Per Arne Vollan 		achristensen: review-
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Per Arne Vollan
Comment 1 2016-06-24 03:03:04 PDT
Created attachment 281963 [details] Patch
Per Arne Vollan
Comment 2 2016-06-24 04:04:19 PDT
Created attachment 281967 [details] Patch
Alex Christensen
Comment 3 2016-06-24 09:40:22 PDT
Does this make most of the INTL tests pass? I wish we didn't have to do something like this.
Alex Christensen
Comment 4 2016-06-28 00:02:35 PDT
Comment on attachment 281967 [details] Patch All right. Let's do this. It will be mostly complete.
Per Arne Vollan
Comment 5 2016-06-28 08:29:04 PDT
(In reply to comment #4) > Comment on attachment 281967 [details] > Patch > > All right. Let's do this. It will be mostly complete. Thanks for reviewing :) I got an assertion failure when running the tests js/intl-datetimeformat.html and js/intl-numberformat.html, so it cannot be committed as is.
Per Arne Vollan
Comment 6 2016-06-28 09:06:19 PDT
Sorry for not catching this before!
Per Arne Vollan
Comment 7 2016-06-29 13:49:22 PDT
Created attachment 282369 [details] Patch
WebKit Commit Bot
Comment 8 2016-06-29 13:52:16 PDT
Attachment 282369 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/IntlObject.cpp:67: unumsys_openAvailableNames is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/JavaScriptCore/runtime/IntlObject.cpp:68: unumsys_open is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/JavaScriptCore/runtime/IntlObject.cpp:69: unumsys_close is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/JavaScriptCore/runtime/IntlObject.cpp:70: unumsys_getName is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Total errors found: 4 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Alex Christensen
Comment 9 2016-06-29 13:53:00 PDT
Comment on attachment 282369 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=282369&action=review > Source/JavaScriptCore/runtime/IntlObject.cpp:56 > +#if PLATFORM(WIN) && CPU(X86) I don't like this. Does this mean that only the 32-bit AppleWin build doesn't have updated ICU libraries? I liked the U_ICU_VERSION_MAJOR_NUM solution better than this.
Per Arne Vollan
Comment 10 2016-12-01 01:25:34 PST
We should reenable INTL on Windows.
Note You need to log in before you can comment on or make changes to this bug.