Bug 1538306 - ensure highlighter bounds overlay does not interfere with the accessible audit. r=pbro
authorYura Zenevich <yura.zenevich@gmail.com>
Tue, 26 Mar 2019 13:33:37 +0000
changeset 466097 76e7655d974e628646aa26b7d25fe7ef3a4ec615
parent 466096 72b740c734db3f2241236f35168c5da0658ffa1c
child 466098 f1eec8f7a3f87f9d8edbe3a2c5d6fa9c629eb6f2
push id81440
push useryura.zenevich@gmail.com
push dateTue, 26 Mar 2019 13:35:22 +0000
treeherderautoland@76e7655d974e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbro
bugs1538306
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 1538306 - ensure highlighter bounds overlay does not interfere with the accessible audit. r=pbro Differential Revision: https://phabricator.services.mozilla.com/D24546
devtools/server/actors/accessibility/accessible.js
devtools/server/actors/accessibility/walker.js
devtools/server/actors/highlighters.js
devtools/server/actors/highlighters/accessible.js
devtools/server/actors/highlighters/xul-accessible.js
--- a/devtools/server/actors/accessibility/accessible.js
+++ b/devtools/server/actors/accessibility/accessible.js
@@ -384,23 +384,23 @@ const AccessibleActor = ActorClassWithSp
    */
   async _getContrastRatio() {
     if (!this._isValidTextLeaf(this.rawAccessible)) {
       return null;
     }
 
     const { DOMNode: rawNode } = this.rawAccessible;
     const win = rawNode.ownerGlobal;
-    this.walker.loadTransitionDisablingStyleSheet(win);
+    this.walker.clearStyles(win);
     const contrastRatio = await getContrastRatioFor(rawNode.parentNode, {
       bounds: this.bounds,
       win,
     });
 
-    this.walker.removeTransitionDisablingStyleSheet(win);
+    this.walker.restoreStyles(win);
 
     return contrastRatio;
   },
 
   /**
    * Audit the state of the accessible object.
    *
    * @return {Object|null}
--- a/devtools/server/actors/accessibility/walker.js
+++ b/devtools/server/actors/accessibility/walker.js
@@ -472,49 +472,73 @@ const AccessibleWalkerActor = ActorClass
         }
         break;
       default:
         break;
     }
   },
 
   /**
-   * Load accessibility highlighter style sheet used for preventing transitions and
-   * applying transparency when calculating colour contrast.
+   * Ensure that nothing interferes with the audit for an accessible object
+   * (CSS, overlays) by load accessibility highlighter style sheet used for
+   * preventing transitions and applying transparency when calculating colour
+   * contrast as well as temporarily hiding accessible highlighter overlay.
    * @param  {Object} win
    *         Window where highlighting happens.
    */
