| author | Paolo Amadini <paolo.mozmail@amadzone.org> |
| Thu, 23 Aug 2018 14:04:43 +0100 | |
| changeset 433259 | abd29769ce45f110299ef6bf4c667d5df496d0c1 |
| parent 433258 | f433ed4ff1de863250d6ebf82b5bc5afcea9b6c7 |
| child 433260 | 3eff5274ee785a1f8ecaa2467ee061323962cbe8 |
| push id | 34501 |
| push user | toros@mozilla.com |
| push date | Fri, 24 Aug 2018 09:45:02 +0000 |
| treeherder | mozilla-central@190b827aaa2b [default view] [failures only] |
| perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
| reviewers | Gijs |
| bugs | 1484275 |
| milestone | 63.0a1 |
| 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
|
--- a/browser/components/customizableui/PanelMultiView.jsm +++ b/browser/components/customizableui/PanelMultiView.jsm @@ -511,16 +511,25 @@ var PanelMultiView = class extends Assoc } // We have to set canCancel to false before opening the popup because the // hidePopup method of PanelMultiView can be re-entered by event handlers. // If the openPopup call fails, however, we still have to dispatch the // "popuphidden" event even if canCancel was set to false. 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"); throw ex; } }); } @@ -1051,18 +1060,20 @@ var PanelMultiView = class extends Assoc // sense for all platforms. If the arrow visuals change significantly, // this value will be easy to adjust. const EXTRA_MARGIN_PX = 20; maxHeight -= EXTRA_MARGIN_PX; return maxHeight; } 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) { case "keydown": // Since we start listening for the "keydown" event when the popup is // already showing and stop listening when the panel is hidden, we // always have at least one view open. let currentView = this.openViews[this.openViews.length - 1];
--- a/browser/components/customizableui/test/browser.ini +++ b/browser/components/customizableui/test/browser.ini @@ -139,16 +139,17 @@ skip-if = os == "linux" # crashing on Li [browser_1087303_button_fullscreen.js] tags = fullscreen skip-if = os == "mac" [browser_1087303_button_preferences.js] [browser_1089591_still_customizable_after_reset.js] [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] [browser_customizemode_contextmenu_menubuttonstate.js] [browser_customizemode_dragspace.js] skip-if = os == "linux" # linux doesn't get drag space (no tabsintitlebar) [browser_customizemode_uidensity.js] [browser_drag_outside_palette.js]
new file mode 100644 --- /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,<html><body></body></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; + }); +});
--- a/browser/components/customizableui/test/browser_PanelMultiView.js +++ b/browser/components/customizableui/test/browser_PanelMultiView.js @@ -401,16 +401,17 @@ add_task(async function test_cancel_main "bottomcenter topright"); await promiseHidden; stopRecordingEvents(gPanels[0]); Assert.deepEqual(recordArray, [ "panelview-0: ViewShowing", "panelview-0: ViewHiding", + "panelmultiview-0: PanelMultiViewHidden", "panelmultiview-0: popuphidden", ]); }); /** * Tests the event sequence when opening a subview is canceled. */ add_task(async function test_cancel_subview_event_sequence() { @@ -470,18 +471,19 @@ add_task(async function test_close_while "bottomcenter topright"); await promiseHiding; await promiseHidden; stopRecordingEvents(gPanels[0]); Assert.deepEqual(recordArray, [ "panelview-0: ViewShowing", + "panelview-0: ViewShowing > panelview-0: ViewHiding", + "panelview-0: ViewShowing > panelmultiview-0: PanelMultiViewHidden", "panelview-0: ViewShowing > panelmultiview-0: popuphidden", - "panelview-0: ViewShowing > panelview-0: ViewHiding", ]); }); /** * Tests the event sequence when closing the panel while opening a subview. */ add_task(async function test_close_while_showing_subview_event_sequence() { let recordArray = [];