Changeset 234260 in webkit

Timestamp:

Jul 26, 2018 9:01:04 AM (6 years ago)

Author:

commit-queue@webkit.org

Message:

JSC: Intl API should ignore encoding when parsing BCP 47 language tag from ISO 15897 locale string (passed via LANG)
​https://bugs.webkit.org/show_bug.cgi?id=167991

Patch by Andy VanWagoner <​andy@vanwagoner.family> on 2018-07-26
Reviewed by Michael Catanzaro.

Source/JavaScriptCore:

Improved the conversion of ICU locales to BCP47 tags, using their preferred method.
Checked locale.isEmpty() before returning it from defaultLocale, so there should be
no more cases where you might have an invalid locale come back from resolveLocale.

• runtime/IntlObject.cpp:

(JSC::convertICULocaleToBCP47LanguageTag):
(JSC::defaultLocale):
(JSC::lookupMatcher):

• runtime/IntlObject.h:
• runtime/JSGlobalObject.cpp:

(JSC::JSGlobalObject::intlCollatorAvailableLocales):
(JSC::JSGlobalObject::intlDateTimeFormatAvailableLocales):
(JSC::JSGlobalObject::intlNumberFormatAvailableLocales):
(JSC::JSGlobalObject::intlPluralRulesAvailableLocales):

LayoutTests:

Replaced expecting throwing a runtime error to avoid a crash, with testing for good default locale fallback behavior.

• js/intl-default-locale-expected.txt: Added.
• js/intl-default-locale.html: Added.
• js/intl-invalid-locale-crash-expected.txt: Removed.
• js/intl-invalid-locale-crash.html: Removed.
• platform/win/TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/js/intl-default-locale-expected.txt (added)
• LayoutTests/js/intl-default-locale.html (added)
• LayoutTests/js/intl-invalid-locale-crash-expected.txt (deleted)
• LayoutTests/js/intl-invalid-locale-crash.html (deleted)
• LayoutTests/platform/win/TestExpectations (modified) (2 diffs)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/runtime/IntlObject.cpp (modified) (3 diffs)
• Source/JavaScriptCore/runtime/IntlObject.h (modified) (1 diff)
• Source/JavaScriptCore/runtime/JSGlobalObject.cpp (modified) (4 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2018-07-26  Andy VanWagoner  <andy@vanwagoner.family>
+
+        JSC: Intl API should ignore encoding when parsing BCP 47 language tag from ISO 15897 locale string (passed via LANG)
+        https://bugs.webkit.org/show_bug.cgi?id=167991
+
+        Reviewed by Michael Catanzaro.
+
+        Replaced expecting throwing a runtime error to avoid a crash, with testing for good default locale fallback behavior.
+
+        * js/intl-default-locale-expected.txt: Added.
+        * js/intl-default-locale.html: Added.
+        * js/intl-invalid-locale-crash-expected.txt: Removed.
+        * js/intl-invalid-locale-crash.html: Removed.
+        * platform/win/TestExpectations:
+
 2018-07-26  Miguel Gomez  <magomez@igalia.com>
 

trunk/LayoutTests/platform/win/TestExpectations

@@ -2482 +2482 @@
 js/intl-collator.html [ Skip ]
 js/intl-datetimeformat.html [ Skip ]
+js/intl-default-locale.html [ Skip ]
 js/intl-numberformat.html [ Skip ]
 js/intl-numberformat-format-to-parts.html [ Skip ]
@@ -3348 +3349 @@
 js/dom/string-prototype-properties.html [ Failure ]
 js/dom/webidl-type-mapping.html [ Failure ]
-js/intl-invalid-locale-crash.html [ Failure ]
 js/regress-141098.html [ Failure ]
 pageoverlay/overlay-installation.html [ Failure ]

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2018-07-26  Andy VanWagoner  <andy@vanwagoner.family>
+
+        JSC: Intl API should ignore encoding when parsing BCP 47 language tag from ISO 15897 locale string (passed via LANG)
+        https://bugs.webkit.org/show_bug.cgi?id=167991
+
+        Reviewed by Michael Catanzaro.
+
+        Improved the conversion of ICU locales to BCP47 tags, using their preferred method.
+        Checked locale.isEmpty() before returning it from defaultLocale, so there should be
+        no more cases where you might have an invalid locale come back from resolveLocale.
+
+        * runtime/IntlObject.cpp:
+        (JSC::convertICULocaleToBCP47LanguageTag):
+        (JSC::defaultLocale):
+        (JSC::lookupMatcher):
+        * runtime/IntlObject.h:
+        * runtime/JSGlobalObject.cpp:
+        (JSC::JSGlobalObject::intlCollatorAvailableLocales):
+        (JSC::JSGlobalObject::intlDateTimeFormatAvailableLocales):
+        (JSC::JSGlobalObject::intlNumberFormatAvailableLocales):
+        (JSC::JSGlobalObject::intlPluralRulesAvailableLocales):
+
 2018-07-26  Fujii Hironori  <Hironori.Fujii@sony.com>
 

trunk/Source/JavaScriptCore/runtime/IntlObject.cpp

@@ -131 +131 @@
 }
 
