Bug 1354117 - only dispatch view events once, and fix synced tabs button test, r?jaws
Prior to this patch, both CustomizableUI itself and the PanelMultiView module
tried to ensure that onViewShowing/Shown/Hiding/Hidden listeners were invoked
when the relevant DOM events fired.
PanelMultiView was doing this manually because CUI was only adding listeners
once the corresponding widget was created. Now that the relevant views can be
accessed without the corresponding widgets (via the fixed appMenu), there was
no guarantee that the listeners would be attached, and this caused empty
subviews.
Unfortunately, if the widget *was* present, it caused events to fire more than
once, which understandably broke consumers like the sync remote tabs widget,
which broke the test we're fixing up here. For other views, even if they were
not completely broken it at least did busy-work.
This patch removes the manual event invocation, and delegates the event
listener work to CUI from the PanelMultiView side. This ensures events fire,
and fire only once.
MozReview-Commit-ID: 94GhcrdcBuB
--- a/browser/components/customizableui/CustomizableUI.jsm
+++ b/browser/components/customizableui/CustomizableUI.jsm
@@ -1448,40 +1448,51 @@ var CustomizableUIInternal = {
if (aWidget.type == "view") {
log.debug("Widget " + aWidget.id + " has a view. Auto-registering event handlers.");
let viewNode = aDocument.getElementById(aWidget.viewId);
if (viewNode) {
// PanelUI relies on the .PanelUI-subView class to be able to show only
// one sub-view at a time.
viewNode.classList.add("PanelUI-subView");
-
- for (let eventName of kSubviewEvents) {
- let handler = "on" + eventName;
- if (typeof aWidget[handler] == "function") {
- viewNode.addEventListener(eventName, aWidget[handler]);
- }
- }
-
- log.debug("Widget " + aWidget.id + " showing and hiding event handlers set.");
+ this.ensureSubviewListeners(viewNode);
} else {
log.error("Could not find the view node with id: " + aWidget.viewId +
", for widget: " + aWidget.id + ".");
}
}
if (aWidget.onCreated) {
aWidget.onCreated(node);
}
}
aWidget.instances.set(aDocument, node);
return node;
},
+ ensureSubviewListeners(viewNode) {
+ if (viewNode._addedEventListeners) {
+ return;
+ }
+ let viewId = viewNode.id;
+ let widget = [...gPalette.values()].find(w => w.viewId == viewId);
+ if (!widget) {
+ return;
+ }
+ for (let eventName of kSubviewEvents) {
+ let handler = "on" + eventName;
+ if (typeof widget[handler] == "function") {
+ viewNode.addEventListener(eventName, widget[handler]);
+ }
+ }
+ viewNode._addedEventListeners = true;
+ log.debug("Widget " + widget.id + " showing and hiding event handlers set.");
+ },
+
getLocalizedProperty(aWidget, aProp, aFormatArgs, aDef) {
const kReqStringProps = ["label"];
if (typeof aWidget == "string") {
aWidget = gPalette.get(aWidget);
}
if (!aWidget) {
throw new Error("getLocalizedProperty was passed a non-widget to work with.");
@@ -3509,16 +3520,27 @@ this.CustomizableUI = {
* or if the area is not currently known to CustomizableUI.
*/
getWidgetsInArea(aArea) {
return this.getWidgetIdsInArea(aArea).map(
CustomizableUIInternal.wrapWidget,
CustomizableUIInternal
);
},
+
+ /**
+ * Ensure the customizable widget that matches up with this view node
+ * will get the right subview showing/shown/hiding/hidden events when
+ * they fire.
+ * @param aViewNode the view node to add listeners to if they haven't
+ * been added already.
+ */
+ ensureSubviewListeners(aViewNode) {
+ return CustomizableUIInternal.ensureSubviewListeners(aViewNode);
+ },
/**
* Obtain an array of all the area IDs known to CustomizableUI.
* This array is created for you, so is modifiable without CustomizableUI
* being affected.
*/
get areas() {
return [...gAreas.keys()];
},
--- a/browser/components/customizableui/CustomizableWidgets.jsm
+++ b/browser/components/customizableui/CustomizableWidgets.jsm
@@ -965,16 +965,19 @@ const CustomizableWidgets = [
if (elem.value.toLowerCase() == aCurrentItem.toLowerCase()) {
elem.setAttribute("checked", "true");
} else {
elem.removeAttribute("checked");
}
}
},
onViewShowing(aEvent) {
+ if (!this._inited) {
+ this.onInit();
+ }
let document = aEvent.target.ownerDocument;
let autoDetectLabelId = "PanelUI-characterEncodingView-autodetect-label";
let autoDetectLabel = document.getElementById(autoDetectLabelId);
if (!autoDetectLabel.hasAttribute("value")) {
let label = CharsetBundle.GetStringFromName("charsetMenuAutodet");
autoDetectLabel.setAttribute("value", label);
this.populateList(document,
@@ -1057,16 +1060,17 @@ const CustomizableWidgets = [
CustomizableUI.removeListener(listener);
getPanel().removeEventListener("popupshowing", updateButton);
}
};
CustomizableUI.addListener(listener);
this.onInit();
},
onInit() {
+ this._inited = true;
if (!this.charsetInfo) {
this.charsetInfo = CharsetMenu.getData();
}
}
}, {
id: "email-link-button",
tooltiptext: "email-link-button.tooltiptext3",
onCommand(aEvent) {
--- a/browser/components/customizableui/PanelMultiView.jsm
+++ b/browser/components/customizableui/PanelMultiView.jsm
@@ -6,18 +6,18 @@
this.EXPORTED_SYMBOLS = ["PanelMultiView"];
const {classes: Cc, interfaces: Ci, utils: Cu} = Components;
Cu.import("resource://gre/modules/XPCOMUtils.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "AppConstants",
"resource://gre/modules/AppConstants.jsm");
-XPCOMUtils.defineLazyModuleGetter(this, "CustomizableWidgets",
- "resource:///modules/CustomizableWidgets.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "CustomizableUI",
+ "resource:///modules/CustomizableUI.jsm");
/**
* Simple implementation of the sliding window pattern; panels are added to a
* linked list, in-order, and the currently shown panel is remembered using a
* marker. The marker shifts as navigation between panels is continued, where
* the panel at index 0 is always the starting point:
* ┌────┬────┬────┬────┐
* │▓▓▓▓│ │ │ │ Start
@@ -671,24 +671,19 @@ this.PanelMultiView = class {
* @param {String} eventName Name of the event to dispatch.
* @param {DOMNode} [anchor] Node where the panel is anchored to. Optional.
* @param {Object} [detail] Event detail object. Optional.
* @return {Boolean} `true` if the event was canceled by an event handler, `false`
* otherwise.
*/
_dispatchViewEvent(viewNode, eventName, anchor, detail) {
let cancel = false;
- if (this.panelViews) {
- let custWidget = CustomizableWidgets.find(widget => widget.viewId == viewNode.id);
- let method = "on" + eventName;
- if (custWidget && custWidget[method]) {
- if (anchor && custWidget.onInit)
- custWidget.onInit(anchor);
- custWidget[method]({ target: viewNode, preventDefault: () => cancel = true, detail });
- }
+ if (eventName != "PanelMultiViewHidden" && eventName != "destructed") {
+ // Don't need to do this for PanelMultiViewHidden or "destructed" events
+ CustomizableUI.ensureSubviewListeners(viewNode);
}
let evt = new this.window.CustomEvent(eventName, {
detail,
bubbles: true,
cancelable: eventName == "ViewShowing"
});
viewNode.dispatchEvent(evt);
--- a/browser/components/customizableui/test/browser_synced_tabs_menu.js
+++ b/browser/components/customizableui/test/browser_synced_tabs_menu.js
@@ -34,17 +34,16 @@ let mockedInternal = {
get isConfiguredToSyncTabs() { return true; },
getTabClients() { return Promise.resolve([]); },
syncTabs() { return Promise.resolve(); },
hasSyncedThisSession: false,
};
add_task(async function setup() {
- await SpecialPowers.pushPrefEnv({set: [["browser.photon.structure.enabled", false]]});
let oldInternal = SyncedTabs._internal;
SyncedTabs._internal = mockedInternal;
// This test hacks some observer states to simulate a user being signed
// in to Sync - restore them when the test completes.
let initialObserverStates = {};
for (let id of ["sync-reauth-state", "sync-setup-state", "sync-syncnow-state"]) {
initialObserverStates[id] = document.getElementById(id).hidden;
@@ -57,32 +56,34 @@ add_task(async function setup() {
}
});
});
// The test expects the about:preferences#sync page to open in the current tab
async function openPrefsFromMenuPanel(expectedPanelId, entryPoint) {
info("Check Sync button functionality");
Services.prefs.setCharPref("identity.fxaccounts.remote.signup.uri", "http://example.com/");
+ CustomizableUI.addWidgetToArea("sync-button", CustomizableUI.AREA_FIXED_OVERFLOW_PANEL);
// check the button's functionality
- await PanelUI.show();
+ await document.getElementById("nav-bar").overflowable.show();
if (entryPoint == "uitour") {
UITour.tourBrowsersByWindow.set(window, new Set());
UITour.tourBrowsersByWindow.get(window).add(gBrowser.selectedBrowser);
}
let syncButton = document.getElementById("sync-button");
ok(syncButton, "The Sync button was added to the Panel Menu");
let tabsUpdatedPromise = promiseObserverNotified("synced-tabs-menu:test:tabs-updated");
+ let syncPanel = document.getElementById("PanelUI-remotetabs");
+ let viewShownPromise = BrowserTestUtils.waitForEvent(syncPanel, "ViewShown");
syncButton.click();
- await tabsUpdatedPromise;
- let syncPanel = document.getElementById("PanelUI-remotetabs");
+ await Promise.all([tabsUpdatedPromise, viewShownPromise]);
ok(syncPanel.getAttribute("current"), "Sync Panel is in view");
// Sync is not configured - verify that state is reflected.
let subpanel = document.getElementById(expectedPanelId)
ok(!subpanel.hidden, "sync setup element is visible");
// Find and click the "setup" button.
let setupButton = subpanel.querySelector(".PanelUI-remotetabs-prefs-button");
@@ -100,26 +101,26 @@ async function openPrefsFromMenuPanel(ex
}
gBrowser.selectedBrowser.addEventListener("load", handler, true);
});
newTab = gBrowser.selectedTab;
is(gBrowser.currentURI.spec, "about:preferences?entrypoint=" + entryPoint + "#sync",
"Firefox Sync preference page opened with `menupanel` entrypoint");
- ok(!isPanelUIOpen(), "The panel closed");
+ ok(!isOverflowOpen(), "The panel closed");
- if (isPanelUIOpen()) {
- await panelUIHide();
+ if (isOverflowOpen()) {
+ await hideOverflow();
}
}
-function panelUIHide() {
- let panelHidePromise = promisePanelHidden(window);
- PanelUI.hide();
+function hideOverflow() {
+ let panelHidePromise = promiseOverflowHidden(window);
+ PanelUI.overflowPanel.hidePopup();
return panelHidePromise;
}
async function asyncCleanup() {
Services.prefs.clearUserPref("identity.fxaccounts.remote.signup.uri");
// reset the panel UI to the default state
await resetCustomization();
ok(CustomizableUI.inDefaultState, "The panel UI is in default state again.");
@@ -161,20 +162,20 @@ add_task(async function() {
let syncPanel = document.getElementById("PanelUI-remotetabs");
let links = syncPanel.querySelectorAll(".remotetabs-promo-link");
is(links.length, 2, "found 2 links as expected");
// test each link and left and middle mouse buttons
for (let link of links) {
for (let button = 0; button < 2; button++) {
- await PanelUI.show();
+ await document.getElementById("nav-bar").overflowable.show();
EventUtils.sendMouseEvent({ type: "click", button }, link, window);
// the panel should have been closed.
- ok(!isPanelUIOpen(), "click closed the panel");
+ ok(!isOverflowOpen(), "click closed the panel");
// should be a new tab - wait for the load.
is(gBrowser.tabs.length, 2, "there's a new tab");
await new Promise(resolve => {
if (gBrowser.selectedBrowser.currentURI.spec == "about:blank") {
gBrowser.selectedBrowser.addEventListener("load", function(e) {
resolve();
}, {capture: true, once: true});
return;
@@ -187,41 +188,43 @@ add_task(async function() {
let os = link.getAttribute("mobile-promo-os");
let expectedUrl = `http://example.com/?os=${os}&tail=synced-tabs`;
is(gBrowser.selectedBrowser.currentURI.spec, expectedUrl, "correct URL");
gBrowser.removeTab(gBrowser.selectedTab);
}
}
// test each link and right mouse button - should be a noop.
- await PanelUI.show();
+ await document.getElementById("nav-bar").overflowable.show();
for (let link of links) {
EventUtils.sendMouseEvent({ type: "click", button: 2 }, link, window);
// the panel should still be open
- ok(isPanelUIOpen(), "panel remains open after right-click");
+ ok(isOverflowOpen(), "panel remains open after right-click");
is(gBrowser.tabs.length, 1, "no new tab was opened");
}
- await panelUIHide();
+ await hideOverflow();
Services.prefs.clearUserPref("identity.mobilepromo.android");
Services.prefs.clearUserPref("identity.mobilepromo.ios");
});
// Test the "Sync Now" button
add_task(async function() {
// configure our broadcasters so we are in the right state.
document.getElementById("sync-reauth-state").hidden = true;
document.getElementById("sync-setup-state").hidden = true;
document.getElementById("sync-syncnow-state").hidden = false;
- await PanelUI.show();
+ await document.getElementById("nav-bar").overflowable.show();
let tabsUpdatedPromise = promiseObserverNotified("synced-tabs-menu:test:tabs-updated");
- document.getElementById("sync-button").click();
- await tabsUpdatedPromise;
let syncPanel = document.getElementById("PanelUI-remotetabs");
+ let viewShownPromise = BrowserTestUtils.waitForEvent(syncPanel, "ViewShown");
+ let syncButton = document.getElementById("sync-button");
+ syncButton.click();
+ await Promise.all([tabsUpdatedPromise, viewShownPromise]);
ok(syncPanel.getAttribute("current"), "Sync Panel is in view");
let subpanel = document.getElementById("PanelUI-remotetabs-main")
ok(!subpanel.hidden, "main pane is visible");
let deck = document.getElementById("PanelUI-remotetabs-deck");
// The widget is still fetching tabs, as we've neutered everything that
// provides them
@@ -339,17 +342,17 @@ add_task(async function() {
// There is a single node saying there's no tabs for the client.
node = node.nextSibling;
is(node.nodeName, "label", "node is a label");
is(node.getAttribute("itemtype"), "", "node is neither a tab nor a client");
node = node.nextSibling;
is(node, null, "no more entries");
- await panelUIHide();
+ await hideOverflow();
});
// Test the pagination capabilities (Show More/All tabs)
add_task(async function() {
mockedInternal.getTabClients = () => {
return Promise.resolve([
{
id: "guid_desktop",
@@ -370,23 +373,25 @@ add_task(async function() {
]);
};
// configure our broadcasters so we are in the right state.
document.getElementById("sync-reauth-state").hidden = true;
document.getElementById("sync-setup-state").hidden = true;
document.getElementById("sync-syncnow-state").hidden = false;
- await PanelUI.show();
+ await document.getElementById("nav-bar").overflowable.show();
let tabsUpdatedPromise = promiseObserverNotified("synced-tabs-menu:test:tabs-updated");
- document.getElementById("sync-button").click();
- await tabsUpdatedPromise;
+ let syncPanel = document.getElementById("PanelUI-remotetabs");
+ let viewShownPromise = BrowserTestUtils.waitForEvent(syncPanel, "ViewShown");
+ let syncButton = document.getElementById("sync-button");
+ syncButton.click();
+ await Promise.all([tabsUpdatedPromise, viewShownPromise]);
// Check pre-conditions
- let syncPanel = document.getElementById("PanelUI-remotetabs");
ok(syncPanel.getAttribute("current"), "Sync Panel is in view");
let subpanel = document.getElementById("PanelUI-remotetabs-main")
ok(!subpanel.hidden, "main pane is visible");
let deck = document.getElementById("PanelUI-remotetabs-deck");
is(deck.selectedIndex, DECKINDEX_TABS, "we should be showing tabs");
function checkTabsPage(tabsShownCount, showMoreLabel) {
let tabList = document.getElementById("PanelUI-remotetabs-tabslist");
@@ -423,10 +428,10 @@ add_task(async function() {
showMoreButton = checkTabsPage(50, "Show More");
await clickShowMoreButton();
showMoreButton = checkTabsPage(72, "Show All");
await clickShowMoreButton();
checkTabsPage(77, null);
- await panelUIHide();
+ await hideOverflow();
});