Bug 1099103 - Prevent numbers input using a grouping separator from being mis-processed as if the separator was a decimal separator. r=dholbert
authorJonathan Watt <jwatt@jwatt.org>
Thu, 17 Sep 2015 22:17:35 +0100
changeset 303882 4f554ce35fb804c43ab12f27d18d61ec741d7609
parent 303839 749c557fab382a6b8abe9a6b9dcf52165369f7cd
child 303883 ea407fe50888b54de1ada0680017ac5dd6da7701
push id1001
push userraliiev@mozilla.com
push dateMon, 18 Jan 2016 19:06:03 +0000
treeherdermozilla-release@8b89261f3ac4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1099103
milestone44.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1099103 - Prevent numbers input using a grouping separator from being mis-processed as if the separator was a decimal separator. r=dholbert
dom/html/test/forms/test_input_number_data.js
dom/html/test/forms/test_input_number_l10n.html
dom/html/test/forms/test_input_number_validation.html
layout/forms/nsNumberControlFrame.cpp
--- a/dom/html/test/forms/test_input_number_data.js
+++ b/dom/html/test/forms/test_input_number_data.js
@@ -25,8 +25,14 @@ var tests = [
     langTag: "de", inputWithGrouping: "123.456",
     inputWithoutGrouping: "123456", value: 123456
   },
   { desc: "Hebrew",
     langTag: "he", inputWithGrouping: "123,456.78",
     inputWithoutGrouping: "123456.78", value: 123456.78
   },
 ];
+
+var invalidTests = [
+  // Right now this will pass in a 'de' build, but not in the 'en' build that
+  // are used for testing. See bug .
+  // { desc: "Invalid German", langTag: "de", input: "12.34" }
+];
--- a/dom/html/test/forms/test_input_number_l10n.html
+++ b/dom/html/test/forms/test_input_number_l10n.html
@@ -44,19 +44,32 @@ function runTest(test) {
                                      "') localization with grouping separator");
   elem.value = 0;
   elem.select();
   sendString(test.inputWithoutGrouping);
   is(elem.value, String(test.value), "Test " + test.desc + " ('" + test.langTag +
                                      "') localization without grouping separator");
 }
 
+function runInvalidInputTest(test) {
+  elem.lang = test.langTag;
+  elem.value = 0;
+  elem.focus();
+  elem.select();
+  sendString(test.input);
+  is(elem.value, "", "Test " + test.desc + " ('" + test.langTag +
+                             "') with invalid input: " + test.input);
+}
+
 function startTests() {
   elem = document.getElementById("input");
   for (var test of tests) {
     runTest(test, elem);
   }
+  for (var test of invalidTests) {
+    runInvalidInputTest(test, elem);
+  }
 }
 
 </script>
 </pre>
 </body>
 </html>
--- a/dom/html/test/forms/test_input_number_validation.html
+++ b/dom/html/test/forms/test_input_number_validation.html
@@ -52,21 +52,36 @@ function runTest(test) {
   elem.value = 0;
   elem.select();
   sendString(test.inputWithoutGrouping);
   checkIsValid(elem, `${desc} without grouping separator`);
   sendChar("a");
   checkIsInvalid(elem, `${desc} without grouping separator`);
 }
 
+function runInvalidInputTest(test) {
+  elem.lang = test.langTag;
+
+  gInvalid = false; // reset
+  var desc = `${test.desc}  (lang='${test.langTag}', id='${elem.id}')`;
+  elem.value = 0;
+  elem.focus();
+  elem.select();
+  sendString(test.input);
+  checkIsInvalid(elem, `${desc} with invalid input ${test.input}`);
+}
+
 function startTests() {
   elem = document.getElementById("input");
   for (var test of tests) {
     runTest(test);
   }
+  for (var test of invalidTests) {
+    runInvalidInputTest(test);
+  }
   elem = document.getElementById("requiredinput");
   for (var test of tests) {
     runTest(test);
   }
 
   gInvalid = false; // reset
   elem.value = "";
   checkIsInvalidEmptyValue(elem, "empty value");
--- a/layout/forms/nsNumberControlFrame.cpp
+++ b/layout/forms/nsNumberControlFrame.cpp
@@ -771,56 +771,50 @@ nsNumberControlFrame::GetValueOfAnonText
 #ifdef ENABLE_INTL_API
   // 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
   //
-  // 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.
+  // This is necessary to allow the number that we return to 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
+  // '.').
+
   ICUUtils::LanguageTagIterForContent langTagIter(mContent);
   double value = ICUUtils::ParseNumber(aValue, langTagIter);
-  if (IsFinite(value) &&
-      value != HTMLInputElement::StringToDecimal(aValue).toDouble()) {
+  if (!IsFinite(value)) {
     aValue.Truncate();
-    aValue.AppendFloat(value);
+    return;
   }
+  if (value == HTMLInputElement::StringToDecimal(aValue).toDouble()) {
+    // We want to preserve the formatting of the number as typed in by the user
+    // whenever possible. Since the localized serialization parses to the same
+    // number as the de-localized serialization, we can do that. This helps
+    // prevent normalization of input such as "2e2" (which would otherwise be
+    // converted to "200"). Content relies on this.
+    //
+    // Typically we will only get here for locales in which numbers are
+    // formatted in the same way as they are for HTML5's "valid floating-point
+    // number" format.
+    return;
+  }
+  // We can't preserve the formatting, otherwise functions such as
+  // HTMLInputElement::StringToDecimal would incorrectly process the number
+  // input by the user. For example, "12.345" with lang=de de-localizes as
+  // 12345, but HTMLInputElement::StringToDecimal would mistakenly parse it as
+  // 12.345. Another example would be "12,345" with lang=de which de-localizes
+  // as 12.345, but HTMLInputElement::StringToDecimal would parse it to NaN.
+  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;
   }