Backed out changeset ed74d4e4b273 (bug 1657680) for dt failure on browser_markup_overflow_badge.js
authorNarcis Beleuzu <nbeleuzu@mozilla.com>
Sat, 22 Aug 2020 04:42:50 +0300
changeset 545721 67fa236b5e663c99199f957d9251379c09b82340
parent 545720 bc44124a66ff29bbd0b686cd577d57ee2b6085fc
child 545722 189c5843356f5cbe0973896b3750deaa6f46c8bf
push id124758
push usernbeleuzu@mozilla.com
push dateSat, 22 Aug 2020 01:43:24 +0000
treeherderautoland@67fa236b5e66 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1657680
milestone81.0a1
backs outed74d4e4b273ce346a2419727ab24a4d92bcc4a0
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
Backed out changeset ed74d4e4b273 (bug 1657680) for dt failure on browser_markup_overflow_badge.js
devtools/client/inspector/markup/views/element-editor.js
devtools/client/locales/en-US/inspector.properties
devtools/client/themes/badge.css
devtools/client/themes/markup.css
devtools/server/actors/inspector/node.js
devtools/server/actors/inspector/walker.js
devtools/shared/specs/walker.js
--- a/devtools/client/inspector/markup/views/element-editor.js
+++ b/devtools/client/inspector/markup/views/element-editor.js
@@ -88,35 +88,28 @@ function ElementEditor(container, node) 
   this.inspector = this.markup.inspector;
   this.highlighters = this.markup.highlighters;
   this._cssProperties = this.inspector.cssProperties;
 
   this.isOverflowDebuggingEnabled = Services.prefs.getBoolPref(
     "devtools.overflow.debugging.enabled"
   );
 
-  // If this is a scrollable element, this specifies whether or not its overflow causing
-  // elements are highlighted. Otherwise, it is null if the element is not scrollable.
-  this.highlightingOverflowCausingElements = this.node.isScrollable
-    ? false
-    : null;
-
   this.attrElements = new Map();
   this.animationTimers = {};
 
   this.elt = null;
   this.tag = null;
   this.closeTag = null;
   this.attrList = null;
   this.newAttr = null;
   this.closeElt = null;
 
   this.onCustomBadgeClick = this.onCustomBadgeClick.bind(this);
   this.onDisplayBadgeClick = this.onDisplayBadgeClick.bind(this);
-  this.onScrollableBadgeClick = this.onScrollableBadgeClick.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
   this.buildMarkup();
 
@@ -319,17 +312,16 @@ ElementEditor.prototype = {
     }
 
     this.updateEventBadge();
     this.updateDisplayBadge();
     this.updateCustomBadge();
     this.updateScrollableBadge();
     this.updateTextEditor();
     this.updateOverflowBadge();