-void convertICULocaleToBCP47LanguageTag(String& locale)
-{
-    locale.replace('_', '-');
+String convertICULocaleToBCP47LanguageTag(const char* localeID)
+{
+    UErrorCode status = U_ZERO_ERROR;
+    Vector<char, 32> buffer(32);
+    auto length = uloc_toLanguageTag(localeID, buffer.data(), buffer.size(), false, &status);
+    if (status == U_BUFFER_OVERFLOW_ERROR) {
+        buffer.grow(length);
+        status = U_ZERO_ERROR;
+        uloc_toLanguageTag(localeID, buffer.data(), buffer.size(), false, &status);
+    }
+    if (!U_FAILURE(status))
+        return String(buffer.data(), length);
+    return String();
 }
 
@@ -591 +601 @@
     VM& vm = state.vm();
     if (auto defaultLanguage = state.jsCallee()->globalObject(vm)->globalObjectMethodTable()->defaultLanguage) {
-        String locale = defaultLanguage();
+        String locale = canonicalizeLanguageTag(defaultLanguage());
         if (!locale.isEmpty())
-            return canonicalizeLanguageTag(locale);
+            return locale;
     }
 
     Vector<String> languages = userPreferredLanguages();
-    if (!languages.isEmpty() && !languages[0].isEmpty())
-        return canonicalizeLanguageTag(languages[0]);
+    for (const auto& language : languages) {
+        String locale = canonicalizeLanguageTag(language);
+        if (!locale.isEmpty())
+            return locale;
+    }
 
     // If all else fails, ask ICU. It will probably say something bogus like en_us even if the user
     // has configured some other language, but being wrong is better than crashing.
-    String locale = uloc_getDefault();
-    convertICULocaleToBCP47LanguageTag(locale);
-    return locale;
+    String locale = convertICULocaleToBCP47LanguageTag(uloc_getDefault());
+    if (!locale.isEmpty())
+        return locale;
+
+    return "en"_s;
 }
 
@@ -647 +662 @@
 
     MatcherResult result;
-    if (!availableLocale.isNull()) {
+    if (!availableLocale.isEmpty()) {
         result.locale = availableLocale;
         if (locale != noExtensionsLocale) {

trunk/Source/JavaScriptCore/runtime/IntlObject.h

@@ -60 +60 @@
 
 String defaultLocale(ExecState&);
-void convertICULocaleToBCP47LanguageTag(String& locale);
+String convertICULocaleToBCP47LanguageTag(const char* localeID);
 bool intlBooleanOption(ExecState&, JSValue options, PropertyName, bool& usesFallback);
 String intlStringOption(ExecState&, JSValue options, PropertyName, std::initializer_list<const char*> values, const char* notFound, const char* fallback);

trunk/Source/JavaScriptCore/runtime/JSGlobalObject.cpp

@@ -1583 +1583 @@
         int32_t count = ucol_countAvailable();
         for (int32_t i = 0; i < count; ++i) {
-            String locale(ucol_getAvailable(i));
-            convertICULocaleToBCP47LanguageTag(locale);
-            m_intlCollatorAvailableLocales.add(locale);
+            String locale = convertICULocaleToBCP47LanguageTag(ucol_getAvailable(i));
+            if (!locale.isEmpty())
+                m_intlCollatorAvailableLocales.add(locale);
         }
         addMissingScriptLocales(m_intlCollatorAvailableLocales);
@@ -1597 +1597 @@
         int32_t count = udat_countAvailable();
         for (int32_t i = 0; i < count; ++i) {
-            String locale(udat_getAvailable(i));
-            convertICULocaleToBCP47LanguageTag(locale);
-            m_intlDateTimeFormatAvailableLocales.add(locale);
+            String locale = convertICULocaleToBCP47LanguageTag(udat_getAvailable(i));
+            if (!locale.isEmpty())
+                m_intlDateTimeFormatAvailableLocales.add(locale);
         }
         addMissingScriptLocales(m_intlDateTimeFormatAvailableLocales);
@@ -1611 +1611 @@
         int32_t count = unum_countAvailable();
         for (int32_t i = 0; i < count; ++i) {
-            String locale(unum_getAvailable(i));
-            convertICULocaleToBCP47LanguageTag(locale);
-            m_intlNumberFormatAvailableLocales.add(locale);
+            String locale = convertICULocaleToBCP47LanguageTag(unum_getAvailable(i));
+            if (!locale.isEmpty())
+                m_intlNumberFormatAvailableLocales.add(locale);
         }
         addMissingScriptLocales(m_intlNumberFormatAvailableLocales);
@@ -1625 +1625 @@
         int32_t count = uloc_countAvailable();
         for (int32_t i = 0; i < count; ++i) {
-            String locale(uloc_getAvailable(i));
-            convertICULocaleToBCP47LanguageTag(locale);
-            m_intlPluralRulesAvailableLocales.add(locale);
+            String locale = convertICULocaleToBCP47LanguageTag(uloc_getAvailable(i));
+            if (!locale.isEmpty())
+                m_intlPluralRulesAvailableLocales.add(locale);
         }
         addMissingScriptLocales(m_intlPluralRulesAvailableLocales);