Bug 945784, part 2 - Fire 'change' events for <input type=number> more frequently, per the new HTML5 rules. r=smaug
authorJonathan Watt <jwatt@jwatt.org>
Thu, 05 Dec 2013 16:20:33 +0000
changeset 174736 8a9e9debc9b19226ef62a5c434d538a634d6d1bc
parent 174735 8b9c628c3c53f1959222a07f07196915a2e21c0a
child 174737 45decd50b1180a19bd4e57f44f6db969f331ee68
push id445
push userffxbld
push dateMon, 10 Mar 2014 22:05:19 +0000
treeherdermozilla-release@dc38b741b04e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs945784
milestone28.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 945784, part 2 - Fire 'change' events for <input type=number> more frequently, per the new HTML5 rules. r=smaug
content/html/content/src/HTMLInputElement.cpp
content/html/content/src/HTMLInputElement.h
content/html/content/test/forms/test_change_event.html
--- a/content/html/content/src/HTMLInputElement.cpp
+++ b/content/html/content/src/HTMLInputElement.cpp
@@ -2046,17 +2046,17 @@ HTMLInputElement::GetStepBase() const
       ConvertStringToNumber(valueStr, stepBase)) {
     return stepBase;
   }
 
   return kDefaultStepBase;
 }
 
 nsresult
-HTMLInputElement::ApplyStep(int32_t aStep)
+HTMLInputElement::GetValueIfStepped(int32_t aStep, Decimal* aNextStep)
 {
   if (!DoStepDownStepUpApply()) {
     return NS_ERROR_DOM_INVALID_STATE_ERR;
   }
 
   Decimal step = GetStep();
   if (step == kStepAny) {
     return NS_ERROR_DOM_INVALID_STATE_ERR;
@@ -2126,21 +2126,35 @@ HTMLInputElement::ApplyStep(int32_t aSte
   // If we go down, we want to clamp on min.
   } else if (aStep < 0 && minimum == minimum) {
     value = std::max(value, minimum);
   // If we go up, we want to clamp on max.
   } else if (aStep > 0 && maximum == maximum) {
     value = std::min(value, maximum);
   }
 
-  SetValue(value);
+  *aNextStep = value;
 
   return NS_OK;
 }
 
+nsresult
+HTMLInputElement::ApplyStep(int32_t aStep)
+{
+  Decimal nextStep = Decimal::nan(); // unchanged if value will not change
+
+  nsresult rv = GetValueIfStepped(aStep, &nextStep);
+
+  if (NS_SUCCEEDED(rv) && nextStep.isFinite()) {
+    SetValue(nextStep);
+  }
+
+  return rv;
+}
+
 NS_IMETHODIMP
 HTMLInputElement::StepDown(int32_t n, uint8_t optional_argc)
 {
   return ApplyStep(optional_argc ? -n : -1);
 }
 
 NS_IMETHODIMP
 HTMLInputElement::StepUp(int32_t n, uint8_t optional_argc)
@@ -3342,16 +3356,28 @@ HTMLInputElement::PreHandleEvent(nsEvent
           // is moved to our anonymous text control.
           nsNumberControlFrame* numberControlFrame =
             do_QueryFrame(GetPrimaryFrame());
           if (numberControlFrame) {
             numberControlFrame->HandleFocusEvent(aVisitor.mEvent);
           }
         }
       }
+    } else if (aVisitor.mEvent->message == NS_KEY_UP) {
+      WidgetKeyboardEvent* keyEvent = aVisitor.mEvent->AsKeyboardEvent();
+      if ((keyEvent->keyCode == NS_VK_UP || keyEvent->keyCode == NS_VK_DOWN) &&
+          !(keyEvent->IsShift() || keyEvent->IsControl() ||
+            keyEvent->IsAlt() || keyEvent->IsMeta() ||
+            keyEvent->IsAltGraph() || keyEvent->IsFn() ||
+            keyEvent->IsOS())) {
+        // The up/down arrow key events fire 'change' events when released
+        // so that at the end of a series of up/down arrow key repeat events
+        // the value is considered to be "commited" by the user.
+        FireChangeEventIfNeeded();
+      }
     }
   }
 
   nsresult rv = nsGenericHTMLFormElementWithState::PreHandleEvent(aVisitor);
 
   // We do this after calling the base class' PreHandleEvent so that
   // nsIContent::PreHandleEvent doesn't reset any change we make to mCanHandle.
   if (mType == NS_FORM_INPUT_NUMBER &&
@@ -3501,17 +3527,28 @@ HTMLInputElement::StopNumberControlSpinn
 
     FireChangeEventIfNeeded();
   }
 }
 
 void
 HTMLInputElement::StepNumberControlForUserEvent(int32_t aDirection)
 {
-  ApplyStep(aDirection);
+  Decimal newValue = Decimal::nan(); // unchanged if value will not change
+
+  nsresult rv = GetValueIfStepped(aDirection, &newValue);
+
+  if (NS_FAILED(rv) || !newValue.isFinite()) {
+    return; // value should not or will not change
+  }
+
+  nsAutoString newVal;
+  ConvertNumberToString(newValue, newVal);
+  SetValueInternal(newVal, true, true);
+
   nsContentUtils::DispatchTrustedEvent(OwnerDoc(),
                                        static_cast<nsIDOMHTMLInputElement*>(this),
                                        NS_LITERAL_STRING("input"), true,
                                        false);
 }
 
 static bool
 SelectTextFieldOnFocus()
