Bug 1286509 - Range input does not fire ‘change’ event when the range is changed using the keyboard. r=smaug. a=ritu
authorStone Shih <sshih@mozilla.com>
Mon, 01 Aug 2016 10:47:34 +0800
changeset 350649 da1c556f903c52918b73061c63fe682eb0ee58c6
parent 350648 f25b571379d01af61d001906b4181105c6854740
child 350650 61f5a6000c752b08ba01a18d64a186d500cb91a4
push id1230
push userjlund@mozilla.com
push dateMon, 31 Oct 2016 18:13:35 +0000
treeherdermozilla-release@5e06e3766db2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, ritu
bugs1286509
milestone50.0
Bug 1286509 - Range input does not fire ‘change’ event when the range is changed using the keyboard. r=smaug. a=ritu
dom/html/HTMLInputElement.cpp
dom/html/HTMLInputElement.h
dom/html/test/forms/mochitest.ini
dom/html/test/forms/test_bug1286509.html
--- a/dom/html/HTMLInputElement.cpp
+++ b/dom/html/HTMLInputElement.cpp
@@ -3770,29 +3770,29 @@ HTMLInputElement::PreHandleEvent(EventCh
         // Our frame's nested <input type=text> will be invalidated when it
         // loses focus, but since we are also native themed we need to make
         // sure that our entire area is repainted since any focus highlight
         // from the theme should be removed from us (the repainting of the
         // sub-area occupied by the anon text control is not enough to do
         // that).
         frame->InvalidateFrame();
       }
-    } else if (aVisitor.mEvent->mMessage == eKeyUp) {
-      WidgetKeyboardEvent* keyEvent = aVisitor.mEvent->AsKeyboardEvent();
-      if ((keyEvent->mKeyCode == NS_VK_UP ||
-           keyEvent->mKeyCode == 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();
-      }
+    }
+  }
+
+  if (aVisitor.mEvent->mMessage == eKeyUp && aVisitor.mEvent->IsTrusted()) {
+    WidgetKeyboardEvent* keyEvent = aVisitor.mEvent->AsKeyboardEvent();
+    if (MayFireChangeOnKeyUp(keyEvent->mKeyCode) &&
+        !(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 &&
@@ -8152,12 +8152,24 @@ HTMLInputElement::UpdateEntries(const ns
 
 void
 HTMLInputElement::GetWebkitEntries(nsTArray<RefPtr<Entry>>& aSequence)
 {
   Telemetry::Accumulate(Telemetry::BLINK_FILESYSTEM_USED, true);
   aSequence.AppendElements(mEntries);
 }
 
+bool HTMLInputElement::MayFireChangeOnKeyUp(uint32_t aKeyCode) const
+{
+  switch (mType) {
+    case NS_FORM_INPUT_NUMBER:
+      return aKeyCode == NS_VK_UP || aKeyCode == NS_VK_DOWN;
+    case NS_FORM_INPUT_RANGE:
+      return aKeyCode == NS_VK_UP || aKeyCode == NS_VK_DOWN ||
+             aKeyCode == NS_VK_LEFT || aKeyCode == NS_VK_RIGHT;
+  }
+  return false;
+}
+
 } // namespace dom
 } // namespace mozilla
 
 #undef NS_ORIGINAL_CHECKED_VALUE
--- a/dom/html/HTMLInputElement.h
+++ b/dom/html/HTMLInputElement.h
@@ -1492,16 +1492,22 @@ private:
   }
 
   static bool MayFireChangeOnBlur(uint8_t aType) {
     return IsSingleLineTextControl(false, aType) ||
            aType == NS_FORM_INPUT_RANGE ||
            aType == NS_FORM_INPUT_NUMBER;
   }
 
+  /**
+   * Returns true if the key code may induce the input's value changed to fire
+   * a "change" event accordingly.
+   */
+  bool MayFireChangeOnKeyUp(uint32_t aKeyCode) const;
+
   struct nsFilePickerFilter {
     nsFilePickerFilter()
       : mFilterMask(0) {}
 
     explicit nsFilePickerFilter(int32_t aFilterMask)
       : mFilterMask(aFilterMask) {}
 
     nsFilePickerFilter(const nsString& aTitle,
--- a/dom/html/test/forms/mochitest.ini
+++ b/dom/html/test/forms/mochitest.ini
@@ -1,15 +1,17 @@
 [DEFAULT]
 support-files =
   save_restore_radio_groups.sjs
   test_input_number_data.js
   !/dom/html/test/reflect.js
 
 [test_bug1039548.html]
+[test_bug1286509.html]
+skip-if = os == "android" || appname == "b2g" # up/down arrow keys not supported on android/b2g
 [test_button_attributes_reflection.html]
 [test_input_radio_radiogroup.html]
 [test_input_radio_required.html]
 [test_change_event.html]
 skip-if = (buildapp == 'b2g' && toolkit != 'gonk') #Bug 931116, b2g desktop specific, initial triage
 [test_datalist_element.html]
 [test_form_attribute-1.html]
 [test_form_attribute-2.html]
new file mode 100644
--- /dev/null
+++ b/dom/html/test/forms/test_bug1286509.html
@@ -0,0 +1,49 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1286509
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 1286509</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1286509">Mozilla Bug 1286509</a>
+<p id="display"></p>
+<div id="content">
+  <input type="range" id="test_input" min="0" max="10" value="5">
+</div>
+<pre id="test">
+  <script type="application/javascript">
+  /** Test for Bug 1286509 **/
+  SimpleTest.waitForExplicitFinish();
+  var expectedEventSequence = ['keydown', 'change', 'keyup'];
+  var eventCounts = {};
+  var expectedEventIdx = 0;
+
+  function test() {
+    var range = document.getElementById("test_input");
+    range.focus();
+    expectedEventSequence.forEach((eventName) => {
+      eventCounts[eventName] = 0;
+      range.addEventListener(eventName, (e) => {
+        ++eventCounts[eventName];
+        is(expectedEventSequence[expectedEventIdx], e.type, "Events sequence should be keydown, change, keyup");
+        expectedEventIdx = (expectedEventIdx + 1) % 3;
+      }, false);
+    });
+    synthesizeKey("VK_UP", {});
+    synthesizeKey("VK_DOWN", {});
+    synthesizeKey("VK_LEFT", {});
+    synthesizeKey("VK_RIGHT", {});
+    is(eventCounts['change'], 4, "Expect key up/down/left/right should trigger range input to fire change events");
+    SimpleTest.finish();
+  }
+  addLoadEvent(test);
+  </script>
+</pre>
+</body>
+</html>