Bug 1512411 - Don't expose broken label children accessibles on a XUL toolbarbutton which specifies the label attribute. r=Jamie, a=RyanVM
authorMarco Zehe <mzehe@mozilla.com>
Fri, 14 Dec 2018 08:20:02 +0000
changeset 509040 d2c26e2f11646f98fa28c8047b9d98b0e97b7399
parent 509039 3ebfb70bbd0bd65cb4ab8b7ed1bb3a4e43af3fa6
child 509041 5cf22fd4b3c0109d9e4f17081f9ee13e0a8025ad
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersJamie, RyanVM
bugs1512411, 1507365
milestone65.0
Bug 1512411 - Don't expose broken label children accessibles on a XUL toolbarbutton which specifies the label attribute. r=Jamie, a=RyanVM In bug 1507365, we introduced the ability of nested label children of toolbar buttons to provide the accessible name for a xul:toolbarbutton element. However, the XBL of xul:toolbarbutton causes the creation of a label accessible even if no xul:label child is present, and only the label attribute is being used. The nsIAccessibleText interface on such labels is, however, completely broken. This causes NVDA's mouse tracking, which always uses the deepest nested child, to fail. As a result, when hovering over Hamburger menu items, the announcement would be wrong, e. g., lag one behind the actual button the mouse is hovering over. To fix this, we only accept xul:label children if they come from real XUL markup, not from the label attribute and the XBL creating the label. This means that some of the test changes from bug 1507365 have to be reverted to accommodate the new old behavior. But the new test for an actual label child still passes. Differential Revision: https://phabricator.services.mozilla.com/D14469
accessible/tests/mochitest/tree/test_button.xul
accessible/tests/mochitest/tree/test_formctrl.xul
accessible/tests/mochitest/treeupdate/test_menubutton.xul
accessible/xul/XULFormControlAccessible.cpp
--- a/accessible/tests/mochitest/tree/test_button.xul
+++ b/accessible/tests/mochitest/tree/test_button.xul
@@ -32,20 +32,17 @@
       testAccessibleTree("button1", accTree);
 
       //////////////////////////////////////////////////////////////////////////
       // toolbarbutton
 
       var accTree = {
         role: ROLE_PUSHBUTTON,
         name: "hello",
-        children: [ {
-          role: ROLE_LABEL,
-          name: "hello"
-        } ]
+        children: [ ]
       };
       testAccessibleTree("button2", accTree);
 
       SimpleTest.finish()
     }
 
     SimpleTest.waitForExplicitFinish();
     addA11yLoadEvent(doTest);
--- a/accessible/tests/mochitest/tree/test_formctrl.xul
+++ b/accessible/tests/mochitest/tree/test_formctrl.xul
@@ -57,42 +57,32 @@
       // toolbar
       accTree = {
         role: ROLE_TOOLBAR,
         name: "My toolbar",
         children: [
           {
             role: ROLE_PUSHBUTTON,
             name: "hello",
-            children: [
-              {
-                role: ROLE_LABEL,
-                name: "hello"
-              }
-            ]
+            children: [ ]
           }
         ]
       };
 
       testAccessibleTree("toolbar", accTree);
 
       // toolbar
       accTree = {
         role: ROLE_TOOLBAR,
         name: "My second toolbar",
         children: [
           {
             role: ROLE_PUSHBUTTON,
             name: "hello",
-            children: [
-              {
-                role: ROLE_LABEL,
-                name: "hello"
-              }
-            ]
+            children: [ ]
           }
         ]
       };
 
       testAccessibleTree("toolbar2", accTree);
 
       if (!SEAMONKEY)
         testAccessibleTree("tb_customizable", { TOOLBAR: [] });
--- a/accessible/tests/mochitest/treeupdate/test_menubutton.xul
+++ b/accessible/tests/mochitest/treeupdate/test_menubutton.xul
@@ -30,59 +30,34 @@
       this.button = getAccessible(aButtonID);
       this.menupopup = this.button.firstChild;
 
       var checker = new invokerChecker(EVENT_REORDER, this.menupopup);
       this.__proto__ = new synthClick(aButtonID, checker);
 
       this.invoke = function openMenu_invoke()
       {
-        var tree = {};
-        if (this.button.name === "toolbarbutton") {
-          tree = 
-            { PUSHBUTTON: [
-              { MENUPOPUP: [ ] },
-              { LABEL: [
-                { TEXT_LEAF: [ ] }
-              ] }
-            ] };
-        } else {
-          tree = 
-            { PUSHBUTTON: [
-              { MENUPOPUP: [ ] }
-            ] };     
-        }
+        var tree =
+          { PUSHBUTTON: [
+            { MENUPOPUP: [ ] }
+          ] };
         testAccessibleTree(this.button, tree);
 
         this.__proto__.invoke();
       }
 
       this.finalCheck = function openMenu_finalCheck()
       {
-        var tree = {};
-        if (this.button.name === "toolbarbutton") {
-          tree =
-            { PUSHBUTTON: [
-              { MENUPOPUP: [
-                { role: menuItemRole, children: [ ] },
-                { role: menuItemRole, children: [ ] }
-              ] },
-              { LABEL: [
-                { TEXT_LEAF: [ ] }
-              ] }
-            ] };
-        } else {
-          tree =
-            { PUSHBUTTON: [
-              { MENUPOPUP: [
-                { role: menuItemRole, children: [ ] },
-                { role: menuItemRole, children: [ ] }
-              ] }
-            ] };
-        }
+        var tree =
+          { PUSHBUTTON: [
+            { MENUPOPUP: [
+              { role: menuItemRole, children: [ ] },
+              { role: menuItemRole, children: [ ] }
+            ] }
+          ] };
         testAccessibleTree(this.button, tree);
 
         synthesizeKey("KEY_Escape");
       }
 
       this.getID = function openMenu_getID()
       {
         return "open menu of the button " + prettyName(aButtonID);
--- a/accessible/xul/XULFormControlAccessible.cpp
+++ b/accessible/xul/XULFormControlAccessible.cpp
@@ -361,19 +361,21 @@ bool XULToolbarButtonAccessible::IsSepar
 
 ////////////////////////////////////////////////////////////////////////////////
 // XULToolbarButtonAccessible: Widgets
 
 bool XULToolbarButtonAccessible::IsAcceptableChild(nsIContent* aEl) const {
   // In general XUL button has not accessible children. Nevertheless menu
   // buttons can have popup accessibles (@type="menu" or columnpicker).
   // Also: Toolbar buttons can have labels as children.
+  // But only if the label attribute is not present.
   return aEl->IsXULElement(nsGkAtoms::menupopup) ||
          aEl->IsXULElement(nsGkAtoms::popup) ||
-         aEl->IsXULElement(nsGkAtoms::label);
+         (aEl->IsXULElement(nsGkAtoms::label) &&
+          !mContent->AsElement()->HasAttr(kNameSpaceID_None, nsGkAtoms::label));
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // XULToolbarAccessible
 ////////////////////////////////////////////////////////////////////////////////
 
 XULToolbarAccessible::XULToolbarAccessible(nsIContent* aContent,
                                            DocAccessible* aDoc)