--- a/content/html/content/src/HTMLInputElement.h
+++ b/content/html/content/src/HTMLInputElement.h
@@ -1089,16 +1089,30 @@ protected:
 
   /**
    * Returns the default step for the current type.
    * @return the default step for the current type.
    */
   Decimal GetDefaultStep() const;
 
   /**
+   * Sets the aValue outparam to the value that this input would take if
+   * someone tries to step aStep steps and this input's value would change as
+   * a result. Leaves aValue untouched if this inputs value would not change
+   * (e.g. already at max, and asking for the next step up).
+   *
+   * Negative aStep means step down, positive means step up.
+   *
+   * Returns NS_OK or else the error values that should be thrown if this call
+   * was initiated by a stepUp()/stepDown() call from script under conditions
+   * that such a call should throw.
+   */
+  nsresult GetValueIfStepped(int32_t aStep, Decimal* aNextStep);
+
+  /**
    * Apply a step change from stepUp or stepDown by multiplying aStep by the
    * current step value.
    *
    * @param aStep The value used to be multiplied against the step value.
    */
   nsresult ApplyStep(int32_t aStep);
 
   /**
--- a/content/html/content/test/forms/test_change_event.html
+++ b/content/html/content/test/forms/test_change_event.html
@@ -32,16 +32,19 @@ https://bugzilla.mozilla.org/show_bug.cg
 <input type="number" id="input_number" onchange="++numberChange;"></input>
 <input type="range" id="input_range" onchange="++rangeChange;"></input>
  
 </div>
 <pre id="test">
 <script class="testbody" type="text/javascript">
 
   /** Test for Bug 722599 **/
+
+  const isDesktop = !/Mobile|Tablet/.test(navigator.userAgent);
+
   var textareaChange = 0;
   var fileInputChange = 0;
 
   var textInputTypes = ["text", "email", "search", "telephone", "url", "password"];
   var textInputChange = [0, 0, 0, 0, 0, 0];
 
   var NonTextInputTypes = ["button", "submit", "image", "reset", "radio", "checkbox"];
   var NonTextInputChange = [0, 0, 0, 0, 0, 0];
@@ -170,19 +173,30 @@ https://bugzilla.mozilla.org/show_bug.cg
     var number = document.getElementById("input_number");
     number.focus();
     synthesizeKey("a", {});
     number.blur();
     is(numberChange, 0, "Change event shouldn't be dispatched on number input element for key changes that don't change its value");
     number.value = "";
     number.focus();
     synthesizeKey("1", {});
+    synthesizeKey("2", {});
     is(numberChange, 0, "Change event shouldn't be dispatched on number input element for keyboard input until it loses focus");
     number.blur();
     is(numberChange, 1, "Change event should be dispatched on number input element on blur");
+    is(number.value, 12, "Sanity check that number keys were actually handled");
+    if (isDesktop) { // up/down arrow keys not supported on android/b2g
+      number.value = "";
+      number.focus();
+      synthesizeKey("VK_UP", {});
+      synthesizeKey("VK_UP", {});
+      synthesizeKey("VK_DOWN", {});
+      is(numberChange, 4, "Change event should be dispatched on number input element for up/down arrow keys (a special case)");
+      is(number.value, 1, "Sanity check that number and arrow keys were actually handled");
+    }
 
     // Special case type=range
     var range = document.getElementById("input_range");
     range.focus();
     synthesizeKey("a", {});
     range.blur();
     is(rangeChange, 0, "Change event shouldn't be dispatched on range input element for key changes that don't change its value");
     range.focus();