Bug 1317102 - Part 2: Refactor the highlighter event handlers into ElementEditor and prevent flickering of the display badge when toggled from a click. r=jdescottes

- Refactor the highlighter shown/hidden event handlers from MarkupElementContainer in ElementEditor.
We do this because we want to be a bit more lazy when we set these on/off highlighters to only when
there is only a display badge and appropriate display type.
- This fixes a flickering bug cause by the event handlers when you click on the display badges in the markup view.
STR:
1. Have 2 grids shown on the markup view.
2. Toggle on one of them
3. Toggle the other one.
4. See that the active state toggles on, off and on.
We want to toggle the active state manually instead of waiting for the highlighter event handler
to toggle its active state when we click on the display badge. This is because waiting for
the highlight event handler to toggle the active state will cause a brief delay from showing
the active state when the user clicked on the display badge. To fix this, we added a way
to start/stop the highlighter event handlers.

devtools/client/inspector/markup/views/element-container.js

@@ -34,16 +34,6 @@ function MarkupElementContainer(markupView, node) {
   MarkupContainer.prototype.initialize.call(this, markupView, node,
     "elementcontainer");
 
-  this.onFlexboxHighlighterChange = this.onFlexboxHighlighterChange.bind(this);
-  this.onGridHighlighterChange = this.onGridHighlighterChange.bind(this);
-
-  this.markup.highlighters.on("flexbox-highlighter-hidden",
-    this.onFlexboxHighlighterChange);
-  this.markup.highlighters.on("flexbox-highlighter-shown",
-    this.onFlexboxHighlighterChange);
-  this.markup.highlighters.on("grid-highlighter-hidden", this.onGridHighlighterChange);
-  this.markup.highlighters.on("grid-highlighter-shown", this.onGridHighlighterChange);
-
   if (node.nodeType === nodeConstants.ELEMENT_NODE) {
     this.editor = new ElementEditor(this, node);
   } else {
@@ -54,17 +44,6 @@ function MarkupElementContainer(markupView, node) {
 }
 
 MarkupElementContainer.prototype = extend(MarkupContainer.prototype, {
-  destroy: function() {
-    this.markup.highlighters.off("flexbox-highlighter-hidden",
-      this.onFlexboxHighlighterChange);
-    this.markup.highlighters.off("flexbox-highlighter-shown",
-      this.onFlexboxHighlighterChange);
-    this.markup.highlighters.off("grid-highlighter-hidden", this.onGridHighlighterChange);
-    this.markup.highlighters.off("grid-highlighter-shown", this.onGridHighlighterChange);
-
-    MarkupContainer.prototype.destroy.call(this);
-  },
-
   onContainerClick: function(event) {
     if (!event.target.hasAttribute("data-event")) {
       return;
@@ -73,32 +52,6 @@ MarkupElementContainer.prototype = extend(MarkupContainer.prototype, {
     this._buildEventTooltipContent(event.target);
   },
 
-  /**
-   * Handler for "flexbox-highlighter-hidden" and "flexbox-highlighter-shown" event
-   * emitted from the HighlightersOverlay. Toggles the active state of the display badge
-   * if it matches the highlighted flex container node.
-   */
-  onFlexboxHighlighterChange: function() {
-    if (!this.editor._displayBadge) {
-      return;
-    }
-    this.editor._displayBadge.classList.toggle("active",
-      this.markup.highlighters.flexboxHighlighterShown === this.node);
-  },
-
-  /**
-   * Handler for "grid-highlighter-hidden" and "grid-highlighter-shown" event emitted from
-   * the HighlightersOverlay. Toggles the active state of the display badge if it matches
-   * the highlighted grid node.
-   */
-  onGridHighlighterChange: function() {
-    if (!this.editor._displayBadge) {
-      return;
-    }
-    this.editor._displayBadge.classList.toggle("active",
-      this.markup.highlighters.gridHighlighters.has(this.node));
-  },
-
   async _buildEventTooltipContent(target) {
     const tooltip = this.markup.eventDetailsTooltip;
     await tooltip.hide();

devtools/client/inspector/markup/views/element-editor.js

@@ -77,6 +77,8 @@ function ElementEditor(container, node) {
   this.onCustomBadgeClick = this.onCustomBadgeClick.bind(this);
   this.onDisplayBadgeClick = this.onDisplayBadgeClick.bind(this);
   this.onExpandBadgeClick = this.onExpandBadgeClick.bind(this);
+  this.onFlexboxHighlighterChange = this.onFlexboxHighlighterChange.bind(this);
+  this.onGridHighlighterChange = this.onGridHighlighterChange.bind(this);
   this.onTagEdit = this.onTagEdit.bind(this);
 
   // Create the main editor
@@ -302,14 +304,20 @@ ElementEditor.prototype = {
    * Update the markup display badge.
    */
   updateDisplayBadge: function() {
-    const showDisplayBadge = this.node.displayType in DISPLAY_TYPES;
+    const displayType = this.node.displayType;
+    const showDisplayBadge = displayType in DISPLAY_TYPES;
+
     if (this._displayBadge && !showDisplayBadge) {
+      this.stopTrackingFlexboxHighlighterEvents();
+      this.stopTrackingGridHighlighterEvents();
+
       this._displayBadge.remove();
       this._displayBadge = null;
     } else if (showDisplayBadge) {
       if (!this._displayBadge) {
         this._createDisplayBadge();
       }
+
       this._updateDisplayBadgeContent();
     }
   },
@@ -320,6 +328,9 @@ ElementEditor.prototype = {
     this._displayBadge.addEventListener("click", this.onDisplayBadgeClick);
     // Badges order is [event][display][custom], insert display badge before custom.
     this.elt.insertBefore(this._displayBadge, this._customBadge);
+
+    this.startTrackingFlexboxHighlighterEvents();
+    this.startTrackingGridHighlighterEvents();
   },
 
   _updateDisplayBadgeContent: function() {
@@ -700,26 +711,56 @@ ElementEditor.prototype = {
     this.markup.inspector.once("markupmutation", onMutations);
   },
 
+  startTrackingFlexboxHighlighterEvents() {
+    this.highlighters.on("flexbox-highlighter-hidden", this.onFlexboxHighlighterChange);
+    this.highlighters.on("flexbox-highlighter-shown", this.onFlexboxHighlighterChange);
+  },
+
+  startTrackingGridHighlighterEvents() {
+    this.highlighters.on("grid-highlighter-hidden", this.onGridHighlighterChange);
+    this.highlighters.on("grid-highlighter-shown", this.onGridHighlighterChange);
+  },
+
+  stopTrackingFlexboxHighlighterEvents() {
+    this.highlighters.off("flexbox-highlighter-hidden", this.onFlexboxHighlighterChange);
+    this.highlighters.off("flexbox-highlighter-shown", this.onFlexboxHighlighterChange);
+  },
+
+  stopTrackingGridHighlighterEvents() {
+    this.highlighters.off("grid-highlighter-hidden", this.onGridHighlighterChange);
+    this.highlighters.off("grid-highlighter-shown", this.onGridHighlighterChange);
+  },
+
   /**
-   * Called when the display badge is clicked. Toggles on the grid highlighter for the
-   * selected node if it is a grid container.
+   * Called when the display badge is clicked. Toggles on the flex/grid highlighter for
+   * the selected node if it is a grid container.
    */
-  onDisplayBadgeClick: function(event) {
+  onDisplayBadgeClick: async function(event) {
     event.stopPropagation();
 
     const target = event.target;
 
     if (Services.prefs.getBoolPref("devtools.inspector.flexboxHighlighter.enabled") &&
         (target.dataset.display === "flex" || target.dataset.display === "inline-flex")) {
-      this._displayBadge.classList.add("active");
-      this.highlighters.toggleFlexboxHighlighter(this.inspector.selection.nodeFront,
+      // Stop tracking highlighter events to avoid flickering of the active class.
+      this.stopTrackingFlexboxHighlighterEvents();
+
+      this._displayBadge.classList.toggle("active");
+      await this.highlighters.toggleFlexboxHighlighter(this.inspector.selection.nodeFront,
         "markup");
+
+      this.startTrackingFlexboxHighlighterEvents();
     }
 
     if (target.dataset.display === "grid" || target.dataset.display === "inline-grid") {
-      this._displayBadge.classList.add("active");
-      this.highlighters.toggleGridHighlighter(this.inspector.selection.nodeFront,
+      // Stop tracking highlighter events to avoid flickering of the active class.
+      this.stopTrackingGridHighlighterEvents();
+
+      this._displayBadge.classList.toggle("active");
+      await this.highlighters.toggleGridHighlighter(this.inspector.selection.nodeFront,
         "markup");
+
+      this.startTrackingGridHighlighterEvents();
     }
   },
 
@@ -732,6 +773,32 @@ ElementEditor.prototype = {
     this.container.expandContainer();
   },
 
+  /**
+   * Handler for "flexbox-highlighter-hidden" and "flexbox-highlighter-shown" event
+   * emitted from the HighlightersOverlay. Toggles the active state of the display badge
+   * if it matches the highlighted flex container node.
+   */
+  onFlexboxHighlighterChange: function() {
+    if (!this._displayBadge) {
+      return;
+    }
+    this._displayBadge.classList.toggle("active",
+      this.highlighters.flexboxHighlighterShown === this.node);
+  },
+
+  /**
+   * Handler for "grid-highlighter-hidden" and "grid-highlighter-shown" event emitted from
+   * the HighlightersOverlay. Toggles the active state of the display badge if it matches
+   * the highlighted grid node.
+   */
+  onGridHighlighterChange: function() {
+    if (!this._displayBadge) {
+      return;
+    }
+    this._displayBadge.classList.toggle("active",
+      this.highlighters.gridHighlighters.has(this.node));
+  },
+
   /**
    * Called when the tag name editor has is done editing.
    */
@@ -754,7 +821,10 @@ ElementEditor.prototype = {
   destroy: function() {
     if (this._displayBadge) {
       this._displayBadge.removeEventListener("click", this.onDisplayBadgeClick);
+      this.stopTrackingFlexboxHighlighterEvents();
+      this.stopTrackingGridHighlighterEvents();
     }
+
     if (this._customBadge) {
       this._customBadge.removeEventListener("click", this.onCustomBadgeClick);
     }