Bug 1514856 - make sure a11y highlighter does not do unnecessary when highlighted accessible gets updated. r=pbro
authorYura Zenevich <yura.zenevich@gmail.com>
Wed, 16 Jan 2019 19:00:27 +0000
changeset 514123 ee36ea46016d1bbe51bbc1e5d6c398f1dc3f1a83
parent 514122 28bcc4da309251ab6c1f10a3d4e67c1c55ee4fad
child 514124 4fe5d0b7257233694b6c069d9b3c6c8a690943b3
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbro
bugs1514856
milestone66.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 1514856 - make sure a11y highlighter does not do unnecessary when highlighted accessible gets updated. r=pbro MozReview-Commit-ID: AhfECIMsDPm Differential Revision: https://phabricator.services.mozilla.com/D15112
devtools/server/actors/accessibility/accessible.js
devtools/server/actors/accessibility/walker.js
--- a/devtools/server/actors/accessibility/accessible.js
+++ b/devtools/server/actors/accessibility/accessible.js
@@ -393,17 +393,17 @@ const AccessibleActor = ActorClassWithSp
   },
 
   /**
    * Audit the state of the accessible object.
    *
    * @return {Object|null}
    *         Audit results for the accessible object.
   */
-  get audit() {
+  async audit() {
     return this.isDefunct ? null : {
       contrastRatio: this._getContrastRatio(),
     };
   },
 
   snapshot() {
     return getSnapshot(this.rawAccessible, this.walker.a11yService);
   },
--- a/devtools/server/actors/accessibility/walker.js
+++ b/devtools/server/actors/accessibility/walker.js
@@ -472,60 +472,110 @@ const AccessibleWalkerActor = ActorClass
         }
         break;
       default:
         break;
     }
   },
 
   /**
+   * Load accessibility highlighter style sheet used for preventing transitions and
+   * applying transparency when calculating colour contrast.
+   * @param  {Object} win
+   *         Window where highlighting happens.
+   */
+  loadTransitionDisablingStyleSheet(win) {
+    if (this._sheetLoaded) {
+      return;
+    }
+
+    // Disable potential mouse driven transitions (This is important because accessibility
+    // highlighter temporarily modifies text color related CSS properties. In case where
+    // there are transitions that affect them, there might be unexpected side effects when
+    // taking a snapshot for contrast measurement).
+    loadSheet(win, HIGHLIGHTER_STYLES_SHEET);
+    this._sheetLoaded = true;
+  },
+
+  /**
+   * Unload accessibility highlighter style sheet used for preventing transitions and
+   * applying transparency when calculating colour contrast.
+   * @param  {Object} win
+   *         Window where highlighting was happenning.
+   */
+  removeTransitionDisablingStyleSheet(win) {
+    if (!this._sheetLoaded) {
+      return;
+    }
+
+    removeSheet(win, HIGHLIGHTER_STYLES_SHEET);
+    this._sheetLoaded = false;
+  },
+
+  /**
    * Public method used to show an accessible object highlighter on the client
    * side.
    *
    * @param  {Object} accessible
    *         AccessibleActor to be highlighted.
    * @param  {Object} options
    *         Object used for passing options. Available options:
    *         - duration {Number}
    *                    Duration of time that the highlighter should be shown.
    * @return {Boolean}
    *         True if highlighter shows the accessible object.
    */
-  highlightAccessible(accessible, options = {}) {
+  async highlightAccessible(accessible, options = {}) {
     this.unhighlight();
+    // Do not highlight if accessible is dead.
+    if (!accessible || accessible.isDefunct || accessible.indexInParent < 0) {
+      return false;
+    }
+
+    this._highlightingAccessible = accessible;
     const { bounds } = accessible;
     if (!bounds) {
       return false;
     }
 
-    // Disable potential mouse driven transitions (This is important because accessibility
-    // highlighter temporarily modifies text color related CSS properties. In case where
-    // there are transitions that affect them, there might be unexpected side effects when
-    // taking a snapshot for contrast measurement)
     const { DOMNode: rawNode } = accessible.rawAccessible;
     const win = rawNode.ownerGlobal;
-    loadSheet(win, HIGHLIGHTER_STYLES_SHEET);
-    const { audit, name, role } = accessible;
+    this.loadTransitionDisablingStyleSheet(win);
+    const audit = await accessible.audit();
+    if (this._highlightingAccessible !== accessible) {
+      if (!this._highlightingAccessible) {
+        // Unhilight might have been called while waiting for audit, remove style sheet
+        // (re-enable transitions) if so.
+        this.removeTransitionDisablingStyleSheet(win);
+      }
+
+      return false;
+    }
+
+    this.removeTransitionDisablingStyleSheet(win);
+
+    const { name, role } = accessible;
     const shown = this.highlighter.show({ rawNode },
                                         { ...options, ...bounds, name, role, audit });
-    // Re-enable transitions.
-    removeSheet(win, HIGHLIGHTER_STYLES_SHEET);
+    this._highlightingAccessible = null;
+
     return shown;
   },
 
   /**
    * Public method used to hide an accessible object highlighter on the client
    * side.
    */
   unhighlight() {
     if (!this._highlighter) {
       return;
     }
 
     this.highlighter.hide();
+    this._highlightingAccessible = null;
   },
 
   /**
    * Picking state that indicates if picking is currently enabled and, if so,
    * what the current and hovered accessible objects are.
    */
   _isPicking: false,
   _currentAccessible: null,
@@ -596,35 +646,38 @@ const AccessibleWalkerActor = ActorClass
   },
 
   /**
    * Hover event handler for when picking is enabled.
    *
    * @param  {Object} event
    *         Current hover event.
    */
-  onHovered(event) {
+  async onHovered(event) {
     if (!this._isPicking) {
       return;
     }
 
     this._preventContentEvent(event);
     if (!this._isEventAllowed(event)) {
       return;
     }
 
     const accessible = this._findAndAttachAccessible(event);
-    if (!accessible) {
+    if (!accessible || this._currentAccessible === accessible) {
       return;
     }
 
-    if (this._currentAccessible !== accessible) {
-      this.highlightAccessible(accessible);
+    this._currentAccessible = accessible;
+    // Highlight current accessible and by the time we are done, if accessible that was
+    // highlighted is not current any more (user moved the mouse to a new node) highlight
+    // the most current accessible again.
+    const shown = await this.highlightAccessible(accessible);
+    if (this._isPicking && shown && accessible === this._currentAccessible) {
       events.emit(this, "picker-accessible-hovered", accessible);
-      this._currentAccessible = accessible;
     }
   },
 
   /**
    * Keyboard event handler for when picking is enabled.
    *
    * @param  {Object} event
    *         Current keyboard event.