-  loadTransitionDisablingStyleSheet(win) {
+  clearStyles(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;
+    this.hideHighlighter();
   },
 
   /**
-   * Unload accessibility highlighter style sheet used for preventing transitions and
-   * applying transparency when calculating colour contrast.
+   * Restore CSS and overlays that could've interfered with the audit for an
+   * accessible object by unloading accessibility highlighter style sheet used
+   * for preventing transitions and applying transparency when calculating
+   * colour contrast and potentially restoring accessible highlighter overlay.
    * @param  {Object} win
    *         Window where highlighting was happenning.
    */
-  removeTransitionDisablingStyleSheet(win) {
+  restoreStyles(win) {
     if (!this._sheetLoaded) {
       return;
     }
 
+    this.showHighlighter();
     removeSheet(win, HIGHLIGHTER_STYLES_SHEET);
     this._sheetLoaded = false;
   },
 
+  hideHighlighter() {
+    // TODO: Fix this workaround that temporarily removes higlighter bounds
+    // overlay that can interfere with the contrast ratio calculation.
+    if (this._highlighter) {
+      const highlighter = this._highlighter.instance;
+      highlighter.hideAccessibleBounds();
+    }
+  },
+
+  showHighlighter() {
+    // TODO: Fix this workaround that temporarily removes higlighter bounds
+    // overlay that can interfere with the contrast ratio calculation.
+    if (this._highlighter) {
+      const highlighter = this._highlighter.instance;
+      highlighter.showAccessibleBounds();
+    }
+  },
+
   /**
    * 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:
--- a/devtools/server/actors/highlighters.js
+++ b/devtools/server/actors/highlighters.js
@@ -505,16 +505,23 @@ exports.CustomHighlighterActor = protoco
     protocol.Actor.prototype.destroy.call(this);
     this.finalize();
     this._parent = null;
   },
 
   release: function() {},
 
   /**
+   * Get current instance of the highlighter object.
+   */
+  get instance() {
+    return this._highlighter;
+  },
+
+  /**
    * Show the highlighter.
    * This calls through to the highlighter instance's |show(node, options)|
    * method.
    *
    * Most custom highlighters are made to highlight DOM nodes, hence the first
    * NodeActor argument (NodeActor as in
    * devtools/server/actor/inspector).
    * Note however that some highlighters use this argument merely as a context
--- a/devtools/server/actors/highlighters/accessible.js
+++ b/devtools/server/actors/highlighters/accessible.js
@@ -228,27 +228,62 @@ class AccessibleHighlighter extends Auto
     setIgnoreLayoutChanges(true);
     this._hideAccessibleBounds();
     this.accessibleInfobar.hide();
     setIgnoreLayoutChanges(false,
                            this.highlighterEnv.window.document.documentElement);
   }
 
   /**
+   * Public API method to temporarily hide accessible bounds for things like
+   * color contrast calculation.
+   */
+  hideAccessibleBounds() {
+    if (this.getElement("elements").hasAttribute("hidden")) {
+      return;
+    }
+
+    this._hideAccessibleBounds();
+    this._shouldRestoreBoundsVisibility = true;
+  }
+
+  /**
+   * Public API method to show accessible bounds in case they were temporarily
+   * hidden.
+   */
+  showAccessibleBounds() {
+    if (this._shouldRestoreBoundsVisibility) {
+      this._showAccessibleBounds();
+    }
+  }
+
+  /**
    * Hide the accessible bounds container.
    */
   _hideAccessibleBounds() {
+    this._shouldRestoreBoundsVisibility = null;
+    setIgnoreLayoutChanges(true);
     this.getElement("elements").setAttribute("hidden", "true");
+    setIgnoreLayoutChanges(false,
+      this.highlighterEnv.window.document.documentElement);
   }
 
   /**
    * Show the accessible bounds container.
    */
   _showAccessibleBounds() {
+    this._shouldRestoreBoundsVisibility = null;
+    if (!this.currentNode || !this.highlighterEnv.window) {
+      return;
+    }
+
+    setIgnoreLayoutChanges(true);
     this.getElement("elements").removeAttribute("hidden");
+    setIgnoreLayoutChanges(false,
+      this.highlighterEnv.window.document.documentElement);
   }
 
   /**
    * Get current accessible bounds.
    *
    * @return {Object|null} Returns, if available, positioning and bounds
    *                       information for the accessible object.
    */
--- a/devtools/server/actors/highlighters/xul-accessible.js
+++ b/devtools/server/actors/highlighters/xul-accessible.js
@@ -333,28 +333,57 @@ class XULWindowAccessibleHighlighter {
    *         The node to highlight.
    * @return {Boolean} whether or not node is valid.
    */
   _isNodeValid(node) {
     return isNodeValid(node) || isNodeValid(node, TEXT_NODE);
   }
 
   /**
+   * Public API method to temporarily hide accessible bounds for things like
+   * color contrast calculation.
+   */
+  hideAccessibleBounds() {
+    if (this.container.hasAttribute("hidden")) {
+      return;
+    }
+
+    this._hideAccessibleBounds();
+    this._shouldRestoreBoundsVisibility = true;
+  }
+
+  /**
+   * Public API method to show accessible bounds in case they were temporarily
+   * hidden.
+   */
+  showAccessibleBounds() {
+    if (this._shouldRestoreBoundsVisibility) {
+      this._showAccessibleBounds();
+    }
+  }
+
+  /**
    * Show accessible bounds highlighter.
    */
   _showAccessibleBounds() {
+    this._shouldRestoreBoundsVisibility = null;
     if (this.container) {
+      if (!this.currentNode || !this.highlighterEnv.window) {
+        return;
+      }
+
       this.container.removeAttribute("hidden");
     }
   }
 
   /**
    * Hide accessible bounds highlighter.
    */
   _hideAccessibleBounds() {
+    this._shouldRestoreBoundsVisibility = null;
     if (this.container) {
       this.container.setAttribute("hidden", "true");
     }
   }
 
   /**
    * Hide accessible highlighter, clean up and remove the markup.
    */