Bug 291082 - preventDefault now blocks keyboard navigation in select-one drop-down lists. r=mounir, r=Neil
authorMina Almasry <almasry.mina@gmail.com>
Fri, 19 Jul 2013 10:32:51 -0400
changeset 151523 952442f77c132ff3b18ca6d35b4501658583b416
parent 151522 868ce514bba712fda6578a692505ad5cb938edb7
child 151524 657c02a2ff0f9f1cc2a18c30b530959110b5e59f
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)
reviewersmounir, Neil
bugs291082
milestone25.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 291082 - preventDefault now blocks keyboard navigation in select-one drop-down lists. r=mounir, r=Neil This patch makes select drop-down lists respect preventDefault keypress event, and so the select list doesn't process them anymore.
layout/forms/nsListControlFrame.cpp
layout/forms/test/Makefile.in
layout/forms/test/test_select_prevent_default.html
--- a/layout/forms/nsListControlFrame.cpp
+++ b/layout/forms/nsListControlFrame.cpp
@@ -137,24 +137,24 @@ nsListControlFrame::DestroyFrom(nsIFrame
   // get the receiver interface from the browser button's content node
   ENSURE_TRUE(mContent);
 
   // Clear the frame pointer on our event listener, just in case the
   // event listener can outlive the frame.
 
   mEventListener->SetFrame(nullptr);
 
-  mContent->RemoveEventListener(NS_LITERAL_STRING("keypress"), mEventListener,
-                                false);
-  mContent->RemoveEventListener(NS_LITERAL_STRING("mousedown"), mEventListener,
-                                false);
-  mContent->RemoveEventListener(NS_LITERAL_STRING("mouseup"), mEventListener,
-                                false);
-  mContent->RemoveEventListener(NS_LITERAL_STRING("mousemove"), mEventListener,
-                                false);
+  mContent->RemoveSystemEventListener(NS_LITERAL_STRING("keypress"),
+                                      mEventListener, false);
+  mContent->RemoveSystemEventListener(NS_LITERAL_STRING("mousedown"),
+                                      mEventListener, false);
+  mContent->RemoveSystemEventListener(NS_LITERAL_STRING("mouseup"),
+                                      mEventListener, false);
+  mContent->RemoveSystemEventListener(NS_LITERAL_STRING("mousemove"),
+                                      mEventListener, false);
 
   nsFormControlFrame::RegUnRegAccessKey(static_cast<nsIFrame*>(this), false);
   nsHTMLScrollFrame::DestroyFrom(aDestructRoot);
 }
 
 void
 nsListControlFrame::BuildDisplayList(nsDisplayListBuilder*   aBuilder,
                                      const nsRect&           aDirtyRect,
@@ -991,24 +991,24 @@ nsListControlFrame::Init(nsIContent*    
   nsHTMLScrollFrame::Init(aContent, aParent, aPrevInFlow);
 
   // we shouldn't have to unregister this listener because when
   // our frame goes away all these content node go away as well
   // because our frame is the only one who references them.
   // we need to hook up our listeners before the editor is initialized
   mEventListener = new nsListEventListener(this);
 
-  mContent->AddEventListener(NS_LITERAL_STRING("keypress"), mEventListener,
-                             false, false);
-  mContent->AddEventListener(NS_LITERAL_STRING("mousedown"), mEventListener,
-                             false, false);
-  mContent->AddEventListener(NS_LITERAL_STRING("mouseup"), mEventListener,
-                             false, false);
-  mContent->AddEventListener(NS_LITERAL_STRING("mousemove"), mEventListener,
-                             false, false);
+  mContent->AddSystemEventListener(NS_LITERAL_STRING("keypress"),
+                                   mEventListener, false, false);
+  mContent->AddSystemEventListener(NS_LITERAL_STRING("mousedown"),
+                                   mEventListener, false, false);
+  mContent->AddSystemEventListener(NS_LITERAL_STRING("mouseup"),
+                                   mEventListener, false, false);
+  mContent->AddSystemEventListener(NS_LITERAL_STRING("mousemove"),
+                                   mEventListener, false, false);
 
   mStartSelectionIndex = kNothingSelected;
   mEndSelectionIndex = kNothingSelected;
 
   mLastDropdownBackstopColor = PresContext()->DefaultBackgroundColor();
 
   if (IsInDropDownMode()) {
     AddStateBits(NS_FRAME_IN_POPUP);
@@ -2369,17 +2369,27 @@ nsListControlFrame::KeyPress(nsIDOMEvent
 #endif
 
     case nsIDOMKeyEvent::DOM_VK_TAB: {
       return NS_OK;
     }
 
     default: { // Select option with this as the first character
                // XXX Not I18N compliant
-      
+
+      // We skip processing all key events in the keys that are not in the
+      // cases above, if they have been prevented. The keys listed in the cases
+      // above are required to navigate inside the list. These keys are also
+      // not prevented in most UAs, so this is also a compatibility issue.
+      bool defaultPrevented;
+      aKeyEvent->GetDefaultPrevented(&defaultPrevented);
+      if (defaultPrevented) {
+        return NS_OK;
+      }
+
       if (isControl && charcode != ' ') {
         return NS_OK;
       }
 
       didIncrementalSearch = true;
       if (charcode == 0) {
         // Backspace key will delete the last char in the string
         if (keycode == NS_VK_BACK && !GetIncrementalString().IsEmpty()) {
--- a/layout/forms/test/Makefile.in
+++ b/layout/forms/test/Makefile.in
@@ -40,16 +40,17 @@ MOCHITEST_FILES =	test_bug231389.html \
 		test_bug572649.html \
 		test_bug620936.html \
 		test_bug595310.html \
 		test_bug644542.html \
 		test_bug672810.html \
 		test_bug704049.html \
 		test_bug869314.html \
 		test_listcontrol_search.html \
+		test_select_prevent_default.html \
 		$(NULL)
 
 MOCHITEST_CHROME_FILES = \
 		     bug536567_iframe.html \
 		test_bug536567_perwindowpb.html \
 		     bug536567_subframe.html \
 		test_bug665540.html \
 		     bug665540_window.xul \
new file mode 100644
--- /dev/null
+++ b/layout/forms/test/test_select_prevent_default.html
@@ -0,0 +1,107 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=291082
+-->
+<head>
+<meta charset="utf-8">
+<title>Test for Bug 291082</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"/>
+<script type="application/javascript">
+  /** Test for Bug 291082 **/
+  SimpleTest.waitForExplicitFinish();
+
+  function preventDefault(event) {
+    event.preventDefault();
+  }
+
+  function test() {
+    document.getElementById("keydown").addEventListener("keydown", preventDefault);
+    document.getElementById("keypress").addEventListener("keypress", preventDefault);
+
+    SimpleTest.waitForFocus(function() {
+      var testData = [ "one", "two", "three", "four", "keydown", "keypress" ];
+
+      // The order of the keys in otherKeys is important for the test to function properly.
+      var otherKeys = [ "DOWN", "UP", "RIGHT", "LEFT", "PAGE_DOWN", "PAGE_UP",
+                        "END", "HOME" ];
+
+      testData.forEach(function(id) {
+        var element = document.getElementById(id);
+        element.focus();
+        var previousValue = element.value;
+        sendChar('2');
+        is(element.value, previousValue, "value should not have changed");
+        previousValue = element.value;
+        otherKeys.forEach(function(key) {
+          sendKey(key);
+          isnot(element.value, previousValue, "value should have changed while testing key " + key);
+          previousValue = element.value;
+          });
+        });
+      SimpleTest.finish();
+    });
+  }
+</script>
+</head>
+<body onload="test();">
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=291082">Mozilla Bug 291082</a>
+<div>
+  <ul>
+    <li>
+    <select id="one" onkeydown="event.preventDefault();">
+      <option>0</option>
+      <option>1</option>
+      <option>2</option>
+    </select>
+    select onkeydown="event.preventDefault();"
+    </li>
+    <li>
+    <select id="two" onkeypress="event.preventDefault();">
+      <option>0</option>
+      <option>1</option>
+      <option>2</option>
+    </select>
+    select onkeypress="event.preventDefault();"
+    </li>
+    <li onkeydown="event.preventDefault();">
+    <select id="three">
+      <option>0</option>
+      <option>1</option>
+      <option>2</option>
+    </select>
+    li onkeydown="event.preventDefault();"
+    </li>
+    <li onkeypress="event.preventDefault();">
+    <select id="four">
+      <option>0</option>
+      <option>1</option>
+      <option>2</option>
+    </select>
+    li onkeypress="event.preventDefault();"
+    </li>
+    <li>
+    <select id="keydown">
+      <option>0</option>
+      <option>1</option>
+      <option>2</option>
+    </select>
+    select.addEventListener("keydown", function(event) { event.preventDefault(); });
+    </li>
+    <li>
+    <select id="keypress">
+      <option>0</option>
+      <option>1</option>
+      <option>2</option>
+      <option>9</option>
+    </select>
+    select.addEventListener("keypress", function(event) { event.preventDefault(); });
+    </li>
+  </ul>
+</div>
+<pre id="test">
+</pre>
+</body>
+</html>