From 885de209fb9a35a0c77b43be3d2647823aa1321c Mon Sep 17 00:00:00 2001 From: Paolo Amadini Date: Mon, 26 Feb 2018 13:18:22 +0000 Subject: [PATCH] Bug 1440333 - Part 3 - Raise the ViewShown event after the main view is active. r=Gijs This also adds a new "active" property that can be used by regression tests to determine whether they should still wait for the ViewShown event. MozReview-Commit-ID: K25F09llooj --HG-- extra : rebase_source : 8c9e2a86285be6453cd60b3da448876e6dfa7e8a --- .../customizableui/PanelMultiView.jsm | 52 ++++++++++++++++--- ...r_981418-widget-onbeforecreated-handler.js | 21 +++----- 2 files changed, 52 insertions(+), 21 deletions(-) diff --git a/browser/components/customizableui/PanelMultiView.jsm b/browser/components/customizableui/PanelMultiView.jsm index 06105467c188..c535a0c920a4 100644 --- a/browser/components/customizableui/PanelMultiView.jsm +++ b/browser/components/customizableui/PanelMultiView.jsm @@ -55,6 +55,20 @@ * When navigating backwards, an open subview will first become invisible and * then will be closed. * + * -- Active or inactive + * + * This indicates whether the view is fully scrolled into the visible area + * and ready to receive mouse and keyboard events. An active view is always + * visible, but a visible view may be inactive. For example, during a scroll + * transition, both views will be inactive. + * + * When a view becomes active, the ViewShown event is fired synchronously. + * For the main view of the panel, this happens during the "popupshown" + * event, which means that other "popupshown" handlers may be called before + * the view is active. However, the main view can already receive mouse and + * keyboard events at this point, only because this allows regression tests + * to use the "popupshown" event to simulate interaction. + * * -- Navigating with the keyboard * * An open view may keep state related to keyboard navigation, even if it is @@ -673,7 +687,7 @@ var PanelMultiView = class extends this.AssociatedToNode { } await this._transitionViews(prevPanelView.node, viewNode, false, anchor); - this._viewShown(nextPanelView); + this._activateView(nextPanelView); } /** @@ -697,7 +711,7 @@ var PanelMultiView = class extends this.AssociatedToNode { this._closeLatestView(); - this._viewShown(nextPanelView); + this._activateView(nextPanelView); } /** @@ -734,7 +748,6 @@ var PanelMultiView = class extends this.AssociatedToNode { nextPanelView.visible = true; nextPanelView.descriptionHeightWorkaround(); - this._viewShown(nextPanelView); return true; } @@ -774,10 +787,12 @@ var PanelMultiView = class extends this.AssociatedToNode { } /** - * Raises the ViewShown event if the specified view is still open. + * Activates the specified view and raises the ViewShown event, unless the + * view was closed in the meantime. */ - _viewShown(panelView) { + _activateView(panelView) { if (panelView.node.panelMultiView == this.node) { + panelView.active = true; panelView.dispatchCustomEvent("ViewShown"); } } @@ -939,6 +954,14 @@ var PanelMultiView = class extends this.AssociatedToNode { await window.promiseDocumentFlushed(() => {}); + // For proper bookkeeping, mark the view that is about to scrolled out of + // the visible area as inactive, because it won't be possible to simulate + // mouse events on it properly. In practice this isn't important, because we + // use the separate "transitioning" attribute on the panel to suppress + // pointer events. This allows mouse events to be available for the main + // view in regression tests that wait for the "popupshown" event. + prevPanelView.active = false; + // Kick off the transition! details.phase = TRANSITION_PHASES.TRANSITION; this._viewStack.style.transform = "translateX(" + (moveToLeft ? "" : "-") + deltaX + "px)"; @@ -1123,9 +1146,11 @@ var PanelMultiView = class extends this.AssociatedToNode { break; } case "popupshown": + let mainPanelView = PanelView.forNode(this._mainView); // Now that the main view is visible, we can check the height of the // description elements it contains. - PanelView.forNode(this._mainView).descriptionHeightWorkaround(); + mainPanelView.descriptionHeightWorkaround(); + this._activateView(mainPanelView); break; case "popuphidden": { // WebExtensions consumers can hide the popup from viewshowing, or @@ -1155,6 +1180,16 @@ var PanelMultiView = class extends this.AssociatedToNode { * This is associated to elements. */ var PanelView = class extends this.AssociatedToNode { + constructor(node) { + super(node); + + /** + * Indicates whether the view is active. When this is false, consumers can + * wait for the ViewShown event to know when the view becomes active. + */ + this.active = false; + } + /** * The "mainview" attribute is set before the panel is opened when this view * is displayed as the main view, and is removed before the is @@ -1169,11 +1204,16 @@ var PanelView = class extends this.AssociatedToNode { } } + /** + * Determines whether the view is visible. Setting this to false also resets + * the "active" property. + */ set visible(value) { if (value) { this.node.setAttribute("visible", true); } else { this.node.removeAttribute("visible"); + this.active = false; } } diff --git a/browser/components/customizableui/test/browser_981418-widget-onbeforecreated-handler.js b/browser/components/customizableui/test/browser_981418-widget-onbeforecreated-handler.js index 74efd4bc150e..f3ec519b22f7 100644 --- a/browser/components/customizableui/test/browser_981418-widget-onbeforecreated-handler.js +++ b/browser/components/customizableui/test/browser_981418-widget-onbeforecreated-handler.js @@ -31,21 +31,12 @@ add_task(async function testAddOnBeforeCreatedWidget() { ok(widgetNode, "Widget should exist"); ok(viewNode, "Panelview should exist"); - let widgetPanel; - let panelShownPromise; - let viewShownPromise = new Promise(resolve => { - viewNode.addEventListener("ViewShown", () => { - widgetPanel = document.getElementById("customizationui-widget-panel"); - ok(widgetPanel, "Widget panel should exist"); - // Add the popupshown event listener directly inside the ViewShown event - // listener to avoid missing the event. - panelShownPromise = promisePanelElementShown(window, widgetPanel); - resolve(); - }, { once: true }); - }); + let viewShownPromise = BrowserTestUtils.waitForEvent(viewNode, "ViewShown"); widgetNode.click(); await viewShownPromise; - await panelShownPromise; + + let widgetPanel = document.getElementById("customizationui-widget-panel"); + ok(widgetPanel, "Widget panel should exist"); let panelHiddenPromise = promisePanelElementHidden(window, widgetPanel); widgetPanel.hidePopup(); @@ -55,9 +46,9 @@ add_task(async function testAddOnBeforeCreatedWidget() { await waitForOverflowButtonShown(); await document.getElementById("nav-bar").overflowable.show(); + viewShownPromise = BrowserTestUtils.waitForEvent(viewNode, "ViewShown"); widgetNode.click(); - - await BrowserTestUtils.waitForEvent(viewNode, "ViewShown"); + await viewShownPromise; let panelHidden = promiseOverflowHidden(window); PanelUI.overflowPanel.hidePopup();