Bug 1541446 - added audited event to AccessibleActor/Front. r=pbro
☠☠ backed out by b9bbbfcdd693 ☠ ☠
authorYura Zenevich <yura.zenevich@gmail.com>
Wed, 10 Apr 2019 00:58:24 +0000
changeset 468715 9f43419c7b402fd90950749c1053669e92a1c7fd
parent 468714 e3294a1be947087cbf6eb48998326e1742c9ab8e
child 468716 a27b2d2038852219b9c9983702197630f2d3ad82
push id112747
push userncsoregi@mozilla.com
push dateWed, 10 Apr 2019 13:22:28 +0000
treeherdermozilla-inbound@06ea3fe18fda [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbro
bugs1541446
milestone68.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 1541446 - added audited event to AccessibleActor/Front. r=pbro Differential Revision: https://phabricator.services.mozilla.com/D26457
devtools/client/accessibility/components/Checks.js
devtools/client/accessibility/test/browser/browser_accessibility_sidebar_checks.js
devtools/server/actors/accessibility/accessible.js
devtools/server/actors/highlighters/utils/accessibility.js
devtools/server/tests/browser/browser.ini
devtools/server/tests/browser/browser_accessibility_node_audit.js
devtools/shared/constants.js
devtools/shared/fronts/accessibility.js
devtools/shared/moz.build
devtools/shared/specs/accessibility.js
--- a/devtools/client/accessibility/components/Checks.js
+++ b/devtools/client/accessibility/components/Checks.js
@@ -9,16 +9,18 @@ const { connect } = require("devtools/cl
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 const { div } = require("devtools/client/shared/vendor/react-dom-factories");
 
 const List = createFactory(require("devtools/client/shared/components/List").List);
 const ColorContrastCheck =
   createFactory(require("./ColorContrastAccessibility").ColorContrastCheck);
 const { L10N } = require("../utils/l10n");
 
+const { accessibility: { AUDIT_TYPE } } = require("devtools/shared/constants");
+
 function EmptyChecks() {
   return (
     div({
       className: "checks-empty",
       role: "presentation",
     }, L10N.getStr("accessibility.checks.empty"))
   );
 }
@@ -28,17 +30,17 @@ function EmptyChecks() {
 class Checks extends Component {
   static get propTypes() {
     return {
       audit: PropTypes.object,
       labelledby: PropTypes.string.isRequired,
     };
   }
 
-  contrastRatio(contrastRatio) {
+  [AUDIT_TYPE.CONTRAST](contrastRatio) {
     return ColorContrastCheck(contrastRatio);
   }
 
   render() {
     const { audit, labelledby } = this.props;
     if (!audit) {
       return EmptyChecks();
     }
--- a/devtools/client/accessibility/test/browser/browser_accessibility_sidebar_checks.js
+++ b/devtools/client/accessibility/test/browser/browser_accessibility_sidebar_checks.js
@@ -22,44 +22,44 @@ const TEST_URI = `<html>
  *   setup    {Function}  An optional setup that needs to be performed before
  *                        the state of the tree and the sidebar can be checked.
  *   expected {JSON}      An expected states for the tree and the sidebar.
  * }
  */
 const tests = [{
   desc: "Test the initial accessibility audit state.",
   expected: {
-    audit: { contrastRatio: null },
+    audit: { CONTRAST: null },
   },
 }, {
   desc: "Check accessible representing text node in red.",
   setup: async ({ doc }) => {
     await toggleRow(doc, 0);
     await toggleRow(doc, 1);
     await selectRow(doc, 2);
   },
   expected: {
     audit: {
-      "contrastRatio": {
+      "CONTRAST": {
         "value": 4.00,
         "color": [255, 0, 0, 1],
         "backgroundColor": [255, 255, 255, 1],
         "isLargeText": false,
       },
     },
   },
 }, {
   desc: "Check accessible representing text node in blue.",
   setup: async ({ doc }) => {
     await toggleRow(doc, 3);
     await selectRow(doc, 4);
   },
   expected: {
     audit: {
-      "contrastRatio": {
+      "CONTRAST": {
         "value": 8.59,
         "color": [0, 0, 255, 1],
         "backgroundColor": [255, 255, 255, 1],
         "isLargeText": false,
       },
     },
   },
 }];
--- a/devtools/server/actors/accessibility/accessible.js
+++ b/devtools/server/actors/accessibility/accessible.js
@@ -2,20 +2,22 @@
  * 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, Cu } = require("chrome");
 const { Actor, ActorClassWithSpec } = require("devtools/shared/protocol");
 const { accessibleSpec } = require("devtools/shared/specs/accessibility");
+const { accessibility: { AUDIT_TYPE } } = require("devtools/shared/constants");
 
 loader.lazyRequireGetter(this, "getContrastRatioFor", "devtools/server/actors/accessibility/contrast", true);
 loader.lazyRequireGetter(this, "isDefunct", "devtools/server/actors/utils/accessibility", true);
 loader.lazyRequireGetter(this, "findCssSelector", "devtools/shared/inspector/css-logic", true);
+loader.lazyRequireGetter(this, "events", "devtools/shared/event-emitter");
 
 const RELATIONS_TO_IGNORE = new Set([
   Ci.nsIAccessibleRelation.RELATION_CONTAINING_APPLICATION,
   Ci.nsIAccessibleRelation.RELATION_CONTAINING_TAB_PANE,
   Ci.nsIAccessibleRelation.RELATION_CONTAINING_WINDOW,
   Ci.nsIAccessibleRelation.RELATION_PARENT_WINDOW_OF,
   Ci.nsIAccessibleRelation.RELATION_SUBWINDOW_OF,
 ]);
@@ -150,16 +152,20 @@ const AccessibleActor = ActorClassWithSp
   },
 
   destroy() {
     Actor.prototype.destroy.call(this);
     this.walker = null;
     this.rawAccessible = null;
   },
 
+  get isDestroyed() {
+    return this.actorID == null;
+  },
+
   get role() {
     if (this.isDefunct) {
       return null;
     }
     return this.walker.a11yService.getStringRole(this.rawAccessible.role);
   },
 
   get name() {
@@ -364,16 +370,17 @@ const AccessibleActor = ActorClassWithSp
       description: this.description,
       keyboardShortcut: this.keyboardShortcut,
       childCount: this.childCount,
       domNodeType: this.domNodeType,
       indexInParent: this.indexInParent,
       states: this.states,
       actions: this.actions,
       attributes: this.attributes,
+      checks: this._lastAudit,
     };
   },
 
   _isValidTextLeaf(rawAccessible) {
     return !isDefunct(rawAccessible) &&
            TEXT_ROLES.has(rawAccessible.role) &&
            rawAccessible.name && rawAccessible.name.trim().length > 0;
   },
@@ -383,52 +390,66 @@ const AccessibleActor = ActorClassWithSp
    */
   async _getContrastRatio() {
     if (!this._isValidTextLeaf(this.rawAccessible)) {
       return null;
     }
 
     const { DOMNode: rawNode } = this.rawAccessible;
     const win = rawNode.ownerGlobal;
-    this.walker.clearStyles(win);
+    // Keep the reference to the walker actor in case the actor gets destroyed
+    // during the colour contrast ratio calculation.
+    const { walker } = this;
+    walker.clearStyles(win);
     const contrastRatio = await getContrastRatioFor(rawNode.parentNode, {
       bounds: this.bounds,
       win,
     });
 
-    this.walker.restoreStyles(win);
+    walker.restoreStyles(win);
 
     return contrastRatio;
   },
 
   /**
    * Audit the state of the accessible object.
    *
    * @return {Object|null}
    *         Audit results for the accessible object.
   */
-  async audit() {
+  audit() {
     if (this._auditing) {
       return this._auditing;
     }
 
     // More audit steps will be added here in the near future. In addition to
     // colour contrast ratio we will add autits for to the missing names,
     // invalid states, etc. (For example see bug 1518808).
     this._auditing = Promise.all([
       this._getContrastRatio(),
     ]).then(([
       contrastRatio,
     ]) => {
-      const audit = this.isDefunct ? null : {
-        contrastRatio,
-      };
+      let audit = null;
+      if (!this.isDefunct && !this.isDestroyed) {
+        audit = {
+          [AUDIT_TYPE.CONTRAST]: contrastRatio,
+        };
+        this._lastAudit = audit;
+        events.emit(this, "audited", audit);
+      }
 
+      return audit;
+    }).catch(error => {
+      if (!this.isDefunct && !this.isDestroyed) {
+        throw error;
+      }
+      return null;
+    }).finally(() => {
       this._auditing = null;
-      return audit;
     });
 
     return this._auditing;
   },
 
   snapshot() {
     return getSnapshot(this.rawAccessible, this.walker.a11yService);
   },
--- a/devtools/server/actors/highlighters/utils/accessibility.js
+++ b/devtools/server/actors/highlighters/utils/accessibility.js
@@ -8,16 +8,18 @@ const DevToolsUtils = require("devtools/
 const { getCurrentZoom, getViewportDimensions } = require("devtools/shared/layout/utils");
 const { moveInfobar, createNode } = require("./markup");
 const { truncateString } = require("devtools/shared/inspector/utils");
 
 const STRINGS_URI = "devtools/shared/locales/accessibility.properties";
 loader.lazyRequireGetter(this, "LocalizationHelper", "devtools/shared/l10n", true);
 DevToolsUtils.defineLazyGetter(this, "L10N", () => new LocalizationHelper(STRINGS_URI));
 
+const { accessibility: { AUDIT_TYPE } } = require("devtools/shared/constants");
+
 // Max string length for truncating accessible name values.
 const MAX_STRING_LENGTH = 50;
 
 /**
  * The AccessibleInfobar is a class responsible for creating the markup for the
  * accessible highlighter. It is also reponsible for updating content within the
  * infobar such as role and name values.
  */
@@ -537,17 +539,17 @@ class ContrastRatio extends AuditReport 
   /**
    * Update contrast ratio score infobar markup.
    * @param  {Number}
    *         Contrast ratio for an accessible object being highlighted.
    * @return {Boolean}
    *         True if the contrast ratio markup was updated correctly and infobar audit
    *         block should be visible.
    */
-  update({ contrastRatio }) {
+  update({ [AUDIT_TYPE.CONTRAST]: contrastRatio }) {
     const els = {};
     for (const key of ["label", "min", "max", "error", "separator"]) {
       const el = els[key] = this.getElement(`contrast-ratio-${key}`);
       if (["min", "max"].includes(key)) {
         ["fail", "AA", "AAA"].forEach(className => el.classList.remove(className));
         this.setTextContent(el, "");
       }
 
--- a/devtools/server/tests/browser/browser.ini
+++ b/devtools/server/tests/browser/browser.ini
@@ -37,16 +37,17 @@ support-files =
   !/devtools/client/shared/test/telemetry-test-helpers.js
   !/devtools/server/tests/mochitest/hello-actor.js
 
 [browser_accessibility_highlighter_infobar.js]
 skip-if = (os == 'win' && processor == 'aarch64') # bug 1533184
 [browser_accessibility_infobar_show.js]
 [browser_accessibility_node.js]
 skip-if = (os == 'win' && processor == 'aarch64') # bug 1533184
+[browser_accessibility_node_audit.js]
 [browser_accessibility_node_events.js]
 skip-if = (os == 'win' && processor == 'aarch64') # bug 1533184
 [browser_accessibility_simple.js]
 skip-if = (os == 'win' && processor == 'aarch64') # bug 1533184
 [browser_accessibility_walker.js]
 skip-if = (os == 'win' && processor == 'aarch64') # bug 1533487
 [browser_actor_error.js]
 [browser_animation_actor-lifetime.js]
new file mode 100644
--- /dev/null
+++ b/devtools/server/tests/browser/browser_accessibility_node_audit.js
@@ -0,0 +1,45 @@
+/* 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";
+
+/**
+ * Checks functionality around audit for the AccessibleActor. This includes
+ * tests for the return value when calling the audit method, payload of the
+ * corresponding event as well as the AccesibleFront state being up to date.
+ */
+
+add_task(async function() {
+  const {target, walker, accessibility} =
+    await initAccessibilityFrontForUrl(MAIN_DOMAIN + "doc_accessibility_infobar.html");
+
+  const a11yWalker = await accessibility.getWalker();
+  await accessibility.enable();
+  const buttonNode = await walker.querySelector(walker.rootNode, "#button");
+  const buttonFront = await a11yWalker.getAccessibleFor(buttonNode);
+  const [textLeafNode] = await buttonFront.children();
+
+  const onAudited = textLeafNode.once("audited");
+  const audit = await textLeafNode.audit();
+  const auditFromEvent = await onAudited;
+
+  const expectedAudit = {
+    "CONTRAST": {
+      "value": 21,
+      "color": [0, 0, 0, 1],
+      "backgroundColor": [255, 255, 255, 1],
+      "isLargeText": false,
+    },
+  };
+
+  Assert.deepEqual(audit, expectedAudit, "Audit results are correct.");
+  Assert.deepEqual(textLeafNode.checks, expectedAudit, "Checks are correct.");
+  Assert.deepEqual(auditFromEvent, expectedAudit,
+    "Audit results from event are correct.");
+
+  await accessibility.disable();
+  await waitForA11yShutdown();
+  await target.destroy();
+  gBrowser.removeCurrentTab();
+});
new file mode 100644
--- /dev/null
+++ b/devtools/shared/constants.js
@@ -0,0 +1,18 @@
+/* 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";
+
+/**
+ * Constants used in various panels, shared between client and the server.
+ */
+
+/* Accessibility Panel ====================================================== */
+
+exports.accessibility = {
+  // List of audit types.
+  AUDIT_TYPE: {
+    CONTRAST: "CONTRAST",
+  },
+};
--- a/devtools/shared/fronts/accessibility.js
+++ b/devtools/shared/fronts/accessibility.js
@@ -13,16 +13,17 @@ const {
   accessibilitySpec,
 } = require("devtools/shared/specs/accessibility");
 const events = require("devtools/shared/event-emitter");
 
 class AccessibleFront extends FrontClassWithSpec(accessibleSpec) {
   constructor(client) {
     super(client);
 
+    this.before("audited", this.audited.bind(this));
     this.before("name-change", this.nameChange.bind(this));
     this.before("value-change", this.valueChange.bind(this));
     this.before("description-change", this.descriptionChange.bind(this));
     this.before("shortcut-change", this.shortcutChange.bind(this));
     this.before("reorder", this.reorder.bind(this));
     this.before("text-change", this.textChange.bind(this));
     this.before("index-in-parent-change", this.indexInParentChange.bind(this));
     this.before("states-change", this.statesChange.bind(this));
@@ -73,16 +74,20 @@ class AccessibleFront extends FrontClass
   get actions() {
     return this._form.actions;
   }
 
   get attributes() {
     return this._form.attributes;
   }
 
+  get checks() {
+    return this._form.checks;
+  }
+
   form(form) {
     this.actorID = form.actor;
     this._form = form;
   }
 
   nameChange(name, parent, walker) {
     this._form.name = name;
     // Name change event affects the tree rendering, we fire this event on
@@ -131,16 +136,20 @@ class AccessibleFront extends FrontClass
 
   actionsChange(actions) {
     this._form.actions = actions;
   }
 
   attributesChange(attributes) {
     this._form.attributes = attributes;
   }
+
+  audited(checks) {
+    this._form.checks = checks;
+  }
 }
 
 class AccessibleWalkerFront extends FrontClassWithSpec(accessibleWalkerSpec) {
   constructor(client) {
     super(client);
     this.before("accessible-destroy", this.accessibleDestroy.bind(this));
   }
 
--- a/devtools/shared/moz.build
+++ b/devtools/shared/moz.build
@@ -42,16 +42,17 @@ XPCSHELL_TESTS_MANIFESTS += ['tests/unit
 
 JAR_MANIFESTS += ['jar.mn']
 
 DevToolsModules(
     'async-storage.js',
     'async-utils.js',
     'base-loader.js',
     'builtin-modules.js',
+    'constants.js',
     'content-observer.js',
     'debounce.js',
     'defer.js',
     'deprecated-sync-thenables.js',
     'DevToolsUtils.js',
     'dom-node-constants.js',
     'dom-node-filter-constants.js',
     'event-emitter.js',
--- a/devtools/shared/specs/accessibility.js
+++ b/devtools/shared/specs/accessibility.js
@@ -72,16 +72,20 @@ const accessibleSpec = generateActorSpec
     "text-change": {
       type: "textChange",
       walker: Arg(0, "nullable:accessiblewalker"),
     },
     "index-in-parent-change": {
       type: "indexInParentChange",
       indexInParent: Arg(0, "number"),
     },
+    "audited": {
+      type: "audited",
+      audit: Arg(0, "nullable:json"),
+    },
   },
 
   methods: {
     audit: {
       request: {},
       response: {
         audit: RetVal("nullable:json"),
       },