-    this.updateOverflowHighlight();
   },
 
   updateEventBadge: function() {
     const showEventBadge = this.node.hasEventListeners;
     if (this._eventBadge && !showEventBadge) {
       this._eventBadge.remove();
       this._eventBadge = null;
     } else if (showEventBadge && !this._eventBadge) {
@@ -358,43 +350,24 @@ ElementEditor.prototype = {
       this._createScrollableBadge();
     } else if (this._scrollableBadge && !this.node.isScrollable) {
       this._scrollableBadge.remove();
       this._scrollableBadge = null;
     }
   },
 
   _createScrollableBadge: function() {
-    const isInteractive =
-      this.isOverflowDebuggingEnabled &&
-      this.node.walkerFront.traits.supportsOverflowDebugging &&
-      // Document elements cannot have interative scrollable badges since retrieval of their
-      // overflow causing elements is not supported.
-      !this.node.isDocumentElement;
-
     this._scrollableBadge = this.doc.createElement("div");
-    this._scrollableBadge.className = `inspector-badge scrollable-badge ${
-      isInteractive ? "interactive" : ""
-    }`;
-
+    this._scrollableBadge.className = "inspector-badge scrollable-badge";
     this._scrollableBadge.textContent = INSPECTOR_L10N.getStr(
       "markupView.scrollableBadge.label"
     );
     this._scrollableBadge.title = INSPECTOR_L10N.getStr(
-      isInteractive
-        ? "markupView.scrollableBadge.interactive.tooltip"
-        : "markupView.scrollableBadge.tooltip"
+      "markupView.scrollableBadge.tooltip"
     );
-
-    if (isInteractive) {
-      this._scrollableBadge.addEventListener(
-        "click",
-        this.onScrollableBadgeClick
-      );
-    }
     this.elt.insertBefore(this._scrollableBadge, this._customBadge);
   },
 
   /**
    * Update the markup display badge.
    */
   updateDisplayBadge: function() {
     const displayType = this.node.displayType;
@@ -494,57 +467,16 @@ ElementEditor.prototype = {
       "markupView.custom.tooltiptext"
     );
     this._customBadge.addEventListener("click", this.onCustomBadgeClick);
     // Badges order is [event][display][custom], insert custom badge at the end.
     this.elt.appendChild(this._customBadge);
   },
 
   /**
-   * If node causes overflow, toggle its overflow highlight if its scrollable ancestor's
-   * scrollable badge is active/inactive.
-   */
-  updateOverflowHighlight: async function() {
-    if (
-      !this.isOverflowDebuggingEnabled ||
-      !this.node.walkerFront.traits.supportsOverflowDebugging
-    ) {
-      return;
-    }
-
-    let showOverflowHighlight = false;
-
-    if (this.node.causesOverflow) {
-      const scrollableAncestor = await this.node.walkerFront.getScrollableAncestorNode(
-        this.node
-      );
-      const markupContainer = scrollableAncestor
-        ? this.markup.getContainer(scrollableAncestor)
-        : null;
-
-      showOverflowHighlight = !!markupContainer?.editor
-        .highlightingOverflowCausingElements;
-    }
-
-    this.setOverflowHighlight(showOverflowHighlight);
-  },
-
-  /**
-   * Show overflow highlight if showOverflowHighlight is true, otherwise hide it.
-   *
-   * @param {Boolean} showOverflowHighlight
-   */
-  setOverflowHighlight: function(showOverflowHighlight) {
-    this.container.tagState.classList.toggle(
-      "overflow-causing-highlighted",
-      showOverflowHighlight
-    );
-  },
-
-  /**
    * Update the inline text editor in case of a single text child node.
    */
   updateTextEditor: function() {
     const node = this.node.inlineTextChild;
 
     if (this.textEditor && this.textEditor.node != node) {
       this.elt.removeChild(this.textEditor.elt);
       this.textEditor.destroy();
@@ -1004,43 +936,16 @@ ElementEditor.prototype = {
     );
   },
 
   onExpandBadgeClick: function() {
     this.container.expandContainer();
   },
 
   /**
-   * Called when the scrollable badge is clicked. Shows the overflow causing elements and
-   * highlights their container if the scroll badge is active.
-   */
-  onScrollableBadgeClick: async function() {
-    this.highlightingOverflowCausingElements = this._scrollableBadge.classList.toggle(
-      "active"
-    );
-
-    const overflowCausingElements = await this.node.walkerFront.getOverflowCausingElements(
-      this.node
-    );
-    const overflowCausingElementsList = await overflowCausingElements.items();
-
-    for (const element of overflowCausingElementsList) {
-      if (this.highlightingOverflowCausingElements) {
-        await this.markup.showNode(element);
-      }
-
-      const markupContainer = this.markup.getContainer(element);
-
-      markupContainer.editor.setOverflowHighlight(
-        this.highlightingOverflowCausingElements
-      );
-    }
-  },
-
-  /**
    * 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;
     }
@@ -1099,23 +1004,16 @@ ElementEditor.prototype = {
       this.stopTrackingFlexboxHighlighterEvents();
       this.stopTrackingGridHighlighterEvents();
     }
 
     if (this._customBadge) {
       this._customBadge.removeEventListener("click", this.onCustomBadgeClick);
     }
 
-    if (this._scrollableBadge) {
-      this._scrollableBadge.removeEventListener(
-        "click",
-        this.onScrollableBadgeClick
-      );
-    }
-
     this.expandBadge.removeEventListener("click", this.onExpandBadgeClick);
 
     for (const key in this.animationTimers) {
       clearTimeout(this.animationTimers[key]);
     }
     this.animationTimers = null;
   },
 };
--- a/devtools/client/locales/en-US/inspector.properties
+++ b/devtools/client/locales/en-US/inspector.properties
@@ -502,20 +502,16 @@ inspector.colorSchemeSimulation.tooltip=
 # LOCALIZATION NOTE (markupView.scrollableBadge.label): This is the text displayed inside a
 # badge, in the inspector, next to nodes that are scrollable in the page.
 markupView.scrollableBadge.label=scroll
 
 # LOCALIZATION NOTE (markupView.scrollableBadge.tooltip): This is the tooltip that is displayed
 # when hovering over badges next to scrollable elements in the inspector.
 markupView.scrollableBadge.tooltip=This element has scrollable overflow.
 
-# LOCALIZATION NOTE (markupView.scrollableBadge.interactive.tooltip): This is the tooltip that is displayed
-# when hovering over interactive badges next to scrollable elements in the inspector.
-markupView.scrollableBadge.interactive.tooltip=This element has scrollable overflow. Click to reveal elements that are causing the overflow.
-
 # LOCALIZATION NOTE (markupView.overflowBadge.label): This is the text displayed inside a
 # badge, in the inspector, next to nodes that are causing overflow in other elements.
 markupView.overflowBadge.label=overflow
 
 # LOCALIZATION NOTE (markupView.overflowBadge.tooltip): This is the tooltip that is displayed
 # when hovering over badges next to overflow causing elements in the inspector.
 markupView.overflowBadge.tooltip=This element is causing an element to overflow.
 
--- a/devtools/client/themes/badge.css
+++ b/devtools/client/themes/badge.css
@@ -6,32 +6,27 @@
   --badge-active-background-color: var(--blue-50);
   --badge-active-border-color: #FFFFFFB3;
   --badge-background-color: white;
   --badge-border-color: #CACAD1;
   --badge-color: var(--grey-60);
   --badge-hover-background-color: #DFDFE8;
   --badge-interactive-background-color: var(--grey-20);
   --badge-interactive-color: var(--grey-90);
-  --badge-scrollable-color: #8000D7;
-  --badge-scrollable-background-color: #FFFFFF;
-  --badge-scrollable-active-background-color: #8000D7;
 }
 
 .theme-dark:root {
   --badge-active-background-color: var(--blue-60);
   --badge-active-border-color: #FFF6;
   --badge-background-color: var(--grey-80);
   --badge-border-color: var(--grey-50);
   --badge-color: var(--grey-40);
   --badge-hover-background-color: var(--grey-80);
   --badge-interactive-background-color: var(--grey-70);
   --badge-interactive-color: var(--grey-30);
-  --badge-scrollable-color: #B98EFF;
-  --badge-scrollable-background-color: transparent;
 }
 
 /* Inspector badge */
 .inspector-badge,
 .editor.text .whitespace::before {
   display: inline-block;
   /* 9px text is too blurry on low-resolution screens */
   font-size: 10px;
@@ -75,19 +70,8 @@
 }
 
 .inspector-badge.active,
 .inspector-badge.interactive.active {
   background-color: var(--badge-active-background-color);
   border-color: var(--badge-active-border-color);
   color: var(--theme-selection-color);
 }
-
-.inspector-badge.interactive.scrollable-badge {
-  background-color: var(--badge-scrollable-background-color);
-  border-color: var(--badge-scrollable-color);
-  color: var(--badge-scrollable-color);
-}
-
-.inspector-badge.interactive.scrollable-badge.active {
-  background-color: var(--badge-scrollable-active-background-color);
-  color: var(--theme-selection-color);
-}
--- a/devtools/client/themes/markup.css
+++ b/devtools/client/themes/markup.css
@@ -6,28 +6,26 @@
   --markup-hidden-attr-name-color: var(--grey-43);
   --markup-hidden-attr-value-color: var(--grey-55);
   --markup-hidden-punctuation-color: var(--grey-43);
   --markup-pseudoclass-disk-color: #e9c600;
   --markup-hidden-tag-color: var(--grey-50);
   --markup-outline: var(--theme-splitter-color);
   --markup-drag-line: var(--grey-40);
   --markup-drop-line: var(--blue-55);
-  --markup-overflow-causing-background-color: rgba(128, 0, 215, 0.15);
 }
 
 .theme-dark:root {
   --markup-hidden-attr-name-color: #787878;
   --markup-hidden-attr-value-color: #a4a4a4;
   --markup-hidden-punctuation-color: #787878;
   --markup-hidden-tag-color: var(--grey-45);
   --markup-outline: var(--theme-selection-background);
   --markup-drag-line: var(--grey-55);
   --markup-drop-line: var(--blue-50);
-  --markup-overflow-causing-background-color: rgba(148, 0, 255, 0.38);
 }
 
 * {
   padding: 0;
   margin: 0;
 }
 
 :root {
@@ -306,20 +304,16 @@ ul.children + .tag-line::before {
 }
 
 /* Hide HTML void elements (img, hr, br, …) closing tag when the element is not
  * expanded (it can be if it has pseudo-elements attached) */
 .child.collapsed > .tag-line .void-element .close {
   display: none;
 }
 
-.tag-line .tag-state.overflow-causing-highlighted:not(.theme-selected) {
-  background-color: var(--markup-overflow-causing-background-color);
-}
-
 .closing-bracket {
   pointer-events: none;
 }
 
 .newattr {
   margin-right: -13px;
 }
 
--- a/devtools/server/actors/inspector/node.js
+++ b/devtools/server/actors/inspector/node.js
@@ -103,22 +103,21 @@ const NodeActor = protocol.ActorClassWit
     this._eventCollector = new EventCollector(this.walker.targetActor);
 
     // Store the original display type and scrollable state and whether or not the node is
     // displayed to track changes when reflows occur.
     this.currentDisplayType = this.displayType;
     this.wasDisplayed = this.isDisplayed;
     this.wasScrollable = this.isScrollable;
 
-    if (this.isScrollable) {
-      this.walker.updateOverflowCausingElements(
-        this,
-        this.walker.overflowCausingElementsMap
-      );
-    }
+    this.walker.updateOverflowCausingElements(
+      this.rawNode,
+      this,
+      this.walker.overflowCausingElementsSet
+    );
   },
 
   toString: function() {
     return (
       "[NodeActor " + this.actorID + " for " + this.rawNode.toString() + "]"
     );
   },
 
@@ -179,17 +178,17 @@ const NodeActor = protocol.ActorClassWit
       nodeName: this.rawNode.nodeName,
       nodeValue: this.rawNode.nodeValue,
       displayName: getNodeDisplayName(this.rawNode),
       numChildren: this.numChildren,
       inlineTextChild: inlineTextChild ? inlineTextChild.form() : undefined,
       displayType: this.displayType,
       isScrollable: this.isScrollable,
       isTopLevelDocument: this.isTopLevelDocument,
-      causesOverflow: this.walker.overflowCausingElementsMap.has(this.rawNode),
+      causesOverflow: this.walker.overflowCausingElementsSet.has(this.rawNode),
 
       // doctype attributes
       name: this.rawNode.name,
       publicId: this.rawNode.publicId,
       systemId: this.rawNode.systemId,
 
       attrs: this.writeAttrs(),
       customElementLocation: this.getCustomElementLocation(),
--- a/devtools/server/actors/inspector/walker.js
+++ b/devtools/server/actors/inspector/walker.js
@@ -201,20 +201,17 @@ var WalkerActor = protocol.ActorClassWit
     this._nodeActorsMap = new Map();
 
     this._pendingMutations = [];
     this._activePseudoClassLocks = new Set();
     this._mutationBreakpoints = new WeakMap();
     this.customElementWatcher = new CustomElementWatcher(
       targetActor.chromeEventHandler
     );
-
-    // In this map, the key-value pairs are the overflow causing elements and their
-    // respective ancestor scrollable node actor.
-    this.overflowCausingElementsMap = new Map();
+    this.overflowCausingElementsSet = new Set();
 
     this.showAllAnonymousContent = options.showAllAnonymousContent;
 
     this.walkerSearch = new WalkerSearch(this);
 
     // Nodes which have been removed from the client's known
     // ownership tree are considered "orphaned", and stored in
     // this set.
@@ -362,18 +359,16 @@ var WalkerActor = protocol.ActorClassWit
   // Returns the JSON representation of this object over the wire.
   form: function() {
     return {
       actor: this.actorID,
       root: this.rootNode.form(),
       traits: {
         // Walker implements node picker starting with Firefox 80
         supportsNodePicker: true,
-        // Walker implements overflow debugging support starting with Firefox 81
-        supportsOverflowDebugging: true,
       },
     };
   },
 
   toString: function() {
     return "[WalkerActor " + this.actorID + "]";
   },
 
@@ -419,18 +414,18 @@ var WalkerActor = protocol.ActorClassWit
       return;
     }
     this._destroyed = true;
     protocol.Actor.prototype.destroy.call(this);
     try {
       this.clearPseudoClassLocks();
       this._activePseudoClassLocks = null;
 
-      this.overflowCausingElementsMap.clear();
-      this.overflowCausingElementsMap = null;
+      this.overflowCausingElementsSet.clear();
+      this.overflowCausingElementsSet = null;
 
       this._hoveredNode = null;
       this.rootWin = null;
       this.rootDoc = null;
       this.rootNode = null;
       this.layoutHelpers = null;
       this._orphaned = null;
       this._retainedOrphans = null;
@@ -574,17 +569,17 @@ var WalkerActor = protocol.ActorClassWit
   },
 
   _onReflows: function(reflows) {
     // Going through the nodes the walker knows about, see which ones have had their
     // display, scrollable or overflow state changed and send events if any.
     const displayTypeChanges = [];
     const scrollableStateChanges = [];
 
-    const currentOverflowCausingElementsMap = new Map();
+    const currentOverflowCausingElementsSet = new Set();
 
     for (const [node, actor] of this._nodeActorsMap) {
       if (Cu.isDeadWrapper(node)) {
         continue;
       }
 
       const displayType = actor.displayType;
       const isDisplayed = actor.isDisplayed;
@@ -601,37 +596,36 @@ var WalkerActor = protocol.ActorClassWit
       }
 
       const isScrollable = actor.isScrollable;
       if (isScrollable !== actor.wasScrollable) {
         scrollableStateChanges.push(actor);
         actor.wasScrollable = isScrollable;
       }
 
-      if (isScrollable) {
-        this.updateOverflowCausingElements(
-          actor,
-          currentOverflowCausingElementsMap
-        );
-      }
+      this.updateOverflowCausingElements(
+        node,
+        actor,
+        currentOverflowCausingElementsSet
+      );
     }
 
     // Get the NodeActor for each node in the symmetric difference of
