Bug 1839572 - Fix number input value sanitization. r=dom-core,edgar, …

…a=dsmith Differential Revision: https://phabricator.services.mozilla.com/D183055
i3roly · Jul 19, 2023 · e7c28b8 · e7c28b8
1 parent 3581fd9
commit e7c28b8
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 9 deletions.
diff --git a/dom/html/HTMLInputElement.cpp b/dom/html/HTMLInputElement.cpp
@@ -19,6 +19,7 @@
 #include "mozilla/dom/FileSystemUtils.h"
 #include "mozilla/dom/FormData.h"
 #include "mozilla/dom/GetFilesHelper.h"
+#include "mozilla/dom/NumericInputTypes.h"
 #include "mozilla/dom/WindowContext.h"
 #include "mozilla/dom/InputType.h"
 #include "mozilla/dom/UserActivation.h"
@@ -1632,15 +1633,10 @@ void HTMLInputElement::SetValue(const nsAString& aValue, CallerType aCallerType,
       // NOTE: this is currently quite expensive work (too much string
       // manipulation). We should probably optimize that.
       nsAutoString currentValue;
-      GetValue(currentValue, aCallerType);
+      GetNonFileValueInternal(currentValue);
 
-      // Some types sanitize value, so GetValue doesn't return pure
-      // previous value correctly.
-      //
-      // FIXME(emilio): Shouldn't above just use GetNonFileValueInternal() to
-      // get the unsanitized value?
       nsresult rv = SetValueInternal(
-          aValue, SanitizesOnValueGetter() ? nullptr : &currentValue,
+          aValue, &currentValue,
           {ValueSetterOption::ByContentAPI, ValueSetterOption::SetValueChanged,
            ValueSetterOption::MoveCursorToEndIfValueChanged});
       if (NS_FAILED(rv)) {
@@ -2645,7 +2641,14 @@ nsresult HTMLInputElement::SetValueInternal(
       // prevent doing it if it's useless.
       nsAutoString value(aValue);
 
-      if (mDoneCreating) {
+      if (mDoneCreating &&
+          !(mType == FormControlType::InputNumber &&
+            aOptions.contains(ValueSetterOption::BySetUserInputAPI))) {
+        // When the value of a number input is set by a script, we need to make
+        // sure the value is a valid floating-point number.
+        // https://html.spec.whatwg.org/#valid-floating-point-number
+        // When it's set by a user, however, we need to be more permissive, so
+        // we don't sanitize its value here. See bug 1839572.
         SanitizeValue(value);
       }
       // else DoneCreatingElement calls us again once mDoneCreating is true
@@ -4586,7 +4589,7 @@ void HTMLInputElement::SanitizeValue(nsAString& aValue,
           aValue);
     } break;
     case FormControlType::InputNumber: {
-      if (!aValue.IsEmpty() &&
+      if (aKind == SanitizationKind::Other && !aValue.IsEmpty() &&
           (aValue.First() == '+' || aValue.Last() == '.')) {
         // A value with a leading plus or trailing dot should fail to parse.
         // However, the localized parser accepts this, and when we convert it

diff --git a/...b-platform/tests/html/semantics/forms/the-input-element/number-constraint-validation.html b/...b-platform/tests/html/semantics/forms/the-input-element/number-constraint-validation.html
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>Form input type=number constraint validation</title>
+<link rel="author" title="Adam Vandolder" href="mailto:[email protected]">
+<link rel=help href="https://html.spec.whatwg.org/multipage/#number-state-(type=number)">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/testdriver.js"></script>
+<script src="/resources/testdriver-vendor.js"></script>
+<div id="log"></div>
+<input type="number">
+<script>
+  const input = document.querySelector("input");
+  const invalidInputNumber = "1.e";
+  const invalidSetNumber = "1.";
+
+  promise_test(async () => {
+    await test_driver.click(input);
+    await test_driver.send_keys(input, invalidInputNumber);
+    assert_equals(input.value.length, 0);
+    assert_true(input.validity.badInput);
+  }, "Unparsable number user input triggers sanitization and causes badInput to be set.");
+
+  promise_test(async () => {
+    input.value = invalidInputNumber;
+    assert_equals(input.value.length, 0);
+    assert_false(input.validity.badInput);
+  }, "Setting .value to an unparsable number triggers sanitization but doesn't set badInput.");
+
+  promise_test(async () => {
+    await test_driver.click(input);
+    await test_driver.send_keys(input, invalidSetNumber);
+    assert_equals(input.value, "1");
+    assert_false(input.validity.badInput);
+  }, "Users inputting a parsable but invalid floating point number doesn't trigger sanitization and doesn't set badInput.");
+
+  promise_test(async () => {
+    input.value = invalidSetNumber;
+    assert_equals(input.value.length, 0);
+    assert_false(input.validity.badInput);
+  }, "Setting .value to a parsable but invalid floating point number triggers sanitization but doesn't set badInput.");
+</script>