Bug 1582409 - ensure that we handle SVGs with role=img correctly when running text-label checks. r=nchevobbe
authorYura Zenevich <yura.zenevich@gmail.com>
Thu, 05 Dec 2019 16:21:23 +0000
changeset 505662 b2df27e3e97ad4d8af07aa5eb457a8d8b4323759
parent 505661 6ce1fcebc7cd977b0ee8b9042933825ce5de5d62
child 505663 d62eb73733cca86bb925766fe9d3b1c652158e4c
push id36885
push usernbeleuzu@mozilla.com
push dateThu, 05 Dec 2019 21:53:30 +0000
treeherdermozilla-central@ed92ceb9cb68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnchevobbe
bugs1582409
milestone73.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 1582409 - ensure that we handle SVGs with role=img correctly when running text-label checks. r=nchevobbe Differential Revision: https://phabricator.services.mozilla.com/D55976
devtools/server/actors/accessibility/audit/keyboard.js
devtools/server/actors/accessibility/audit/text-label.js
devtools/server/actors/utils/accessibility.js
devtools/server/tests/browser/browser_accessibility_text_label_audit.js
devtools/server/tests/browser/doc_accessibility_text_label_audit.html
--- a/devtools/server/actors/accessibility/audit/keyboard.js
+++ b/devtools/server/actors/accessibility/audit/keyboard.js
@@ -25,16 +25,23 @@ loader.lazyRequireGetter(
 );
 loader.lazyRequireGetter(
   this,
   "isDefunct",
   "devtools/server/actors/utils/accessibility",
   true
 );
 
