Bug 1733046 - [devtools] Hide Geometry highlighter on various user interaction. r=jdescottes.

The code was written to hide the geometry highlighter when:
- the nodepicker was selected
- another node was selected in the markup view
- a node was hovered in the markup view

but this wasn't working as intended, as the line setting
the first of the event listeners was throwing, and we
don't have tests for this.

This patch fixes the faulty line and adds a test to check
that the geometry highlighter does hide on those actions.
We take this opportunity to use an AbortController to handle
the removal of the event listeners added when starting the
geometry highlighter.

Differential Revision: https://phabricator.services.mozilla.com/D126885

devtools/client/inspector/boxmodel/box-model.js

@@ -69,6 +69,11 @@ BoxModel.prototype = {
     this.inspector.selection.off("new-node-front", this.onNewSelection);
     this.inspector.sidebar.off("select", this.onSidebarSelect);
 
+    if (this._geometryEditorEventsAbortController) {
+      this._geometryEditorEventsAbortController.abort();
+      this._geometryEditorEventsAbortController = null;
+    }
+
     if (this._tooltip) {
       this._tooltip.destroy();
     }
@@ -231,18 +236,13 @@ BoxModel.prototype = {
    * geometry editor enabled state.
    */
   onHideGeometryEditor() {
-    const { markup, selection, inspector } = this.inspector;
-
     this.highlighters.hideGeometryEditor();
     this.store.dispatch(updateGeometryEditorEnabled(false));
 
-    inspector.toolbox.nodePicker.off(
-      "picker-started",
-      this.onHideGeometryEditor
-    );
-    selection.off("new-node-front", this.onHideGeometryEditor);
-    markup.off("leave", this.onMarkupViewLeave);
-    markup.off("node-hover", this.onMarkupViewNodeHover);
+    if (this._geometryEditorEventsAbortController) {
+      this._geometryEditorEventsAbortController.abort();
+      this._geometryEditorEventsAbortController = null;
+    }
   },
 
   /**
@@ -403,7 +403,7 @@ BoxModel.prototype = {
    * toggle button is clicked.
    */
   onToggleGeometryEditor() {
-    const { markup, selection, inspector } = this.inspector;
+    const { markup, selection, toolbox } = this.inspector;
     const nodeFront = this.inspector.selection.nodeFront;
     const state = this.store.getState();
     const enabled = !state.boxModel.geometryEditorEnabled;
@@ -412,23 +412,29 @@ BoxModel.prototype = {
     this.store.dispatch(updateGeometryEditorEnabled(enabled));
 
     if (enabled) {
-      // Hide completely the geometry editor if the picker is clicked or a new node front
-      inspector.toolbox.nodePicker.on(
+      this._geometryEditorEventsAbortController = new AbortController();
+      const eventListenersConfig = {
+        signal: this._geometryEditorEventsAbortController.signal,
+      };
+      // Hide completely the geometry editor if:
+      // - the picker is clicked
+      // - or if a new node is selected
+      toolbox.nodePicker.on(
         "picker-started",
-        this.onHideGeometryEditor
+        this.onHideGeometryEditor,
+        eventListenersConfig
       );
-      selection.on("new-node-front", this.onHideGeometryEditor);
-      // Temporary hide the geometry editor
-      markup.on("leave", this.onMarkupViewLeave);
-      markup.on("node-hover", this.onMarkupViewNodeHover);
-    } else {
-      inspector.toolbox.nodePicker.off(
-        "picker-started",
-        this.onHideGeometryEditor
+      selection.on(
+        "new-node-front",
+        this.onHideGeometryEditor,
+        eventListenersConfig
       );
-      selection.off("new-node-front", this.onHideGeometryEditor);
-      markup.off("leave", this.onMarkupViewLeave);
-      markup.off("node-hover", this.onMarkupViewNodeHover);
+      // Temporarily hide the geometry editor
+      markup.on("leave", this.onMarkupViewLeave, eventListenersConfig);
+      markup.on("node-hover", this.onMarkupViewNodeHover, eventListenersConfig);
+    } else if (this._geometryEditorEventsAbortController) {
+      this._geometryEditorEventsAbortController.abort();
+      this._geometryEditorEventsAbortController = null;
     }
   },
 

devtools/client/inspector/test/browser.ini

@@ -134,6 +134,7 @@ skip-if = os == 'win' # bug 1413442
 [browser_inspector_highlighter-geometry_04.js]
 [browser_inspector_highlighter-geometry_05.js]
 [browser_inspector_highlighter-geometry_06.js]
+[browser_inspector_highlighter-geometry_hide_on_interaction.js]
 [browser_inspector_highlighter-hover_01.js]
 [browser_inspector_highlighter-hover_02.js]
 [browser_inspector_highlighter-hover_03.js]

devtools/client/inspector/test/browser_inspector_highlighter-geometry_hide_on_interaction.js

@@ -0,0 +1,81 @@
+/* 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";
+
+// Test that the geometry highlighter gets hidden when the user performs other actions.
+
+const TEST_URL = `data:text/html;charset=utf-8,
+   <h1 style='background:yellow;position:absolute;left:5rem;'>Hello</h1>`;
+
+add_task(async function() {
+  const { inspector, toolbox } = await openInspectorForURL(TEST_URL);
+
+  info("Select the absolute positioned element");
+  await selectNode("h1", inspector);
+  const boxModelPanel = inspector.getPanel("boxmodel");
+
+  info("Click on the button enabling the highlighter");
+  const button = await waitFor(() =>
+    boxModelPanel.document.querySelector(".layout-geometry-editor")
+  );
+
+  let onHighlighterShown = getOnceHighlighterShown(inspector);
+  button.click();
+
+  await onHighlighterShown;
+  ok(true, "Highlighter is displayed");
+
+  info("Check that hovering a node in the markup view hides the highlighter");
+  const h1Nodecontainer = await getContainerForSelector("h1", inspector);
+
+  let onHighlighterHidden = getOnceHighlighterHidden(inspector);
+  EventUtils.synthesizeMouseAtCenter(
+    h1Nodecontainer.tagLine,
+    { type: "mousemove" },
+    inspector.markup.doc.defaultView
+  );
+  await onHighlighterHidden;
+  ok("Highlighter was hidden when hovering a node in the markup view");
+
+  info("Check that leaving the markupview shows the highlighter again");
+  onHighlighterShown = getOnceHighlighterShown(inspector);
+  EventUtils.synthesizeMouseAtCenter(
+    button,
+    { type: "mousemove" },
+    button.ownerDocument.defaultView
+  );
+  await onHighlighterShown;
+  ok(true, "Highlighter is shown again when leaving the markup view");
+
+  info("Check that starting the node picker disable the highlighter");
+  onHighlighterHidden = getOnceHighlighterHidden(inspector);
+  await startPicker(toolbox);
+  await onHighlighterHidden;
+  ok("Highlighter was hidden when using the node picker");
+
+  // stop the node picker
+  await toolbox.nodePicker.stop();
+
+  info("Check that selecting another node does hide the highlighter");
+  onHighlighterShown = onHighlighterShown = getOnceHighlighterShown(inspector);
+  button.click();
+  await onHighlighterShown;
+  ok(true, "highlighter is displayed again");
+
+  onHighlighterHidden = onHighlighterHidden = getOnceHighlighterHidden(
+    inspector
+  );
+  await selectNode("body", inspector);
+  await onHighlighterHidden;
+  ok(true, "Selecting another node hides the highlighter");
+});
+
+function getOnceHighlighterShown(inspector) {
+  return inspector.highlighters.once("geometry-editor-highlighter-shown");
+}
+
+function getOnceHighlighterHidden(inspector) {
+  return inspector.highlighters.once("geometry-editor-highlighter-hidden");
+}