Bug 967429 - Disable the UI initiated steping behavior for <input type=number> if the value the user typed in is not valid, and instead highlight the field as being invalid. r=smaug
authorJonathan Watt <jwatt@jwatt.org>
Wed, 12 Feb 2014 17:13:32 +0000
changeset 169661 1a0927d0558bdacda2274212065683ea2d8e1d0c
parent 169660 7ac30a8de3d6cc4f3250114475956c6332064423
child 169662 01df9ef2b4048f1bb86c80518f5abea8450a1359
push id270
push userpvanderbeken@mozilla.com
push dateThu, 06 Mar 2014 09:24:21 +0000
reviewerssmaug
bugs967429
milestone30.0a1
Bug 967429 - Disable the UI initiated steping behavior for <input type=number> if the value the user typed in is not valid, and instead highlight the field as being invalid. r=smaug
content/html/content/src/HTMLInputElement.cpp
content/html/content/test/forms/test_input_number_key_events.html
--- a/content/html/content/src/HTMLInputElement.cpp
+++ b/content/html/content/src/HTMLInputElement.cpp
@@ -2788,25 +2788,26 @@ HTMLInputElement::SetValueInternal(const
         if (mType == NS_FORM_INPUT_EMAIL) {
           UpdateAllValidityStates(mParserCreating);
         }
       } else {
         mInputData.mValue = ToNewUnicode(value);
         if (aSetValueChanged) {
           SetValueChanged(true);
         }
-        OnValueChanged(!mParserCreating);
-
         if (mType == NS_FORM_INPUT_NUMBER) {
+          // This has to happen before OnValueChanged is called because that
+          // method needs the new value of our frame's anon text control.
           nsNumberControlFrame* numberControlFrame =
             do_QueryFrame(GetPrimaryFrame());
           if (numberControlFrame) {
             numberControlFrame->SetValueOfAnonTextControl(value);
           }
         }
+        OnValueChanged(!mParserCreating);
       }
 
       // Call parent's SetAttr for color input so its control frame is notified
       // and updated
       if (mType == NS_FORM_INPUT_COLOR) {
         return nsGenericHTMLFormElement::SetAttr(kNameSpaceID_None,
                                                  nsGkAtoms::value, aValue,
                                                  true);
@@ -3628,16 +3629,30 @@ HTMLInputElement::StopNumberControlSpinn
       numberControlFrame->SpinnerStateChanged();
     }
   }
 }
 
 void
 HTMLInputElement::StepNumberControlForUserEvent(int32_t aDirection)
 {
+  if (!IsValid()) {
+    // If the user has typed a value into the control and inadvertently made a
+    // mistake (e.g. put a thousand separator at the wrong point) we do not
+    // want to wipe out what they typed if they try to increment/decrement the
+    // value. Better is to highlight the value as being invalid so that they
+    // can correct what they typed.
+    // We pass 'true' for UpdateValidityUIBits' aIsFocused argument regardless
+    // because we need the UI to update _now_ or the user will wonder why the
+    // step behavior isn't functioning.
+    UpdateValidityUIBits(true);
+    UpdateState(true);
+    return;
+  }
+
   Decimal newValue = Decimal::nan(); // unchanged if value will not change
 
   nsresult rv = GetValueIfStepped(aDirection, CALLED_FOR_USER_EVENT, &newValue);
 
   if (NS_FAILED(rv) || !newValue.isFinite()) {
     return; // value should not or will not change
   }
 
--- a/content/html/content/test/forms/test_input_number_key_events.html
+++ b/content/html/content/test/forms/test_input_number_key_events.html
@@ -141,20 +141,20 @@ function test() {
   elem.min = -3;
   elem.max = 3;
   elem.step = 2;
   var defaultValue = 0;
   var oldVal, expectedVal;
 
   for (key of ["VK_UP", "VK_DOWN"]) {
     // Start at middle:
-    oldVal = elem.value = 0;
+    oldVal = elem.value = -1;
     expectedVal = expectedValAfterKeyEvent(key, elem);
     synthesizeKey(key, {});
-    is(elem.value, expectedVal, "Test " + key + " for number control with value set to the midpoint (" + oldVal + ")");
+    is(elem.value, expectedVal, "Test " + key + " for number control with value set between min/max (" + oldVal + ")");
 
     // Same again:
     expectedVal = expectedValAfterKeyEvent(key, elem);
     synthesizeKey(key, {});
     is(elem.value, expectedVal, "Test repeat of " + key + " for number control");
 
     // Start at maximum:
     oldVal = elem.value = elem.max;
@@ -191,15 +191,22 @@ function test() {
     // Test step="any" behavior:
     var oldStep = elem.step;
     elem.step = "any";
     oldVal = elem.value = 0;
     expectedVal = expectedValAfterKeyEvent(key, elem);
     synthesizeKey(key, {});
     is(elem.value, expectedVal, "Test " + key + " for number control with value set to the midpoint and step='any' (" + oldVal + ")");
     elem.step = oldStep; // restore
+
+    // Test that invalid input blocks UI initiated stepping:
+    oldVal = elem.value = "";
+    elem.select();
+    sendString("abc");
+    synthesizeKey(key, {});
+    is(elem.value, "", "Test " + key + " does nothing when the input is invalid");
   }
 }
 
 </script>
 </pre>
 </body>
 </html>