diff --git a/devtools/client/accessibility/test/browser/head.js b/devtools/client/accessibility/test/browser/head.js index 5bc67d2ba984..8a9d05773c0e 100644 --- a/devtools/client/accessibility/test/browser/head.js +++ b/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." ); diff --git a/devtools/client/responsive/components/App.js b/devtools/client/responsive/components/App.js index 36380f8ea7c0..eb6a024dd3f5 100644 --- a/devtools/client/responsive/components/App.js +++ b/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({ diff --git a/devtools/client/responsive/components/DeviceModal.js b/devtools/client/responsive/components/DeviceModal.js index 7c1f0f924767..01f9de24f3de 100644 --- a/devtools/client/responsive/components/DeviceModal.js +++ b/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) { diff --git a/devtools/client/responsive/components/DeviceSelector.js b/devtools/client/responsive/components/DeviceSelector.js index 2343ab13f279..8fec94ae3f60 100644 --- a/devtools/client/responsive/components/DeviceSelector.js +++ b/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() ); diff --git a/devtools/client/responsive/components/Toolbar.js b/devtools/client/responsive/components/Toolbar.js index 1a731ec0a656..363dd5e59b08 100644 --- a/devtools/client/responsive/components/Toolbar.js +++ b/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, }), diff --git a/devtools/client/responsive/test/browser/head.js b/devtools/client/responsive/test/browser/head.js index bdb9cd9e9b23..e7af2c22574c 100644 --- a/devtools/client/responsive/test/browser/head.js +++ b/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 } diff --git a/devtools/client/responsive/ui.js b/devtools/client/responsive/ui.js index e468cffce531..f446a9618dac 100644 --- a/devtools/client/responsive/ui.js +++ b/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. */ diff --git a/devtools/client/shared/widgets/tooltip/HTMLTooltip.js b/devtools/client/shared/widgets/tooltip/HTMLTooltip.js index 8409fae798a9..47cc1497d0a8 100644 --- a/devtools/client/shared/widgets/tooltip/HTMLTooltip.js +++ b/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"); diff --git a/devtools/client/themes/tooltips.css b/devtools/client/themes/tooltips.css index ab190130c5d5..e5cb3d4a0d09 100644 --- a/devtools/client/themes/tooltips.css +++ b/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 {