From 866357cbca569bdfa04d982bd0eb7848bb3f9db4 Mon Sep 17 00:00:00 2001 From: Paolo Amadini Date: Thu, 23 Aug 2018 14:04:43 +0100 Subject: [PATCH] Bug 1484275 - Fix opening the main menu while another popup is open on Windows. r=Gijs Differential Revision: https://phabricator.services.mozilla.com/D3979 --HG-- extra : rebase_source : 5cf5b9fe368de3c78765cb154ef6532e2340d400 --- .../customizableui/PanelMultiView.jsm | 15 +++- .../customizableui/test/browser.ini | 1 + ..._PanelMultiView_toggle_with_other_popup.js | 87 +++++++++++++++++++ .../test/browser_PanelMultiView.js | 4 +- 4 files changed, 104 insertions(+), 3 deletions(-) create mode 100644 browser/components/customizableui/test/browser_1484275_PanelMultiView_toggle_with_other_popup.js diff --git a/browser/components/customizableui/PanelMultiView.jsm b/browser/components/customizableui/PanelMultiView.jsm index a242de3b6138..6b2d195abe6e 100644 --- a/browser/components/customizableui/PanelMultiView.jsm +++ b/browser/components/customizableui/PanelMultiView.jsm @@ -516,6 +516,15 @@ var PanelMultiView = class extends AssociatedToNode { try { canCancel = false; this._panel.openPopup(...args); + + // On Windows, if another popup is hiding while we call openPopup, the + // call won't fail but the popup won't open. In this case, we have to + // dispatch an artificial "popuphidden" event to reset our state. + if (this._panel.state == "closed" && this.openViews.length) { + this.dispatchCustomEvent("popuphidden"); + return false; + } + return true; } catch (ex) { this.dispatchCustomEvent("popuphidden"); @@ -1056,8 +1065,10 @@ var PanelMultiView = class extends AssociatedToNode { } handleEvent(aEvent) { - if (aEvent.type.startsWith("popup") && aEvent.target != this._panel) { - // Shouldn't act on e.g. context menus being shown from within the panel. + // Only process actual popup events from the panel or events we generate + // ourselves, but not from menus being shown from within the panel. + if (aEvent.type.startsWith("popup") && aEvent.target != this._panel && + aEvent.target != this.node) { return; } switch (aEvent.type) { diff --git a/browser/components/customizableui/test/browser.ini b/browser/components/customizableui/test/browser.ini index 0947cf716551..3ce405ba5820 100644 --- a/browser/components/customizableui/test/browser.ini +++ b/browser/components/customizableui/test/browser.ini @@ -144,6 +144,7 @@ skip-if = os == "mac" [browser_1096763_seen_widgets_post_reset.js] [browser_1161838_inserted_new_default_buttons.js] skip-if = verify +[browser_1484275_PanelMultiView_toggle_with_other_popup.js] [browser_allow_dragging_removable_false.js] [browser_bootstrapped_custom_toolbar.js] [browser_currentset_post_reset.js] diff --git a/browser/components/customizableui/test/browser_1484275_PanelMultiView_toggle_with_other_popup.js b/browser/components/customizableui/test/browser_1484275_PanelMultiView_toggle_with_other_popup.js new file mode 100644 index 000000000000..392bfcae4231 --- /dev/null +++ b/browser/components/customizableui/test/browser_1484275_PanelMultiView_toggle_with_other_popup.js @@ -0,0 +1,87 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +const TEST_URL = "data:text/html,"; + +// This code can be consolidated in the EventUtils module (bug 1126772). +const isWindows = AppConstants.platform == "win"; +const isMac = AppConstants.platform == "macosx"; +const mouseDown = isWindows ? 2 : isMac ? 1 : 4; +const mouseUp = isWindows ? 4 : isMac ? 2 : 7; +const utils = window.windowUtils; +const scale = utils.screenPixelsPerCSSPixel; +function synthesizeNativeMouseClick(aElement) { + let rect = aElement.getBoundingClientRect(); + let win = aElement.ownerGlobal; + let x = win.mozInnerScreenX + (rect.left + rect.right) / 2; + let y = win.mozInnerScreenY + (rect.top + rect.bottom) / 2; + + // Wait for the mouseup event to occur before continuing. + return new Promise((resolve, reject) => { + function eventOccurred(e) { + aElement.removeEventListener("mouseup", eventOccurred, true); + resolve(); + } + + aElement.addEventListener("mouseup", eventOccurred, true); + + utils.sendNativeMouseEvent(x * scale, y * scale, mouseDown, 0, null); + utils.sendNativeMouseEvent(x * scale, y * scale, mouseUp, 0, null); + }); +} + +/** + * Test steps that may lead to the panel being stuck on Windows (bug 1484275). + */ +add_task(async function test_PanelMultiView_toggle_with_other_popup() { + // For proper cleanup, create a bookmark that we will remove later. + let bookmark = await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.unfiledGuid, + url: TEST_URL, + }); + registerCleanupFunction(() => PlacesUtils.bookmarks.remove(bookmark)); + + await BrowserTestUtils.withNewTab({ + gBrowser, + url: TEST_URL, + }, async function(browser) { + // 1. Open the main menu. + await gCUITestUtils.openMainMenu(); + + // 2. Open another popup not managed by PanelMultiView. + let bookmarkPanel = document.getElementById("editBookmarkPanel"); + let shown = BrowserTestUtils.waitForEvent(bookmarkPanel, "popupshown"); + let hidden = BrowserTestUtils.waitForEvent(bookmarkPanel, "popuphidden"); + EventUtils.synthesizeKey("D", { accelKey: true }); + await shown; + + // 3. Click the button to which the main menu is anchored. We need a native + // mouse event to simulate the exact platform behavior with popups. + let clickFn = () => synthesizeNativeMouseClick( + document.getElementById("PanelUI-button")); + + if (AppConstants.platform == "win") { + // On Windows, the operation will close both popups. + await gCUITestUtils.hidePanelMultiView(PanelUI.panel, clickFn); + await new Promise(resolve => executeSoon(resolve)); + + // 4. Test that the popup can be opened again after it's been closed. + await gCUITestUtils.openMainMenu(); + Assert.equal(PanelUI.panel.state, "open"); + } else { + // On other platforms, the operation will close both popups and reopen the + // main menu immediately, so we wait for the reopen to occur. + shown = BrowserTestUtils.waitForEvent(PanelUI.mainView, "ViewShown"); + clickFn(); + await shown; + } + + await gCUITestUtils.hideMainMenu(); + + // Make sure the events for the bookmarks panel have also been processed + // before closing the tab and removing the bookmark. + await hidden; + }); +}); diff --git a/browser/components/customizableui/test/browser_PanelMultiView.js b/browser/components/customizableui/test/browser_PanelMultiView.js index f8cb0d2276ec..54fe4c691f19 100644 --- a/browser/components/customizableui/test/browser_PanelMultiView.js +++ b/browser/components/customizableui/test/browser_PanelMultiView.js @@ -406,6 +406,7 @@ add_task(async function test_cancel_mainview_event_sequence() { Assert.deepEqual(recordArray, [ "panelview-0: ViewShowing", "panelview-0: ViewHiding", + "panelmultiview-0: PanelMultiViewHidden", "panelmultiview-0: popuphidden", ]); }); @@ -475,8 +476,9 @@ add_task(async function test_close_while_showing_mainview_event_sequence() { Assert.deepEqual(recordArray, [ "panelview-0: ViewShowing", - "panelview-0: ViewShowing > panelmultiview-0: popuphidden", "panelview-0: ViewShowing > panelview-0: ViewHiding", + "panelview-0: ViewShowing > panelmultiview-0: PanelMultiViewHidden", + "panelview-0: ViewShowing > panelmultiview-0: popuphidden", ]); });