Backed out 3 changesets (bug 1620098) for devtools jtest failures. CLOSED TREE

Backed out changeset a3a8244860bc (bug 1620098)
Backed out changeset 2996bd22bd97 (bug 1620098)
Backed out changeset 6b397048940f (bug 1620098)

devtools/client/accessibility/test/browser/head.js

@@ -633,14 +633,9 @@ async function toggleMenuItem(doc, toolboxDoc, menuId, menuItemIndex) {
     menuItem.getAttribute("aria-checked") === "true" ? null : "true";
 
   // Make the menu visible first.
-  const onPopupShown = new Promise(r =>
-    toolboxDoc.addEventListener("popupshown", r, { once: true })
-  );
   EventUtils.synthesizeMouseAtCenter(menuButton, {}, panelWin);
-  await onPopupShown;
-  const boundingRect = menuItem.getBoundingClientRect();
-  ok(
-    boundingRect.width > 0 && boundingRect.height > 0,
+  await BrowserTestUtils.waitForCondition(
+    () => !!menuItem.offsetParent,
     "Menu item is visible."
   );
 

devtools/client/responsive/components/App.js

@@ -106,6 +106,9 @@ class App extends PureComponent {
     this.onToggleUserAgentInput = this.onToggleUserAgentInput.bind(this);
     this.onUpdateDeviceDisplayed = this.onUpdateDeviceDisplayed.bind(this);
     this.onUpdateDeviceModal = this.onUpdateDeviceModal.bind(this);
+    this.onUpdateDeviceSelectorMenu = this.onUpdateDeviceSelectorMenu.bind(
+      this
+    );
   }
 
   componentWillUnmount() {
@@ -400,6 +403,23 @@ class App extends PureComponent {
     }
   }
 
+  onUpdateDeviceSelectorMenu(isOpen) {
+    if (Services.prefs.getBoolPref("devtools.responsive.browserUI.enabled")) {
+      const rdmToolbar = window.parent.document.querySelector(".rdm-toolbar");
+      const browserStackEl = rdmToolbar.parentNode;
+
+      // Guarantee a fixed height for the HTMLTooltip to render inside.
+      const style = window.getComputedStyle(browserStackEl);
+      rdmToolbar.style.height = style.height;
+
+      if (isOpen) {
+        browserStackEl.classList.add("device-selector-menu-opened");
+      }
+
+      window.postMessage({ type: "update-device-toolbar-height", isOpen }, "*");
+    }
+  }
+
   render() {
     const { devices, networkThrottling, screenshot, viewports } = this.props;
 
@@ -427,6 +447,7 @@ class App extends PureComponent {
       onToggleUserAgentInput,
       onUpdateDeviceDisplayed,
       onUpdateDeviceModal,
+      onUpdateDeviceSelectorMenu,
     } = this;
 
     if (!viewports.length) {
@@ -465,6 +486,7 @@ class App extends PureComponent {
         onToggleReloadOnUserAgent,
         onToggleUserAgentInput,
         onUpdateDeviceModal,
+        onUpdateDeviceSelectorMenu,
       }),
       !Services.prefs.getBoolPref("devtools.responsive.browserUI.enabled")
         ? Viewports({

devtools/client/responsive/components/DeviceModal.js

@@ -108,7 +108,12 @@ class DeviceModal extends PureComponent {
   }
 
   onDeviceModalSubmit() {
-    const { devices, onDeviceListUpdate, onUpdateDeviceDisplayed } = this.props;
+    const {
+      devices,
+      onDeviceListUpdate,
+      onUpdateDeviceDisplayed,
+      onUpdateDeviceModal,
+    } = this.props;
 
     const preferredDevices = {
       added: new Set(),
@@ -132,6 +137,7 @@ class DeviceModal extends PureComponent {
     }
 
     onDeviceListUpdate(preferredDevices);
+    onUpdateDeviceModal(false);
   }
 
   onEditCustomDevice(newDevice) {

devtools/client/responsive/components/DeviceSelector.js

@@ -40,6 +40,7 @@ class DeviceSelector extends PureComponent {
       devices: PropTypes.shape(Types.devices).isRequired,
       onChangeDevice: PropTypes.func.isRequired,
       onUpdateDeviceModal: PropTypes.func.isRequired,
+      onUpdateDeviceSelectorMenu: PropTypes.func.isRequired,
       selectedDevice: PropTypes.string.isRequired,
       viewportId: PropTypes.number.isRequired,
     };
@@ -138,7 +139,7 @@ class DeviceSelector extends PureComponent {
   }
 
   render() {
-    const { devices } = this.props;
+    const { devices, onUpdateDeviceSelectorMenu } = this.props;
     const selectedDevice = this.getSelectedDevice();
     let { icon, label, tooltip } = this.getMenuProps(selectedDevice);
 
@@ -174,6 +175,8 @@ class DeviceSelector extends PureComponent {
         icon,
         title: tooltip,
         disabled: devices.listState !== Types.loadableState.LOADED,
+        onClick: () => onUpdateDeviceSelectorMenu(true),
+        onCloseButton: () => onUpdateDeviceSelectorMenu(false),
       },
       () => this.renderMenuList()
     );

devtools/client/responsive/components/Toolbar.js

@@ -61,6 +61,7 @@ class Toolbar extends PureComponent {
       onToggleReloadOnUserAgent: PropTypes.func.isRequired,
       onToggleUserAgentInput: PropTypes.func.isRequired,
       onUpdateDeviceModal: PropTypes.func.isRequired,
+      onUpdateDeviceSelectorMenu: PropTypes.func.isRequired,
       screenshot: PropTypes.shape(Types.screenshot).isRequired,
       selectedDevice: PropTypes.string.isRequired,
       selectedPixelRatio: PropTypes.number.isRequired,
@@ -107,6 +108,7 @@ class Toolbar extends PureComponent {
       onToggleReloadOnUserAgent,
       onToggleUserAgentInput,
       onUpdateDeviceModal,
+      onUpdateDeviceSelectorMenu,
       screenshot,
       selectedDevice,
       selectedPixelRatio,
@@ -131,6 +133,7 @@ class Toolbar extends PureComponent {
           devices,
           onChangeDevice,
           onUpdateDeviceModal,
+          onUpdateDeviceSelectorMenu,
           selectedDevice,
           viewportId: viewport.id,
         }),

devtools/client/responsive/test/browser/head.js

@@ -498,6 +498,25 @@ async function selectMenuItem({ toolWindow }, selector, value) {
  *         as an argument.
  */
 async function testMenuItems(toolWindow, button, testFn) {
+  if (button.id === "device-selector") {
+    // device-selector uses a DevTools MenuButton instead of a XUL menu
+    button.click();
+    // Wait for appearance the menu items..
+    await waitUntil(() =>
+      toolWindow.document.querySelector("#device-selector-menu .menuitem")
+    );
+    const tooltip = toolWindow.document.querySelector("#device-selector-menu");
+    const items = tooltip.querySelectorAll(".menuitem > .command");
+    testFn([...items]);
+
+    if (tooltip.classList.contains("tooltip-visible")) {
+      // Close the tooltip explicitly.
+      button.click();
+      await waitUntil(() => !tooltip.classList.contains("tooltip-visible"));
+    }
+    return;
+  }
+
   // The context menu appears only in the top level window, which is different from
   // the inner toolWindow.
   const win = getTopLevelWindow(toolWindow);
@@ -505,31 +524,13 @@ async function testMenuItems(toolWindow, button, testFn) {
   await new Promise(resolve => {
     win.document.addEventListener(
       "popupshown",
-      async () => {
-        if (button.id === "device-selector") {
-          const popup = toolWindow.document.querySelector(
-            "#device-selector-menu"
-          );
-          const menuItems = [...popup.querySelectorAll(".menuitem > .command")];
+      () => {
+        const popup = win.document.querySelector('menupopup[menu-api="true"]');
+        const menuItems = [...popup.children];
 
-          testFn(menuItems);
-
-          if (popup.classList.contains("tooltip-visible")) {
-            // Close the tooltip explicitly.
-            button.click();
-            await waitUntil(() => !popup.classList.contains("tooltip-visible"));
-          }
-        } else {
-          const popup = win.document.querySelector(
-            'menupopup[menu-api="true"]'
-          );
-          const menuItems = [...popup.children];
-
-          testFn(menuItems);
-
-          popup.hidePopup();
-        }
+        testFn(menuItems);
 
+        popup.hidePopup();
         resolve();
       },
       { once: true }

devtools/client/responsive/ui.js

@@ -304,7 +304,11 @@ class ResponsiveUI {
 
           // If the device modal/selector is opened, resize the toolbar height to
           // the size of the stack.
-          if (this.browserStackEl.classList.contains("device-modal-opened")) {
+          if (
+            this.browserStackEl.classList.contains(
+              "device-selector-menu-opened"
+            )
+          ) {
             const style = this.browserWindow.getComputedStyle(
               this.browserStackEl
             );
@@ -582,6 +586,8 @@ class ResponsiveUI {
       case "update-device-modal":
         this.onUpdateDeviceModal(event);
         break;
+      case "update-device-toolbar-height":
+        this.onUpdateToolbarHeight(event);
     }
   }
 
@@ -801,13 +807,31 @@ class ResponsiveUI {
   }
 
   onUpdateDeviceModal(event) {
-    if (event.data.isOpen) {
-      this.browserStackEl.classList.add("device-modal-opened");
-      const style = this.browserWindow.getComputedStyle(this.browserStackEl);
-      this.rdmFrame.style.height = style.height;
-    } else {
-      this.rdmFrame.style.removeProperty("height");
-      this.browserStackEl.classList.remove("device-modal-opened");
+    // Restore the toolbar height if closing
+    if (!event.data.isOpen) {
+      this.restoreToolbarHeight();
+    }
+  }
+
+  /**
+   * Handles setting the height of the toolbar when it's closed. This can happen when
+   * an event occurs outside of the device selector menu component, such as opening the
+   * device modal.
+   */
+  onUpdateToolbarHeight(event) {
+    if (!event.data.isOpen) {
+      const {
+        isModalOpen,
+      } = this.rdmFrame.contentWindow.store.getState().devices;
+
+      // Don't remove the device-selector-menu-opened class if it was closed because
+      // the device modal was opened. We still want to preserve the current height of
+      // toolbar.
+      if (isModalOpen) {
+        return;
+      }
+
+      this.restoreToolbarHeight();
     }
   }
 
@@ -818,6 +842,14 @@ class ResponsiveUI {
     return !!deviceState;
   }
 
+  /**
+   * Restores the toolbar's height to it's original class styling.
+   */
+  restoreToolbarHeight() {
+    this.rdmFrame.style.removeProperty("height");
+    this.browserStackEl.classList.remove("device-selector-menu-opened");
+  }
+
   /**
    * Restores the previous UI state.
    */

devtools/client/shared/widgets/tooltip/HTMLTooltip.js

@@ -374,7 +374,7 @@ function HTMLTooltip(
   // consumeOutsideClicks cannot be used if the tooltip is not closed on click
   this.consumeOutsideClicks = this.noAutoHide ? false : consumeOutsideClicks;
   this.isMenuTooltip = isMenuTooltip;
-  this.useXulWrapper = this._isXULPopupAvailable() && useXulWrapper;
+  this.useXulWrapper = this._isXUL() && useXulWrapper;
   this.preferredWidth = "auto";
   this.preferredHeight = "auto";
 
@@ -404,7 +404,7 @@ function HTMLTooltip(
     this.doc.documentElement.appendChild(this.xulPanelWrapper);
     this.xulPanelWrapper.appendChild(inner);
     inner.appendChild(this.container);
-  } else if (this._hasXULRootElement()) {
+  } else if (this._isXUL()) {
     this.doc.documentElement.appendChild(this.container);
   } else {
     // In non-XUL context the container is ready to use as is.
@@ -979,16 +979,12 @@ HTMLTooltip.prototype = {
   },
 
   /**
-   * Check if the tooltip's owner document has XUL root element.
+   * Check if the tooltip's owner document is a XUL document.
    */
-  _hasXULRootElement: function() {
+  _isXUL: function() {
     return this.doc.documentElement.namespaceURI === XUL_NS;
   },
 
-  _isXULPopupAvailable: function() {
-    return this.doc.nodePrincipal.isSystemPrincipal;
-  },
-
   _createXulPanelWrapper: function() {
     const panel = this.doc.createXULElement("panel");
 

devtools/client/themes/tooltips.css

@@ -133,13 +133,7 @@ strong {
 }
 
 .tooltip-xul-wrapper .tooltip-container {
-  /**
-   * As the width/height of xul:panel (.tooltip-xul-wrapper) is dependent on the
-   * width/height of the inner content. Thus, if "absolute" position here, as the
-   * width/height that are conveyed to the panel will be zero, the tooltip will be
-   * invisible.
-   */
-  position: static;
+  position: absolute;
 }
 
 .tooltip-top {