Bug 969928 - Fix a bug in the de-localization code that can cause validation issues with locales that use '.' as their grouping separator. r=dholbert, a=sylvestre
authorJonathan Watt <jwatt@jwatt.org>
Thu, 20 Feb 2014 14:49:44 +0000
changeset 182945 90263d117191920068c0e691cdec78ac2a87e1ed
parent 182944 0a0d646e9e73fc39e9d63a546b460973e3683340
child 182946 e81d62bab6d0e7c77cf91d71b0d076b3238c27fe
push id3343
push userffxbld
push dateMon, 17 Mar 2014 21:55:32 +0000
treeherdermozilla-beta@2f7d3415f79f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert, sylvestre
bugs969928
milestone29.0a2
Bug 969928 - Fix a bug in the de-localization code that can cause validation issues with locales that use '.' as their grouping separator. r=dholbert, a=sylvestre
content/html/content/test/forms/test_input_number_data.js
content/html/content/test/forms/test_input_typing_sanitization.html
layout/forms/nsNumberControlFrame.cpp
--- a/content/html/content/test/forms/test_input_number_data.js
+++ b/content/html/content/test/forms/test_input_number_data.js
@@ -11,13 +11,22 @@ var tests = [
   { desc: "French",
     langTag: "fr-FR", inputWithGrouping: "123 456,78",
     inputWithoutGrouping: "123456,78", value: 123456.78
   },
   { desc: "German",
     langTag: "de", inputWithGrouping: "123.456,78",
     inputWithoutGrouping: "123456,78", value: 123456.78
   },
+  // Extra german test to check that a locale that uses '.' as its grouping
+  // separator doesn't result in it being invalid (due to step mismatch) due
+  // to the de-localization code mishandling numbers that look like other
+  // numbers formatted for English speakers (i.e. treating this as 123.456
+  // instead of 123456):
+  { desc: "German (test 2)",
+    langTag: "de", inputWithGrouping: "123.456",
+    inputWithoutGrouping: "123456", value: 123456
+  },
   { desc: "Hebrew",
     langTag: "he", inputWithGrouping: "123,456.78",
     inputWithoutGrouping: "123456.78", value: 123456.78
   },
 ];
--- a/content/html/content/test/forms/test_input_typing_sanitization.html
+++ b/content/html/content/test/forms/test_input_typing_sanitization.html
@@ -121,17 +121,17 @@ function runTest()
   var data = [
     {
       type: 'number',
       canHaveBadInputValidityState: true,
       validData: [
         "42",
         "-42", // should work for negative values
         "42.1234",
-        "123.12345678912345",  // double precision
+        "123.123456789123",  // double precision
         "1e2", // e should be usable
         "2e1",
         "1e-1", // value after e can be negative
         "1E2", // E can be used instead of e
       ],
       invalidData: [
         "e",
         "e2",
--- a/layout/forms/nsNumberControlFrame.cpp
+++ b/layout/forms/nsNumberControlFrame.cpp
@@ -622,34 +622,63 @@ nsNumberControlFrame::GetValueOfAnonText
   if (!mTextField) {
     aValue.Truncate();
     return;
   }
 
   HTMLInputElement::FromContent(mTextField)->GetValue(aValue);
 
 #ifdef ENABLE_INTL_API
-  // Here we check if the text field's value is a localized serialization of a
-  // number. If it is we set aValue to the de-localize value, but only if the
-  // localized value isn't also a valid floating-point number according to the
-  // HTML 5 spec:
+  // Here we need to de-localize any number typed in by the user. That is, we
+  // need to convert it from the number format of the user's language, region,
+  // etc. to the format that the HTML 5 spec defines to be a "valid
+  // floating-point number":
   //
   //   http://www.whatwg.org/specs/web-apps/current-work/multipage/common-microsyntaxes.html#floating-point-numbers
   //
-  // This is because content (and tests) expect us to avoid "normalizing" the
-  // number that the user types in if it's not necessary. (E.g. if the user
-  // types "2e2" then inputElement.value should be "2e2" and not "100".
+  // so that it can be parsed by functions like HTMLInputElement::
+  // StringToDecimal (the HTML-5-conforming parsing function) which don't know
+  // how to handle numbers that are formatted differently (for example, with
+  // non-ASCII digits, with grouping separator characters or with a decimal
+  // separator character other than '.').
+  //
+  // We need to be careful to avoid normalizing numbers that are already
+  // formatted for a locale that matches the format of HTML 5's "valid
+  // floating-point number" and have no grouping separator characters. (In
+  // other words we want to return the number as specified by the user, not the
+  // de-localized serialization, since the latter will normalize the value.)
+  // For example, if the user's locale is English and the user types in "2e2"
+  // then inputElement.value should be "2e2" and not "100". This is because
+  // content (and tests) expect us to avoid "normalizing" the number that the
+  // user types in if it's not necessary in order to make sure it conforms to
+  // HTML 5's "valid floating-point number" format.
+  //
+  // Note that we also need to be careful when trying to avoid normalization.
+  // For example, just because "1.234" _looks_ like a valid floating-point
+  // number according to the spec does not mean that it should be returned
+  // as-is. If the user's locale is German, then this represents the value
+  // 1234, not 1.234, so it still needs to be de-localized. Alternatively, if
+  // the user's locale is English and they type in "1,234" we _do_ need to
+  // normalize the number to "1234" because HTML 5's valid floating-point
+  // number format does not allow the ',' grouping separator. We can detect all
+  // the cases where we need to convert by seeing if the locale-specific
+  // parsing function understands the user input to mean the same thing as the
+  // HTML-5-conforming parsing function. If so, then we should return the value
+  // as-is to avoid normalization. Otherwise, we return the de-localized
+  // serialization.
   ICUUtils::LanguageTagIterForContent langTagIter(mContent);
   double value = ICUUtils::ParseNumber(aValue, langTagIter);
   if (NS_finite(value) &&
-      !HTMLInputElement::StringToDecimal(aValue).isFinite()) {
+      value != HTMLInputElement::StringToDecimal(aValue).toDouble()) {
     aValue.Truncate();
     aValue.AppendFloat(value);
   }
 #endif
+  // else, we return whatever FromContent put into aValue (the number as typed
+  // in by the user)
 }
 
 bool
 nsNumberControlFrame::AnonTextControlIsEmpty()
 {
   if (!mTextField) {
     return true;
   }