-    // currentOverflowCausingElementsMap and this.overflowCausingElementsMap
-    const overflowStateChanges = [...currentOverflowCausingElementsMap.keys()]
-      .filter(node => !this.overflowCausingElementsMap.has(node))
+    // currentOverflowCausingElementsSet and this.overflowCausingElementsSet
+    const overflowStateChanges = [...currentOverflowCausingElementsSet]
+      .filter(node => !this.overflowCausingElementsSet.has(node))
       .concat(
-        [...this.overflowCausingElementsMap.keys()].filter(
-          node => !currentOverflowCausingElementsMap.has(node)
+        [...this.overflowCausingElementsSet].filter(
+          node => !currentOverflowCausingElementsSet.has(node)
         )
       )
       .filter(node => this.hasNode(node))
       .map(node => this.getNode(node));
 
-    this.overflowCausingElementsMap = currentOverflowCausingElementsMap;
+    this.overflowCausingElementsSet = currentOverflowCausingElementsSet;
 
     if (displayTypeChanges.length) {
       this.emit("display-change", displayTypeChanges);
     }
 
     if (scrollableStateChanges.length) {
       this.emit("scrollable-change", scrollableStateChanges);
     }
@@ -2864,73 +2858,35 @@ var WalkerActor = protocol.ActorClassWit
     this.nodePicker.pick(doFocus);
   },
 
   cancelPick() {
     this.nodePicker.cancelPick();
   },
 
   /**
-   * Given a scrollable node, find its descendants which are causing overflow in it and
-   * add their raw nodes to the map as keys with the scrollable element as the values.
-   *
-   * @param {NodeActor} scrollableNode A scrollable node.
-   * @param {Map} map The map to which the overflow causing elements are added.
+   * If element has a scrollbar, find the children causing the overflow and
+   * add them to the set.
+   * @param {DOMNode} node The node whose overflow causing elements are returned.
+   * @param {NodeActor} actor The actor of the node.
+   * @param {Set} set The set to which the overflow causing elements are added.
    */