+loader.lazyRequireGetter(
+  this,
+  "getAriaRoles",
+  "devtools/server/actors/utils/accessibility",
+  true
+);
+
 const {
   accessibility: {
     AUDIT_TYPE: { KEYBOARD },
     ISSUE_TYPE: {
       [KEYBOARD]: {
         FOCUSABLE_NO_SEMANTICS,
         FOCUSABLE_POSITIVE_TABINDEX,
         INTERACTIVE_NO_ACTION,
@@ -123,37 +130,16 @@ function isInvalidNode(node) {
     !node ||
     Cu.isDeadWrapper(node) ||
     node.nodeType !== nodeConstants.ELEMENT_NODE ||
     !node.ownerGlobal
   );
 }
 
 /**
- * Get role attribute for an accessible object if specified for its
- * corresponding DOMNode.
- *
- * @param   {nsIAccessible} accessible
- *          Accessible for which to determine its role attribute value.
- *
- * @returns {null|String}
- *          Role attribute value if specified.
- */
-function getAriaRoles(accessible) {
-  try {
-    return accessible.attributes.getStringProperty("xml-roles");
-  } catch (e) {
-    // No xml-roles. nsPersistentProperties throws if the attribute for a key
-    // is not found.
-  }
-
-  return null;
-}
-
-/**
  * Determine if accessible is focusable with the keyboard.
  *
  * @param   {nsIAccessible} accessible
  *          Accessible for which to determine if it is keyboard focusable.
  *
  * @returns {Boolean}
  *          True if focusable with the keyboard.
  */
--- a/devtools/server/actors/accessibility/audit/text-label.js
+++ b/devtools/server/actors/accessibility/audit/text-label.js
@@ -1,15 +1,23 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 const { Ci } = require("chrome");
+
+loader.lazyRequireGetter(
+  this,
+  "getAriaRoles",
+  "devtools/server/actors/utils/accessibility",
+  true
+);
+
 const {
   accessibility: {
     AUDIT_TYPE: { TEXT_LABEL },
     ISSUE_TYPE,
     SCORES: { BEST_PRACTICES, FAIL, WARNING },
   },
 } = require("devtools/shared/constants");
 
@@ -132,16 +140,33 @@ const dialogRule = shouldHaveNonEmptyNam
  * "").
  *
  * @returns {null|Object}
  *          Failure audit report if interactive accessible object has no name,
  *          null otherwise.
  */
 const imageRule = function(accessible) {
   const name = getAccessibleName(accessible);
+  const { DOMNode } = accessible;
+  if (
+    DOMNode instanceof DOMNode.ownerGlobal.SVGElement &&
+    DOMNode.ownerSVGElement
+  ) {
+    let ownerSVGAccessible = accessible.parent;
+    while (ownerSVGAccessible.DOMNode.ownerSVGElement) {
+      ownerSVGAccessible = ownerSVGAccessible.parent;
+    }
+
+    const ariaRoles = getAriaRoles(ownerSVGAccessible);
+    if (ariaRoles && ariaRoles.includes("img")) {
+      // Do not require a defined name if a wrapping SVG has a role="img".
+      return null;
+    }
+  }
+
   return name != null ? null : { score: FAIL, issue: IMAGE_NO_NAME };
 };
 
 /**
  * A text label rule for accessible objects that correspond to form elements.
  * These objects must have a non-empty name and must have a visible label.
  *
  * @returns {null|Object}
--- a/devtools/server/actors/utils/accessibility.js
+++ b/devtools/server/actors/utils/accessibility.js
@@ -98,12 +98,34 @@ function isWebRenderEnabled(win) {
     // Sometimes nsIDOMWindowUtils::layerManagerType fails unexpectedly (see bug
     // 1596428).
     console.warn(e);
   }
 
   return false;
 }
 
+/**
+ * Get role attribute for an accessible object if specified for its
+ * corresponding DOMNode.
+ *
+ * @param   {nsIAccessible} accessible
+ *          Accessible for which to determine its role attribute value.
+ *
+ * @returns {null|String}
+ *          Role attribute value if specified.
+ */
+function getAriaRoles(accessible) {
+  try {
+    return accessible.attributes.getStringProperty("xml-roles");
+  } catch (e) {
+    // No xml-roles. nsPersistentProperties throws if the attribute for a key
+    // is not found.
+  }
+
+  return null;
+}
+
+exports.getAriaRoles = getAriaRoles;
 exports.isDefunct = isDefunct;
+exports.isWebRenderEnabled = isWebRenderEnabled;
 exports.loadSheetForBackgroundCalculation = loadSheetForBackgroundCalculation;
 exports.removeSheetForBackgroundCalculation = removeSheetForBackgroundCalculation;
-exports.isWebRenderEnabled = isWebRenderEnabled;
--- a/devtools/server/tests/browser/browser_accessibility_text_label_audit.js
+++ b/devtools/server/tests/browser/browser_accessibility_text_label_audit.js
@@ -1057,16 +1057,76 @@ add_task(async function() {
       { score: FAIL, issue: TOOLBAR_NO_NAME },
     ],
     [
       "Non-unique aAria toolbar with aria-labelledby an element with empty content",
       "#toolbar-3",
       { score: FAIL, issue: TOOLBAR_NO_NAME },
     ],
     ["Non-unique aria toolbar with aria-labelledby", "#toolbar-4", null],
+    ["SVGElement with role=img that has a title", "#svg-1", null],
+    [
+      "SVGElement with no name and with ownerSVGElement with role=img that has a title",
+      "#svg-2",
+      null,
+    ],
+    ["SVGElement without role=img that has a title", "#svg-3", null],
+    [
+      "SVGElement with no name and with ownerSVGElement without role=img",
+      "#svg-4",
+      { score: FAIL, issue: IMAGE_NO_NAME },
+    ],
+    [
+      "SVGElement with role=img and no name",
+      "#svg-5",
+      { score: FAIL, issue: IMAGE_NO_NAME },
+    ],
+    [
+      "SVGElement with no name and with ownerSVGElement with role=img",
+      "#svg-6",
+      null,
+    ],
+    [
+      "SVGElement with no name",
+      "#svg-7",
+      { score: FAIL, issue: IMAGE_NO_NAME },
+    ],
+    [
+      "SVGElement with no name and with ownerSVGElement with no name",
+      "#svg-8",
+      { score: FAIL, issue: IMAGE_NO_NAME },
+    ],
+    ["SVGElement with a name", "#svg-9", null],
+    [
+      "SVGElement with a name and with ownerSVGElement with a name",
+      "#svg-10",
+      null,
+    ],
+    ["SVGElement with a title", "#svg-11", null],
+    [
+      "SVGElement with a name and with ownerSVGElement with a title",
+      "#svg-12",
+      null,
+    ],
+    ["SVGElement with role=img that has a title", "#svg-13", null],
+    [
+      "SVGElement with a name and with ownerSVGElement with role=img that has a title",
+      "#svg-14",
+      null,
+    ],
+    [
+      "SVGElement with role=img and no title",
+      "#svg-15",
+      { score: FAIL, issue: IMAGE_NO_NAME },
+    ],
+    [
+      "SVGElement with a name and with ownerSVGElement with role=img and no title",
+      "#svg-16",
+      null,
+    ],
   ];
 
   for (const [description, selector, expected] of tests) {
     info(description);
     const node = await walker.querySelector(walker.rootNode, selector);
     const front = await a11yWalker.getAccessibleFor(node);
     const audit = await front.audit({ types: [TEXT_LABEL] });
     Assert.deepEqual(
--- a/devtools/server/tests/browser/doc_accessibility_text_label_audit.html
+++ b/devtools/server/tests/browser/doc_accessibility_text_label_audit.html
@@ -428,10 +428,38 @@
   <label for="togglebutton-17">Button: </label><div role="button" aria-pressed="true" id="togglebutton-17"></div>
   <label id="togglebutton-18-label">Button: </label><div role="button" aria-pressed="true" id="togglebutton-18" aria-labelledby="togglebutton-18-label"></div>
   <span id="toolbar-1" role="toolbar" aria-label="Toolbar"></span>
   <span id="toolbar-2" role="toolbar"></span>
   <p id="toolbar-3-label"></p>
   <span id="toolbar-3" role="toolbar" aria-labelledby="toolbar-3-label"></span>
   <p id="toolbar-4-label">Toolbar</p>
   <span id="toolbar-4" role="toolbar" aria-labelledby="toolbar-4-label"></span>
+  <svg id="svg-1" role="img" viewbox="0 0 100 10" height="10px">
+    <title id="siteLogoTitle">Site Logo</title>
+    <rect id="svg-2" x="0" y="00" width="100" height="10" fill="red"></rect>
+  </svg>
+  <svg id="svg-3" viewbox="0 0 100 10" height="10px">
+    <title id="siteLogoTitle">Site Logo</title>
+    <rect id="svg-4" x="0" y="00" width="100" height="10" fill="red"></rect>
+  </svg>
+  <svg id="svg-5" role="img" viewbox="0 0 100 10" height="10px">
+    <rect id="svg-6" x="0" y="00" width="100" height="10" fill="red"></rect>
+  </svg>
+  <svg id="svg-7" viewbox="0 0 100 10" height="10px">
+    <rect id="svg-8" x="0" y="00" width="100" height="10" fill="red"></rect>
+  </svg>
+  <svg id="svg-9" aria-label="foo" viewbox="0 0 100 10" height="10px">
+    <rect id="svg-10" aria-label="bar" x="0" y="00" width="100" height="10" fill="red"></rect>
+  </svg>
+  <svg id="svg-11" viewbox="0 0 100 10" height="10px">
+    <title id="siteLogoTitle">Site Logo</title>
+    <rect id="svg-12" aria-label="foo" x="0" y="00" width="100" height="10" fill="red"></rect>
+  </svg>
+  <svg id="svg-13" role="img" viewbox="0 0 100 10" height="10px">
+    <title id="siteLogoTitle">Site Logo</title>
+    <rect aria-label="foo" id="svg-14" x="0" y="00" width="100" height="10" fill="red"></rect>
+  </svg>
+  <svg id="svg-15" role="img" viewbox="0 0 100 10" height="10px">
+    <rect aria-label="foo" id="svg-16" x="0" y="00" width="100" height="10" fill="red"></rect>
+  </svg>
 </body>
 </html>