Bug 903715 - Consume Enter keydown event if dropdown of <select> element is closed by it. r=smaug, a=lsblakk
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 14 Aug 2013 15:34:03 +0900
changeset 153898 6a9878e0ae7b18524de74d1d9a5a0a8b80b7f749
parent 153897 25e15866488f5623202a858c42e86bed93daafc0
child 153899 abbdc26298fa24d7e3404fe0dac41e8979b58843
push id2859
push userakeybl@mozilla.com
push dateMon, 16 Sep 2013 19:14:59 +0000
treeherdermozilla-beta@87d3c51cd2bf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, lsblakk
bugs903715
milestone25.0a2
Bug 903715 - Consume Enter keydown event if dropdown of <select> element is closed by it. r=smaug, a=lsblakk
layout/forms/nsListControlFrame.cpp
layout/forms/test/Makefile.in
layout/forms/test/test_bug903715.html
testing/mochitest/android.json
testing/mochitest/androidx86.json
testing/mochitest/b2g.json
--- a/layout/forms/nsListControlFrame.cpp
+++ b/layout/forms/nsListControlFrame.cpp
@@ -2123,33 +2123,47 @@ nsListControlFrame::KeyDown(nsIDOMEvent*
     case NS_VK_DOWN:
     case NS_VK_RIGHT:
       AdjustIndexForDisabledOpt(mEndSelectionIndex, newIndex,
                                 static_cast<int32_t>(numOptions),
                                 1, 1);
       break;
     case NS_VK_RETURN:
       if (mComboboxFrame) {
+        nsWeakFrame weakFrame(this);
         if (mComboboxFrame->IsDroppedDown()) {
-          nsWeakFrame weakFrame(this);
+          // At closing dropdown, users may not expect there is additional
+          // behavior for this key event.  Therefore, let's consume the event.
+          aKeyEvent->PreventDefault();
           ComboboxFinish(mEndSelectionIndex);
           if (!weakFrame.IsAlive()) {
             return NS_OK;
           }
         }
         FireOnChange();
+        if (!weakFrame.IsAlive()) {
+          // If the keydown event causes destroying this, fired keypress on
+          // another element may cause another action which may not be
+          // expected by the user.
+          aKeyEvent->PreventDefault();
+        }
         return NS_OK;
       }
       newIndex = mEndSelectionIndex;
       break;
     case NS_VK_ESCAPE: {
       nsWeakFrame weakFrame(this);
+      // XXX When the Escape keydown causes closing dropdown, it shouldn't
+      //     cause any additonal actions. We should call preventDefault() here.
       AboutToRollup();
       if (!weakFrame.IsAlive()) {
-        aKeyEvent->PreventDefault(); // since we won't reach the one below
+        // If the keydown event causes destroying this, fired keypress on
+        // another element may cause another action which may not be
+        // expected by the user.
+        aKeyEvent->PreventDefault();
         return NS_OK;
       }
       break;
     }
     case NS_VK_PAGE_UP: {
       int32_t itemsPerPage =
         std::max(1, static_cast<int32_t>(mNumDisplayRows - 1));
       AdjustIndexForDisabledOpt(mEndSelectionIndex, newIndex,
--- a/layout/forms/test/Makefile.in
+++ b/layout/forms/test/Makefile.in
@@ -39,16 +39,17 @@ MOCHITEST_FILES =	test_bug231389.html \
 		test_bug572406.html \
 		test_bug572649.html \
 		test_bug620936.html \
 		test_bug595310.html \
 		test_bug644542.html \
 		test_bug672810.html \
 		test_bug704049.html \
 		test_bug869314.html \
+		test_bug903715.html \
 		test_listcontrol_search.html \
 		test_select_prevent_default.html \
 		$(NULL)
 
 MOCHITEST_CHROME_FILES = \
 		     bug536567_iframe.html \
 		test_bug536567_perwindowpb.html \
 		     bug536567_subframe.html \
new file mode 100644
--- /dev/null
+++ b/layout/forms/test/test_bug903715.html
@@ -0,0 +1,80 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=903715
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 903715</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=903715">Mozilla Bug 903715</a>
+<p id="display"></p>
+<div id="content">
+  <form id="form" action="/">
+    <select id="select" name="select">
+      <option>1</option>
+      <option>2</option>
+      <option>3</option>
+      <option>4</option>
+      <option>5</option>
+      <option>6</option>
+      <option>7</option>
+      <option>8</option>
+      <option>9</option>
+    </select>
+    <input id="input-text" name="text" value="some text">
+    <input id="input-submit" type="submit">
+  </form>
+</div>
+<pre id="test">
+</pre>
+<script type="application/javascript">
+SimpleTest.waitForExplicitFinish();
+SimpleTest.waitForFocus(runTests, window);
+
+function runTests()
+{
+  var form = document.getElementById("form");
+  form.addEventListener("keypress", function (aEvent) {
+    ok(false, "keypress event shouldn't be fired when the preceding keydown event caused closing the dropdown of the select element");
+  }, true);
+  form.addEventListener("submit", function (aEvent) {
+    ok(false, "submit shouldn't be performed by the Enter key press on the select element");
+    aEvent.preventDefault();
+  }, true);
+  var select = document.getElementById("select");
+  select.addEventListener("change", function (aEvent) {
+    var input = document.getElementById("input-text");
+    input.focus();
+    input.select();
+  }, false);
+
+  select.focus();
+
+  select.addEventListener("popupshowing", function (aEvent) {
+    setTimeout(function () {
+      synthesizeKey("VK_DOWN", { });
+      select.addEventListener("popuphiding", function (aEvent) {
+        setTimeout(function () {
+          // Enter key should cause closing the dropdown of the select element
+          // and keypress event shouldn't be fired on the input element because
+          // which shouldn't cause sumbmitting the form contents.
+          ok(true, "Test passes if there is no error");
+          SimpleTest.finish();
+        }, 100);
+      }, false);
+      // Close dropdown.
+      synthesizeKey("VK_RETURN", { });
+    }, 100);
+  }, false);
+
+  // Open dropdown.
+  synthesizeKey("VK_DOWN", { altKey: true });
+}
+</script>
+</body>
+</html>
--- a/testing/mochitest/android.json
+++ b/testing/mochitest/android.json
@@ -245,16 +245,17 @@
  "layout/forms/test/test_bug378670.html": "TIMED_OUT",
  "layout/forms/test/test_bug446663.html": "",
  "layout/forms/test/test_bug478219.xhtml": "",
  "layout/forms/test/test_bug564115.html": "TIMED_OUT",
  "layout/forms/test/test_bug571352.html": "TIMED_OUT",
  "layout/forms/test/test_bug572649.html": "TIMED_OUT",
  "layout/forms/test/test_bug644542.html": "TIMED_OUT",
  "layout/forms/test/test_bug672810.html": "",
+ "layout/forms/test/test_bug903715.html":"select elements don't use an in-page popup on Android",
  "layout/forms/test/test_textarea_resize.html": "",
  "layout/forms/test/test_listcontrol_search.html": "select elements don't use an in-page popup on Android",
  "layout/generic/test/test_bug496275.html": "CRASH_DUMP",
  "layout/generic/test/test_bug503813.html": "CRASH_DUMP",
  "layout/generic/test/test_bug514732.html": "CRASH_DUMP",
  "layout/generic/test/test_invalidate_during_plugin_paint.html": "",
  "layout/generic/test/test_plugin_mouse_coords.html": "",
  "layout/generic": "CRASH_DUMP, RANDOM, ONLY IN CHUNK 10",
--- a/testing/mochitest/androidx86.json
+++ b/testing/mochitest/androidx86.json
@@ -246,16 +246,17 @@
  "layout/forms/test/test_bug378670.html": "TIMED_OUT",
  "layout/forms/test/test_bug446663.html": "",
  "layout/forms/test/test_bug478219.xhtml": "",
  "layout/forms/test/test_bug564115.html": "TIMED_OUT",
  "layout/forms/test/test_bug571352.html": "TIMED_OUT",
  "layout/forms/test/test_bug572649.html": "TIMED_OUT",
  "layout/forms/test/test_bug644542.html": "TIMED_OUT",
  "layout/forms/test/test_bug672810.html": "",
+ "layout/forms/test/test_bug903715.html":"select elements don't use an in-page popup on Android",
  "layout/forms/test/test_textarea_resize.html": "",
  "layout/forms/test/test_listcontrol_search.html": "select elements don't use an in-page popup on Android",
  "layout/generic/test/test_bug496275.html": "CRASH_DUMP",
  "layout/generic/test/test_bug503813.html": "CRASH_DUMP",
  "layout/generic/test/test_bug514732.html": "CRASH_DUMP",
  "layout/generic/test/test_invalidate_during_plugin_paint.html": "",
  "layout/generic/test/test_plugin_mouse_coords.html": "",
  "layout/generic": "CRASH_DUMP, RANDOM, ONLY IN CHUNK 10",
--- a/testing/mochitest/b2g.json
+++ b/testing/mochitest/b2g.json
@@ -386,16 +386,17 @@
     "layout/forms/test/test_bug477700.html":"",
     "layout/forms/test/test_bug534785.html":"",
     "layout/forms/test/test_bug549170.html":"",
     "layout/forms/test/test_bug563642.html":"",
     "layout/forms/test/test_bug572649.html":"",
     "layout/forms/test/test_bug644542.html":"",
     "layout/forms/test/test_bug672810.html":"",
     "layout/forms/test/test_bug704049.html":"",
+    "layout/forms/test/test_bug903715.html":"select elements don't use an in-page popup in B2G",
     "layout/forms/test/test_listcontrol_search.html" : "select elements don't use an in-page popup in B2G",
     "layout/generic/test/test_bug240933.html":"",
     "layout/generic/test/test_bug263683.html":"",
     "layout/generic/test/test_bug290397.html":"",
     "layout/generic/test/test_bug392746.html":"",
     "layout/generic/test/test_bug424627.html":"",
     "layout/generic/test/test_bug438840.html":"",
     "layout/generic/test/test_bug468167.html":"",