Bug 792324 - access keys are fired for non-open popups. r=enndeakin
authorMark Hammond <mhammond@skippinet.com.au>
Fri, 28 Sep 2012 15:04:57 +1000
changeset 108492 eb7c07373996caf4a69414214d492f9cd00ca62e
parent 108491 a0c8f93e588c04a59eb5affbe6d84850e7446369
child 108493 358ade692d25dd072f379339758075929d2b9bb3
push id82
push usershu@rfrn.org
push dateFri, 05 Oct 2012 13:20:22 +0000
reviewersenndeakin
bugs792324
milestone18.0a1
Bug 792324 - access keys are fired for non-open popups. r=enndeakin
toolkit/content/tests/chrome/Makefile.in
toolkit/content/tests/chrome/test_bug792324.xul
toolkit/content/widgets/button.xml
--- a/toolkit/content/tests/chrome/Makefile.in
+++ b/toolkit/content/tests/chrome/Makefile.in
@@ -84,16 +84,17 @@ MOCHITEST_CHROME_FILES = 	findbar_window
 		test_bug382990.xul \
 		test_bug457632.xul \
 		test_bug460942.xul \
 		test_bug509732.xul \
 		test_bug554279.xul \
 		test_bug557987.xul\
 		test_bug562554.xul \
 		test_bug585946.xul \
+		test_bug792324.xul \
 		test_button.xul \
 		test_closemenu_attribute.xul \
 		test_colorpicker_popup.xul \
 		test_menulist.xul \
 		test_menuitem_blink.xul \
 		test_menulist_keynav.xul \
 		test_popup_coords.xul \
 		test_popup_recreate.xul \
new file mode 100644
--- /dev/null
+++ b/toolkit/content/tests/chrome/test_bug792324.xul
@@ -0,0 +1,76 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin" type="text/css"?>
+<?xml-stylesheet href="chrome://mochikit/content/tests/SimpleTest/test.css" type="text/css"?>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=792324
+-->
+<window title="Mozilla Bug 792324"
+  xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+
+  <title>Test for Bug 792324</title>
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"/>
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"/>
+<body  xmlns="http://www.w3.org/1999/xhtml">
+  <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=792324">Mozilla Bug 792324</a>
+
+  <p id="display"></p>
+<div id="content" style="display: none">
+</div>
+</body>
+
+<panel id="panel-1">
+  <button label="just a normal button"/>
+  <button id="button-1"
+          accesskey="X"
+          oncommand="clicked(event)"
+          label="Button in panel 1"
+  />
+</panel>
+
+<panel id="panel-2">
+  <button label="just a normal button"/>
+  <button id="button-2"
+          accesskey="X"
+          oncommand="clicked(event)"
+          label="Button in panel 2"
+  />
+</panel>
+
+<script class="testbody" type="application/javascript;version=1.7"><![CDATA[
+
+/** Test for Bug 792324 **/
+let after_click;
+
+function clicked(event) {
+  after_click(event);
+}
+
+function checkAccessKeyOnPanel(panelid, buttonid, cb) {
+  let panel = document.getElementById(panelid);
+  panel.addEventListener("popupshown", function onpopupshown() {
+    panel.removeEventListener("popupshown", onpopupshown);
+    panel.firstChild.focus();
+    after_click = function(event) {
+      is(event.target.id, buttonid, "Accesskey was directed to the button '" + buttonid + "'");
+      panel.hidePopup();
+      cb();
+    }
+    synthesizeKey("X", {});
+  });
+ panel.openPopup(null, "", 100, 100, false, false);
+}
+
+function test() {
+  checkAccessKeyOnPanel("panel-1", "button-1", function() {
+    checkAccessKeyOnPanel("panel-2", "button-2", function() {
+      SimpleTest.finish();
+    });
+  });
+}
+
+SimpleTest.waitForExplicitFinish();
+SimpleTest.waitForFocus(test, window);
+
+]]></script>
+
+</window>
--- a/toolkit/content/widgets/button.xml
+++ b/toolkit/content/widgets/button.xml
@@ -84,18 +84,27 @@
       <property name="autoCheck"
                 onget="return this.getAttribute('autoCheck') == 'true';"
                 onset="this.setAttribute('autoCheck', val); return val;"/>
 
       <method name ="filterButtons">
         <parameter name="node"/>
         <body>
         <![CDATA[
-          if (node.localName == "button" && node.accessKey &&
-            !node.disabled && !node.collapsed && !node.hidden)
+          // if the node isn't visible, don't descend into it.
+          var cs = node.ownerDocument.defaultView.getComputedStyle(node, null);
+          if (cs.visibility != "visible" || cs.display == "none") {
+            return NodeFilter.FILTER_REJECT;
+          }
+          // but it may be a popup element, in which case we look at "state"...
+          if (cs.display == "-moz-popup" && node.state != "open") {
+            return NodeFilter.FILTER_REJECT;
+          }
+          // OK - the node seems visible, so it is a candidate.
+          if (node.localName == "button" && node.accessKey && !node.disabled)
             return NodeFilter.FILTER_ACCEPT;
           return NodeFilter.FILTER_SKIP;
         ]]>
         </body>
       </method>
 
       <method name="fireAccessKeyButton">
         <parameter name="aSubtree"/>