Bug 1217129: Part 5 - [webext] Use CustomizableUI views for BrowserAction popups. r=gijs ui-r=bwinton

This version addresses some popup sizing bugs, and also a few other issues I
ran into when debugging Blake's problems:

 * The standalone popup needs a max width of 800px for Chrome compatibility,
   which is wider than our default max width.

 * I added a flex attribute to our browser so that it fills the entire space
   of the slide-in panel. This is only necessary for browsers with content
   that is shorter than the height of the panel when it gets its desired
   width, but becomes longer when it doesn't, so it didn't show up in my
   initial tests.

 * I also added an extra pixel to the width calculations, since I noticed that
   a lot of single lines of text were unexpectedly wrapping without it. I'll
   look into this more in a follow-up bug.

I also added some comments, and renamed a couple of variables, where things
seemed unclear.

The test changes are mostly just updates to older browser action tests to use
newer helpers, rather than ad-hoc events, to open/close/click the widgets. A
few tests also needed updates to explicitly close the panel when they were
done with it.
This commit is contained in:
Kris Maglione 2016-01-15 15:14:25 -08:00
parent 75cb17bb0e
commit b1feddc3d1
13 changed files with 321 additions and 167 deletions

View File

@ -2,13 +2,14 @@
"extends": "../../../toolkit/components/extensions/.eslintrc",
"globals": {
"AllWindowEvents": true,
"currentWindow": true,
"EventEmitter": true,
"IconDetails": true,
"openPanel": true,
"makeWidgetId": true,
"PanelPopup": true,
"TabContext": true,
"AllWindowEvents": true,
"ViewPopup": true,
"WindowEventManager": true,
"WindowListManager": true,
"WindowManager": true,

View File

@ -13,6 +13,8 @@ var {
runSafe,
} = ExtensionUtils;
const XUL_NS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
// WeakMap[Extension -> BrowserAction]
var browserActionMap = new WeakMap();
@ -24,7 +26,10 @@ function browserActionOf(extension) {
// as the associated popup.
function BrowserAction(options, extension) {
this.extension = extension;
this.id = makeWidgetId(extension.id) + "-browser-action";
let widgetId = makeWidgetId(extension.id);
this.id = `${widgetId}-browser-action`;
this.viewId = `PanelUI-webext-${widgetId}-browser-action-view`;
this.widget = null;
this.tabManager = TabManager.for(extension);
@ -55,31 +60,60 @@ BrowserAction.prototype = {
build() {
let widget = CustomizableUI.createWidget({
id: this.id,
type: "custom",
viewId: this.viewId,
type: "view",
removable: true,
label: this.defaults.title || this.extension.name,
tooltiptext: this.defaults.title || "",
defaultArea: CustomizableUI.AREA_NAVBAR,
onBuild: document => {
let node = document.createElement("toolbarbutton");
node.id = this.id;
node.setAttribute("class", "toolbarbutton-1 chromeclass-toolbar-additional badged-button");
onBeforeCreated: document => {
let view = document.createElementNS(XUL_NS, "panelview");
view.id = this.viewId;
view.setAttribute("flex", "1");
document.getElementById("PanelUI-multiView").appendChild(view);
},
onDestroyed: document => {
let view = document.getElementById(this.viewId);
if (view) {
view.remove();
}
},
onCreated: node => {
node.classList.add("badged-button");
node.setAttribute("constrain-size", "true");
this.updateButton(node, this.defaults);
},
onViewShowing: event => {
let document = event.target.ownerDocument;
let tabbrowser = document.defaultView.gBrowser;
node.addEventListener("command", event => { // eslint-disable-line mozilla/balanced-listeners
let tab = tabbrowser.selectedTab;
let popup = this.getProperty(tab, "popup");
this.tabManager.addActiveTabPermission(tab);
if (popup) {
this.togglePopup(node, popup);
} else {
this.emit("click");
}
});
let tab = tabbrowser.selectedTab;
let popupURL = this.getProperty(tab, "popup");
this.tabManager.addActiveTabPermission(tab);
return node;
// If the widget has a popup URL defined, we open a popup, but do not
// dispatch a click event to the extension.
// If it has no popup URL defined, we dispatch a click event, but do not
// open a popup.
if (popupURL) {
try {
new ViewPopup(this.extension, event.target, popupURL);
} catch (e) {
Cu.reportError(e);
event.preventDefault();
}
} else {
// This isn't not a hack, but it seems to provide the correct behavior
// with the fewest complications.
event.preventDefault();
this.emit("click");
}
},
});
@ -89,10 +123,6 @@ BrowserAction.prototype = {
this.widget = widget;
},
togglePopup(node, popupResource) {
openPanel(node, popupResource, this.extension);
},
// Update the toolbar button |node| with the tab context data
// in |tabData|.
updateButton(node, tabData) {

View File

@ -138,12 +138,16 @@ PageAction.prototype = {
// the any click listeners in the add-on.
handleClick(window) {
let tab = window.gBrowser.selectedTab;
let popup = this.tabContext.get(tab).popup;
let popupURL = this.tabContext.get(tab).popup;
this.tabManager.addActiveTabPermission(tab);
if (popup) {
openPanel(this.getButton(window), popup, this.extension);
// If the widget has a popup URL defined, we open a popup, but do not
// dispatch a click event to the extension.
// If it has no popup URL defined, we dispatch a click event, but do not
// open a popup.
if (popupURL) {
new PanelPopup(this.extension, this.getButton(window), popupURL);
} else {
this.emit("click", tab);
}

View File

@ -2,12 +2,16 @@
/* vim: set sts=2 sw=2 et tw=80: */
"use strict";
XPCOMUtils.defineLazyModuleGetter(this, "CustomizableUI",
"resource:///modules/CustomizableUI.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "PrivateBrowsingUtils",
"resource://gre/modules/PrivateBrowsingUtils.jsm");
Cu.import("resource://gre/modules/ExtensionUtils.jsm");
Cu.import("resource://gre/modules/AddonManager.jsm");
const XUL_NS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
const INTEGER = /^[1-9]\d*$/;
var {
@ -126,103 +130,203 @@ global.makeWidgetId = id => {
return id.replace(/[^a-z0-9_-]/g, "_");
};
// Open a panel anchored to the given node, containing a browser opened
// to the given URL, owned by the given extension. If |popupURL| is not
// an absolute URL, it is resolved relative to the given extension's
// base URL.
global.openPanel = (node, popupURL, extension) => {
let document = node.ownerDocument;
class BasePopup {
constructor(extension, viewNode, popupURL) {
let popupURI = Services.io.newURI(popupURL, null, extension.baseURI);
let popupURI = Services.io.newURI(popupURL, null, extension.baseURI);
Services.scriptSecurityManager.checkLoadURIWithPrincipal(
extension.principal, popupURI,
Services.scriptSecurityManager.DISALLOW_SCRIPT);
Services.scriptSecurityManager.checkLoadURIWithPrincipal(
extension.principal, popupURI,
Services.scriptSecurityManager.DISALLOW_SCRIPT);
this.extension = extension;
this.popupURI = popupURI;
this.viewNode = viewNode;
this.window = viewNode.ownerDocument.defaultView;
let panel = document.createElement("panel");
panel.setAttribute("id", makeWidgetId(extension.id) + "-panel");
panel.setAttribute("class", "browser-extension-panel");
panel.setAttribute("type", "arrow");
panel.setAttribute("role", "group");
this.contentReady = new Promise(resolve => {
this._resolveContentReady = resolve;
});
let anchor;
if (node.localName == "toolbarbutton") {
// Toolbar buttons are a special case. The panel becomes a child of
// the button, and is anchored to the button's icon.
node.appendChild(panel);
anchor = document.getAnonymousElementByAttribute(node, "class", "toolbarbutton-icon");
} else {
// In all other cases, the panel is anchored to the target node
// itself, and is a child of a popupset node.
document.getElementById("mainPopupSet").appendChild(panel);
anchor = node;
this.viewNode.addEventListener(this.DESTROY_EVENT, this);
this.browser = null;
this.browserReady = this.createBrowser(viewNode, popupURI);
}
const XUL_NS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
let browser = document.createElementNS(XUL_NS, "browser");
browser.setAttribute("type", "content");
browser.setAttribute("disableglobalhistory", "true");
panel.appendChild(browser);
destroy() {
this.browserReady.then(() => {
this.browser.removeEventListener("load", this, true);
this.browser.removeEventListener("DOMTitleChanged", this, true);
this.browser.removeEventListener("DOMWindowClose", this, true);
let titleChangedListener = () => {
panel.setAttribute("aria-label", browser.contentTitle);
};
this.viewNode.removeEventListener(this.DESTROY_EVENT, this);
let context;
let popuphidden = () => {
panel.removeEventListener("popuphidden", popuphidden);
browser.removeEventListener("DOMTitleChanged", titleChangedListener, true);
context.unload();
panel.remove();
};
panel.addEventListener("popuphidden", popuphidden);
this.context.unload();
this.browser.remove();
let loadListener = () => {
panel.removeEventListener("load", loadListener);
context = new ExtensionPage(extension, {
type: "popup",
contentWindow: browser.contentWindow,
uri: popupURI,
docShell: browser.docShell,
this.browser = null;
this.viewNode = null;
this.context = null;
});
GlobalManager.injectInDocShell(browser.docShell, extension, context);
browser.setAttribute("src", context.uri.spec);
}
let contentLoadListener = event => {
if (event.target != browser.contentDocument) {
return;
}
browser.removeEventListener("load", contentLoadListener, true);
// Returns the name of the event fired on `viewNode` when the popup is being
// destroyed. This must be implemented by every subclass.
get DESTROY_EVENT() {
throw new Error("Not implemented");
}
let contentViewer = browser.docShell.contentViewer;
let width = {}, height = {};
try {
contentViewer.getContentSize(width, height);
[width, height] = [width.value, height.value];
} catch (e) {
// getContentSize can throw
[width, height] = [400, 400];
}
handleEvent(event) {
switch (event.type) {
case this.DESTROY_EVENT:
this.destroy();
break;
let window = document.defaultView;
width /= window.devicePixelRatio;
height /= window.devicePixelRatio;
width = Math.min(width, 800);
height = Math.min(height, 800);
case "DOMWindowClose":
if (event.target === this.browser.contentWindow) {
event.preventDefault();
this.closePopup();
}
break;
browser.setAttribute("width", width);
browser.setAttribute("height", height);
case "DOMTitleChanged":
this.viewNode.setAttribute("aria-label", this.browser.contentTitle);
break;
panel.openPopup(anchor, "bottomcenter topright", 0, 0, false, false);
};
browser.addEventListener("load", contentLoadListener, true);
case "load":
// We use a capturing listener, so we get this event earlier than any
// load listeners in the content page. Resizing after a timeout ensures
// that we calculate the size after the entire event cycle has completed
// (unless someone spins the event loop, anyway), and hopefully after
// the content has made any modifications.
//
// In the future, to match Chrome's behavior, we'll need to update this
// dynamically, probably in response to MozScrolledAreaChanged events.
this.window.setTimeout(() => this.resizeBrowser(), 0);
break;
}
}
browser.addEventListener("DOMTitleChanged", titleChangedListener, true);
};
panel.addEventListener("load", loadListener);
createBrowser(viewNode, popupURI) {
let document = viewNode.ownerDocument;
return panel;
this.browser = document.createElementNS(XUL_NS, "browser");
this.browser.setAttribute("type", "content");
this.browser.setAttribute("disableglobalhistory", "true");
// Note: When using noautohide panels, the popup manager will add width and
// height attributes to the panel, breaking our resize code, if the browser
// starts out smaller than 30px by 10px. This isn't an issue now, but it
// will be if and when we popup debugging.
// This overrides the content's preferred size when displayed in a
// fixed-size, slide-in panel.
this.browser.setAttribute("flex", "1");
viewNode.appendChild(this.browser);
return new Promise(resolve => {
// The first load event is for about:blank.
// We can't finish setting up the browser until the binding has fully
// initialized. Waiting for the first load event guarantees that it has.
let loadListener = event => {
this.browser.removeEventListener("load", loadListener, true);
resolve();
};
this.browser.addEventListener("load", loadListener, true);
}).then(() => {
let { contentWindow } = this.browser;
contentWindow.QueryInterface(Ci.nsIInterfaceRequestor)
.getInterface(Ci.nsIDOMWindowUtils)
.allowScriptsToClose();
this.context = new ExtensionPage(this.extension, {
type: "popup",
contentWindow,
uri: popupURI,
docShell: this.browser.docShell,
});
GlobalManager.injectInDocShell(this.browser.docShell, this.extension, this.context);
this.browser.setAttribute("src", this.context.uri.spec);
this.browser.addEventListener("load", this, true);
this.browser.addEventListener("DOMTitleChanged", this, true);
this.browser.addEventListener("DOMWindowClose", this, true);
});
}
// Resizes the browser to match the preferred size of the content.
resizeBrowser() {
let width, height;
try {
let w = {}, h = {};
this.browser.docShell.contentViewer.getContentSize(w, h);
width = w.value / this.window.devicePixelRatio;
height = h.value / this.window.devicePixelRatio;
// The width calculation is imperfect, and is often a fraction of a pixel
// too narrow, even after taking the ceiling, which causes lines of text
// to wrap.
width += 1;
} catch (e) {
// getContentSize can throw
[width, height] = [400, 400];
}
width = Math.ceil(Math.min(width, 800));
height = Math.ceil(Math.min(height, 600));
this.browser.style.width = `${width}px`;
this.browser.style.height = `${height}px`;
this._resolveContentReady();
}
}
global.PanelPopup = class PanelPopup extends BasePopup {
constructor(extension, imageNode, popupURL) {
let document = imageNode.ownerDocument;
let panel = document.createElement("panel");
panel.setAttribute("id", makeWidgetId(extension.id) + "-panel");
panel.setAttribute("class", "browser-extension-panel");
panel.setAttribute("type", "arrow");
panel.setAttribute("role", "group");
document.getElementById("mainPopupSet").appendChild(panel);
super(extension, panel, popupURL);
this.contentReady.then(() => {
panel.openPopup(imageNode, "bottomcenter topright", 0, 0, false, false);
});
}
get DESTROY_EVENT() {
return "popuphidden";
}
destroy() {
super.destroy();
this.viewNode.remove();
}
closePopup() {
this.viewNode.hidePopup();
}
};
global.ViewPopup = class ViewPopup extends BasePopup {
get DESTROY_EVENT() {
return "ViewHiding";
}
closePopup() {
CustomizableUI.hidePanelForNode(this.viewNode);
}
};
// Manages tab-specific context data, and dispatching tab select events

View File

@ -4,7 +4,7 @@
function promisePopupShown(popup) {
return new Promise(resolve => {
if (popup.popupOpen) {
if (popup.state == "open") {
resolve();
} else {
let onPopupShown = event => {
@ -116,19 +116,19 @@ add_task(function* testPageActionPopup() {
},
});
let panelId = makeWidgetId(extension.id) + "-panel";
let viewId = `PanelUI-webext-${makeWidgetId(extension.id)}-browser-action-view`;
extension.onMessage("send-click", () => {
clickBrowserAction(extension);
});
extension.onMessage("next-test", Task.async(function* () {
let panel = document.getElementById(panelId);
let panel = getBrowserActionPopup(extension);
if (panel) {
yield promisePopupShown(panel);
panel.hidePopup();
panel = document.getElementById(panelId);
panel = getBrowserActionPopup(extension);
is(panel, null, "panel successfully removed from document after hiding");
}
@ -140,6 +140,6 @@ add_task(function* testPageActionPopup() {
yield extension.unload();
let panel = document.getElementById(panelId);
is(panel, null, "browserAction panel removed from document");
let view = document.getElementById(viewId);
is(view, null, "browserAction view removed from document");
});

View File

@ -33,23 +33,13 @@ add_task(function* () {
yield extension.startup();
let widgetId = makeWidgetId(extension.id) + "-browser-action";
let node = CustomizableUI.getWidget(widgetId).forWindow(window).node;
// Do this a few times to make sure the pop-up is reloaded each time.
for (let i = 0; i < 3; i++) {
let evt = new CustomEvent("command", {
bubbles: true,
cancelable: true,
});
node.dispatchEvent(evt);
clickBrowserAction(extension);
yield extension.awaitMessage("popup");
let panel = node.querySelector("panel");
if (panel) {
panel.hidePopup();
}
closeBrowserAction(extension);
}
yield extension.unload();

View File

@ -110,23 +110,13 @@ add_task(function* () {
yield checkWindow("background", winId2, "win2");
function* triggerPopup(win, callback) {
let widgetId = makeWidgetId(extension.id) + "-browser-action";
let node = CustomizableUI.getWidget(widgetId).forWindow(win).node;
let evt = new CustomEvent("command", {
bubbles: true,
cancelable: true,
});
node.dispatchEvent(evt);
yield clickBrowserAction(extension, win);
yield extension.awaitMessage("popup-ready");
yield callback();
let panel = node.querySelector("panel");
if (panel) {
panel.hidePopup();
}
closeBrowserAction(extension, win);
}
// Set focus to some other window.

View File

@ -116,25 +116,21 @@ add_task(function* () {
yield checkViews("background", 2, 0);
function* triggerPopup(win, callback) {
let widgetId = makeWidgetId(extension.id) + "-browser-action";
let node = CustomizableUI.getWidget(widgetId).forWindow(win).node;
let evt = new CustomEvent("command", {
bubbles: true,
cancelable: true,
});
node.dispatchEvent(evt);
yield clickBrowserAction(extension, win);
yield extension.awaitMessage("popup-ready");
yield callback();
let panel = node.querySelector("panel");
if (panel) {
panel.hidePopup();
}
closeBrowserAction(extension, win);
}
// The popup occasionally closes prematurely if we open it immediately here.
// I'm not sure what causes it to close (it's something internal, and seems to
// be focus-related, but it's not caused by JS calling hidePopup), but even a
// short timeout seems to consistently fix it.
yield new Promise(resolve => win1.setTimeout(resolve, 10));
yield triggerPopup(win1, function*() {
yield checkViews("background", 2, 1);
yield checkViews("popup", 2, 1);

View File

@ -42,19 +42,6 @@ add_task(function* testPageActionPopup() {
},
});
let browserActionId = makeWidgetId(extension.id) + "-browser-action";
let pageActionId = makeWidgetId(extension.id) + "-page-action";
function openPopup(buttonId) {
let button = document.getElementById(buttonId);
if (buttonId == pageActionId) {
// TODO: I don't know why a proper synthesized event doesn't work here.
button.dispatchEvent(new MouseEvent("click", {}));
} else {
EventUtils.synthesizeMouseAtCenter(button, {}, window);
}
}
let promiseConsoleMessage = pattern => new Promise(resolve => {
Services.console.registerListener(function listener(msg) {
if (pattern.test(msg.message)) {
@ -72,21 +59,25 @@ add_task(function* testPageActionPopup() {
// BrowserAction:
let awaitMessage = promiseConsoleMessage(/WebExt Privilege Escalation: BrowserAction/);
SimpleTest.expectUncaughtException();
openPopup(browserActionId);
yield clickBrowserAction(extension);
let message = yield awaitMessage;
ok(message.includes("WebExt Privilege Escalation: BrowserAction: typeof(browser) = undefined"),
`No BrowserAction API injection`);
yield closeBrowserAction(extension);
// PageAction
awaitMessage = promiseConsoleMessage(/WebExt Privilege Escalation: PageAction/);
SimpleTest.expectUncaughtException();
openPopup(pageActionId);
yield clickPageAction(extension);
message = yield awaitMessage;
ok(message.includes("WebExt Privilege Escalation: PageAction: typeof(browser) = undefined"),
`No PageAction API injection: ${message}`);
yield closePageAction(extension);
SimpleTest.expectUncaughtException(false);
@ -95,12 +86,13 @@ add_task(function* testPageActionPopup() {
yield extension.awaitMessage("ok");
// Check that unprivileged documents don't get the API.
openPopup(browserActionId);
yield clickBrowserAction(extension);
yield extension.awaitMessage("from-popup-a");
yield closeBrowserAction(extension);
openPopup(pageActionId);
yield clickPageAction(extension);
yield extension.awaitMessage("from-popup-b");
yield closePageAction(extension);
yield extension.unload();
});

View File

@ -48,6 +48,11 @@ function* testHasPermission(params) {
extension.sendMessage("execute-script");
yield extension.awaitFinish("executeScript");
if (params.tearDown) {
yield params.tearDown(extension);
}
yield extension.unload();
}
@ -82,6 +87,7 @@ add_task(function* testGoodPermissions() {
return Promise.resolve();
},
setup: clickBrowserAction,
tearDown: closeBrowserAction,
});
info("Test activeTab permission with a page action click");
@ -99,6 +105,7 @@ add_task(function* testGoodPermissions() {
});
},
setup: clickPageAction,
tearDown: closePageAction,
});
info("Test activeTab permission with a browser action w/popup click");
@ -108,6 +115,7 @@ add_task(function* testGoodPermissions() {
"browser_action": { "default_popup": "_blank.html" },
},
setup: clickBrowserAction,
tearDown: closeBrowserAction,
});
info("Test activeTab permission with a page action w/popup click");
@ -125,6 +133,7 @@ add_task(function* testGoodPermissions() {
});
},
setup: clickPageAction,
tearDown: closePageAction,
});
info("Test activeTab permission with a context menu click");

View File

@ -63,6 +63,7 @@ add_task(function* () {
clickBrowserAction(extension);
yield extension.awaitMessage("popup-finished");
yield closeBrowserAction(extension);
yield extension.unload();
});

View File

@ -2,7 +2,11 @@
/* vim: set sts=2 sw=2 et tw=80: */
"use strict";
/* exported AppConstants CustomizableUI forceGC makeWidgetId focusWindow clickBrowserAction clickPageAction */
/* exported CustomizableUI makeWidgetId focusWindow forceGC
* clickBrowserAction clickPageAction
* getBrowserActionPopup getPageActionPopup
* closeBrowserAction closePageAction
*/
var {AppConstants} = Cu.import("resource://gre/modules/AppConstants.jsm");
var {CustomizableUI} = Cu.import("resource:///modules/CustomizableUI.jsm");
@ -39,6 +43,10 @@ var focusWindow = Task.async(function* focusWindow(win) {
yield promise;
});
function getBrowserActionPopup(extension, win = window) {
return win.document.getElementById("customizationui-widget-panel");
}
function clickBrowserAction(extension, win = window) {
let browserActionId = makeWidgetId(extension.id) + "-browser-action";
let elem = win.document.getElementById(browserActionId);
@ -47,6 +55,20 @@ function clickBrowserAction(extension, win = window) {
return new Promise(SimpleTest.executeSoon);
}
function closeBrowserAction(extension, win = window) {
let node = getBrowserActionPopup(extension, win);
if (node) {
node.hidePopup();
}
return Promise.resolve();
}
function getPageActionPopup(extension, win = window) {
let panelId = makeWidgetId(extension.id) + "-panel";
return win.document.getElementById(panelId);
}
function clickPageAction(extension, win = window) {
// This would normally be set automatically on navigation, and cleared
// when the user types a value into the URL bar, to show and hide page
@ -63,3 +85,13 @@ function clickPageAction(extension, win = window) {
EventUtils.synthesizeMouseAtCenter(elem, {}, win);
return new Promise(SimpleTest.executeSoon);
}
function closePageAction(extension, win = window) {
let node = getPageActionPopup(extension, win);
if (node) {
node.hidePopup();
}
return Promise.resolve();
}

View File

@ -250,6 +250,11 @@ panelmultiview[nosubviews=true] > .panel-viewcontainer > .panel-viewstack > .pan
max-width: @standaloneSubviewWidth@;
}
/* Give WebExtension stand-alone panels extra width for Chrome compatibility */
.cui-widget-panel[viewId^=PanelUI-webext-] .panel-mainview {
max-width: 800px;
}
panelview:not([mainview]) .toolbarbutton-text,
.cui-widget-panel toolbarbutton > .toolbarbutton-text {
text-align: start;