Bug 1572677: Correct a11y exposure for HTML elements with associated XUL labels. r=MarcoZ a=pascalc
authorJames Teh <jteh@mozilla.com>
Thu, 28 Nov 2019 06:36:04 +0000
changeset 563495 b4717df2d39ff77a9d991d08657049d30494f0e4
parent 563494 9aaa2cc3ad5d439793ad13b24ae06786cbf82bf8
child 563496 c65d4278139f08b1b18e602c3453f62c74bc53c9
push id2202
push usercbrindusan@mozilla.com
push dateThu, 28 Nov 2019 17:50:36 +0000
treeherdermozilla-release@c65d4278139f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMarcoZ, pascalc
bugs1572677
milestone71.0
Bug 1572677: Correct a11y exposure for HTML elements with associated XUL labels. r=MarcoZ a=pascalc A XUL <label control="id"> can refer to an HTML element. Keyboard and mouse support for this already works, but previously, this wasn't being exposed via accessibility APIs. 1. Split the code to get the name from an associated XUL label out of Accessible::XULElmName into a new function Accessible::NameFromAssociatedXULLabel. 2. Use NameFromAssociatedXULLabel for HTMl elements. 3. Update AccessKey and RelationByType to support HTML elements with associated XUL labels. 4. Rename accessible/tests/mochitest/actions/test_keys_menu.xhtml to test_keys.xhtml so it can cover accessKey outside of menus. 5. Add tests. Differential Revision: https://phabricator.services.mozilla.com/D55057
accessible/generic/Accessible.cpp
accessible/generic/Accessible.h
accessible/tests/mochitest/actions/a11y.ini
accessible/tests/mochitest/actions/test_keys.xhtml
accessible/tests/mochitest/actions/test_keys_menu.xhtml
accessible/tests/mochitest/name/test_general.xhtml
accessible/tests/mochitest/relations/test_general.xhtml
--- a/accessible/generic/Accessible.cpp
+++ b/accessible/generic/Accessible.cpp
@@ -225,18 +225,18 @@ KeyBinding Accessible::AccessKey() const
 
     // Copy access key from label node.
     if (mContent->IsHTMLElement()) {
       // Unless it is labeled via an ancestor <label>, in which case that would
       // be redundant.
       HTMLLabelIterator iter(Document(), this,
                              HTMLLabelIterator::eSkipAncestorLabel);
       label = iter.Next();
-
-    } else if (mContent->IsXULElement()) {
+    }
+    if (!label) {
       XULLabelIterator iter(Document(), mContent);
       label = iter.Next();
     }
 
     if (label) key = nsCoreUtils::GetAccessKeyFor(label->GetContent());
   }
 
   if (!key) return KeyBinding();
@@ -748,16 +748,31 @@ void Accessible::TakeFocus() const {
     dom::AutoHandlingUserInputStatePusher inputStatePusher(true);
     // XXXbz: Can we actually have a non-element content here?
     RefPtr<Element> element =
         focusContent->IsElement() ? focusContent->AsElement() : nullptr;
     fm->SetFocus(element, 0);
   }
 }
 