-  updateOverflowCausingElements: function(scrollableNode, map) {
-    if (scrollableNode.rawNode.nodeType !== Node.ELEMENT_NODE) {
+  updateOverflowCausingElements: function(node, actor, set) {
+    if (node.nodeType !== Node.ELEMENT_NODE || !actor.isScrollable) {
       return;
     }
 
     const overflowCausingChildren = [
-      ...InspectorUtils.getOverflowingChildrenOfElement(scrollableNode.rawNode),
+      ...InspectorUtils.getOverflowingChildrenOfElement(node),
     ];
 
-    for (let overflowCausingChild of overflowCausingChildren) {
-      // overflowCausingChild is a Node, but not necessarily an Element.
+    for (let child of overflowCausingChildren) {
+      // child is a Node, but not necessarily an Element.
       // So, get the containing Element
-      if (overflowCausingChild.nodeType !== Node.ELEMENT_NODE) {
-        overflowCausingChild = overflowCausingChild.parentElement;
+      if (child.nodeType !== Node.ELEMENT_NODE) {
+        child = child.parentElement;
       }
-      map.set(overflowCausingChild, scrollableNode);
-    }
-  },
-
-  /**
-   * Return the overflow causing elements for the given node.
-   *
-   * @param {NodeActor} node The scrollable node.
-   */
-  getOverflowCausingElements: function(node) {
-    if (node.rawNode.nodeType !== Node.ELEMENT_NODE || !node.isScrollable) {
-      return [];
+      set.add(child);
     }
-
-    const overflowCausingElements = [
-      ...InspectorUtils.getOverflowingChildrenOfElement(node.rawNode),
-    ].map(overflowCausingChild => {
-      if (overflowCausingChild.nodeType !== Node.ELEMENT_NODE) {
-        overflowCausingChild = overflowCausingChild.parentElement;
-      }
-
-      this.attachElement(overflowCausingChild);
-
-      return overflowCausingChild;
-    });
-
-    return new NodeListActor(this, overflowCausingElements);
-  },
-
-  /**
-   * Return the scrollable ancestor node which has overflow because of the given node.
-   *
-   * @param {NodeActor} overflowCausingNode
-   */
-  getScrollableAncestorNode: function(overflowCausingNode) {
-    if (!this.overflowCausingElementsMap.has(overflowCausingNode.rawNode)) {
-      return null;
-    }
-
-    return this.overflowCausingElementsMap.get(overflowCausingNode.rawNode);
   },
 });
 
 exports.WalkerActor = WalkerActor;
--- a/devtools/shared/specs/walker.js
+++ b/devtools/shared/specs/walker.js
@@ -379,28 +379,12 @@ const walkerSpec = generateActorSpec({
     watchRootNode: {
       request: {},
       response: {},
     },
     unwatchRootNode: {
       request: {},
       oneway: true,
     },
-    getOverflowCausingElements: {
-      request: {
-        node: Arg(0, "domnode"),
-      },
-      response: {
-        list: RetVal("domnodelist"),
-      },
-    },
-    getScrollableAncestorNode: {
-      request: {
-        node: Arg(0, "domnode"),
-      },
-      response: {
-        node: RetVal("nullable:domnode"),
-      },
-    },
   },
 });
 
 exports.walkerSpec = walkerSpec;