+void Accessible::NameFromAssociatedXULLabel(DocAccessible* aDocument,
+                                            nsIContent* aElm, nsString& aName) {
+  Accessible* label = nullptr;
+  XULLabelIterator iter(aDocument, aElm);
+  while ((label = iter.Next())) {
+    // Check if label's value attribute is used
+    label->Elm()->GetAttr(kNameSpaceID_None, nsGkAtoms::value, aName);
+    if (aName.IsEmpty()) {
+      // If no value attribute, a non-empty label must contain
+      // children that define its text -- possibly using HTML
+      nsTextEquivUtils::AppendTextEquivFromContent(label, label->Elm(), &aName);
+    }
+  }
+}
+
 void Accessible::XULElmName(DocAccessible* aDocument, nsIContent* aElm,
                             nsString& aName) {
   /**
    * 3 main cases for XUL Controls to be labeled
    *   1 - control contains label="foo"
    *   2 - control has, as a child, a label element
    *        - label has either value="foo" or children
    *   3 - non-child label contains control="controlID"
@@ -781,28 +796,17 @@ void Accessible::XULElmName(DocAccessibl
     if (!select) {
       aElm->AsElement()->GetAttr(kNameSpaceID_None, nsGkAtoms::label, aName);
     }
   }
 
   // CASES #2 and #3 ------ label as a child or <label control="id" ... >
   // </label>
   if (aName.IsEmpty()) {
-    Accessible* label = nullptr;
-    XULLabelIterator iter(aDocument, aElm);
-    while ((label = iter.Next())) {
-      // Check if label's value attribute is used
-      label->Elm()->GetAttr(kNameSpaceID_None, nsGkAtoms::value, aName);
-      if (aName.IsEmpty()) {
-        // If no value attribute, a non-empty label must contain
-        // children that define its text -- possibly using HTML
-        nsTextEquivUtils::AppendTextEquivFromContent(label, label->Elm(),
-                                                     &aName);
-      }
-    }
+    NameFromAssociatedXULLabel(aDocument, aElm, aName);
   }
 
   aName.CompressWhitespace();
 }
 
 nsresult Accessible::HandleAccEvent(AccEvent* aEvent) {
   NS_ENSURE_ARG_POINTER(aEvent);
 
@@ -1615,19 +1619,18 @@ Relation Accessible::RelationByType(Rela
   // Relationships are defined on the same content node that the role would be
   // defined on.
   switch (aType) {
     case RelationType::LABELLED_BY: {
       Relation rel(
           new IDRefsIterator(mDoc, mContent, nsGkAtoms::aria_labelledby));
       if (mContent->IsHTMLElement()) {
         rel.AppendIter(new HTMLLabelIterator(Document(), this));
-      } else if (mContent->IsXULElement()) {
-        rel.AppendIter(new XULLabelIterator(Document(), mContent));
       }
+      rel.AppendIter(new XULLabelIterator(Document(), mContent));
 
       return rel;
     }
 
     case RelationType::LABEL_FOR: {
       Relation rel(new RelatedAccIterator(Document(), mContent,
                                           nsGkAtoms::aria_labelledby));
       if (mContent->IsXULElement(nsGkAtoms::label))
@@ -1974,16 +1977,21 @@ ENameValueFlag Accessible::NativeName(ns
     while ((label = iter.Next())) {
       nsTextEquivUtils::AppendTextEquivFromContent(this, label->GetContent(),
                                                    &aName);
       aName.CompressWhitespace();
     }
 
     if (!aName.IsEmpty()) return eNameOK;
 
+    NameFromAssociatedXULLabel(mDoc, mContent, aName);
+    if (!aName.IsEmpty()) {
+      return eNameOK;
+    }
+
     nsTextEquivUtils::GetNameFromSubtree(this, aName);
     return aName.IsEmpty() ? eNameOK : eNameFromSubtree;
   }
 
   if (mContent->IsXULElement()) {
     XULElmName(mDoc, mContent, aName);
     if (!aName.IsEmpty()) return eNameOK;
 
--- a/accessible/generic/Accessible.h
+++ b/accessible/generic/Accessible.h
@@ -1071,16 +1071,23 @@ class Accessible : public nsISupports {
   // Name helpers
 
   /**
    * Returns the accessible name specified by ARIA.
    */
   void ARIAName(nsString& aName) const;
 
   /**
+   * Returns the accessible name specified for this control using XUL
+   * <label control="id" ...>.
+   */
+  static void NameFromAssociatedXULLabel(DocAccessible* aDocument,
+                                         nsIContent* aElm, nsString& aName);
+
+  /**
    * Return the name for XUL element.
    */
   static void XULElmName(DocAccessible* aDocument, nsIContent* aElm,
                          nsString& aName);
 
   // helper method to verify frames
   static nsresult GetFullKeyName(const nsAString& aModifierName,
                                  const nsAString& aKeyName,
--- a/accessible/tests/mochitest/actions/a11y.ini
+++ b/accessible/tests/mochitest/actions/a11y.ini
@@ -4,14 +4,14 @@ support-files =
   !/dom/media/test/bug461281.ogg
 
 [test_anchors.html]
 [test_aria.html]
 [test_controls.html]
 [test_general.html]
 [test_general.xhtml]
 [test_keys.html]
-[test_keys_menu.xhtml]
+[test_keys.xhtml]
 [test_link.html]
 [test_media.html]
 [test_select.html]
 [test_tree.xhtml]
 [test_treegrid.xhtml]
rename from accessible/tests/mochitest/actions/test_keys_menu.xhtml
rename to accessible/tests/mochitest/actions/test_keys.xhtml
--- a/accessible/tests/mochitest/actions/test_keys_menu.xhtml
+++ b/accessible/tests/mochitest/actions/test_keys.xhtml
@@ -1,29 +1,30 @@
 <?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"?>
 
 <window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
+        xmlns:html="http://www.w3.org/1999/xhtml"
         title="Accessible XUL access keys and shortcut keys tests">
 
   <script src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js" />
 
   <script type="application/javascript"
           src="../common.js" />
   <script type="application/javascript"
           src="../events.js" />
 
   <script type="application/javascript">
   <![CDATA[
     function openMenu(aMenuID, aMenuitemID)
     {
-      this.menuNode = getNode(aMenuID),
-      this.menuitemNode = getNode(aMenuitemID),
+      this.menuNode = getNode(aMenuID);
+      this.menuitemNode = getNode(aMenuitemID);
 
       this.eventSeq = [
         new invokerChecker(EVENT_FOCUS, this.menuNode)
       ];
 
       this.invoke = function openMenu_invoke()
       {
         // Show menu.
@@ -48,16 +49,21 @@
         return "menuitem accesskey and shortcut test " +
           prettyName(this.menuItemNode);
       }
     }
 
     var gQueue = null;
     function doTest()
     {
+      // HTML element should get accessKey from associated XUL label.
+      let input = getAccessible("input");
+      is(input.accessKey, (MAC ? "⌃⌥i" : "Alt+Shift+i"),
+         "Wrong accessKey on input");
+
       gQueue = new eventQueue();
       gQueue.push(new openMenu("menu", "menuitem"));
       gQueue.invoke(); // Will call SimpleTest.finish();
     }
 
     SimpleTest.waitForExplicitFinish();
     addA11yLoadEvent(doTest);
   ]]>
@@ -73,16 +79,19 @@
       <p id="display"></p>
       <div id="content" style="display: none">
       </div>
       <pre id="test">
       </pre>
     </body>
 
     <vbox flex="1">
+      <label control="input" accesskey="i">input</label>
+      <html:input id="input"/>
+
       <keyset>
         <key key="l" modifiers="control" id="key1"/>
       </keyset>
 
       <menubar>
         <menu label="menu" id="menu" accesskey="u">
           <menupopup>
             <menuitem accesskey="p" key="key1" label="item1" id="menuitem"/>
--- a/accessible/tests/mochitest/name/test_general.xhtml
+++ b/accessible/tests/mochitest/name/test_general.xhtml
@@ -95,16 +95,19 @@
       testName("btn_label_2", "label2");
 
       // The label and button are siblings.
       testName("btn_label_3", "label3");
 
       // Multiple labels for single button: XUL button takes the last one.
       testName("btn_label_4", "label5");
 
+      // Label associated with HTML element.
+      testName("input_label", "input label");
+
 
       //////////////////////////////////////////////////////////////////////////
       // tooltiptext (if nothing above isn't presented then tooltiptext is used)
       testName("box_tooltiptext", "tooltiptext label");
 
 
       //////////////////////////////////////////////////////////////////////////
       // Name from the @title attribute of <toolbaritem/> (original bug 237249).
@@ -262,16 +265,19 @@
       </box>
     </box>
     <label control="btn_label_3">label3</label>
     <button id="btn_label_3"/>
 
     <label control="btn_label_4">label4</label>
     <label control="btn_label_4">label5</label>
     <button id="btn_label_4"/>
+
+    <label control="input_label">input label</label>
+    <html:input id="input_label"/>
   </hbox>
 
   <!-- tooltiptext -->
   <box id="box_tooltiptext"
        role="group"
        tooltiptext="tooltiptext label"/>
 
   <!-- the name from @title of toolbaritem -->
--- a/accessible/tests/mochitest/relations/test_general.xhtml
+++ b/accessible/tests/mochitest/relations/test_general.xhtml
@@ -1,14 +1,15 @@
 <?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"?>
 
 <window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
+        xmlns:html="http://www.w3.org/1999/xhtml"
         title="nsIAccessible::getAccessibleRelated() tests">
 
   <script src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js" />
 
   <script type="application/javascript"
           src="../common.js" />
   <script type="application/javascript"
           src="../relations.js" />
@@ -33,16 +34,20 @@
       testRelation("label2", RELATION_LABEL_FOR, "checkbox2");
       testRelation("checkbox2", RELATION_LABELLED_BY, "label2");
 
       // aria-labelledby, multiple relations
       testRelation("label3", RELATION_LABEL_FOR, "checkbox3");
       testRelation("label4", RELATION_LABEL_FOR, "checkbox3");
       testRelation("checkbox3", RELATION_LABELLED_BY, ["label3", "label4"]);
 
+      // xul:label@control referring to HTML element
+      testRelation("label_input", RELATION_LABEL_FOR, "input");
+      testRelation("input", RELATION_LABELLED_BY, "label_input");
+
       // aria-describedby
       testRelation("descr1", RELATION_DESCRIPTION_FOR, "checkbox4");
       testRelation("checkbox4", RELATION_DESCRIBED_BY, "descr1");
 
       // aria-describedby, multiple relations
       testRelation("descr2", RELATION_DESCRIPTION_FOR, "checkbox5");
       testRelation("descr3", RELATION_DESCRIPTION_FOR, "checkbox5");
       testRelation("checkbox5", RELATION_DESCRIBED_BY, ["descr2", "descr3"]);
@@ -153,16 +158,19 @@
     <description id="label2">label</description>
     <description role="checkbox" id="checkbox2" aria-labelledby="label2"/>
 
     <description id="label3">label</description>
     <description id="label4">label</description>
     <description role="checkbox" id="checkbox3"
                  aria-labelledby="label3 label4"/>
 
+    <label id="label_input" control="input">label</label>
+    <html:input id="input"/>
+
     <description id="descr1">description</description>
     <description role="checkbox" id="checkbox4" aria-describedby="descr1"/>
 
     <description id="descr2">label</description>
     <description id="descr3">label</description>
     <description role="checkbox" id="checkbox5"
                  aria-describedby="descr2 descr3"/>