Bug 1421811 - Part 3: Update shortcut in sidebar on update r?mixedpuppy,gijs
MozReview-Commit-ID: 4y02mCqwacg
--- a/browser/base/content/browser-sidebar.js
+++ b/browser/base/content/browser-sidebar.js
@@ -114,29 +114,43 @@ var SidebarUI = {
gNavigatorBundle.getString("sidebar.moveToRight");
this._reversePositionButton.setAttribute("label", label);
this._switcherPanel.hidden = false;
this._switcherPanel.openPopup(this._icon);
this._switcherTarget.classList.add("active");
},
+ updateShortcut({button, key}) {
+ // If the shortcuts haven't been rendered yet then it will be set correctly
+ // on the first render so there's nothing to do now.
+ if (!this._addedShortcuts) {
+ return;
+ }
+ if (key) {
+ let keyId = key.getAttribute("id");
+ button = this._switcherPanel.querySelector(`[key="${keyId}"]`);
+ } else if (button) {
+ let keyId = button.getAttribute("key");
+ key = document.getElementById(keyId);
+ }
+ if (!button || !key) {
+ return;
+ }
+ button.setAttribute("shortcut", ShortcutUtils.prettifyShortcut(key));
+ },
+
_addedShortcuts: false,
_ensureShortcutsShown() {
if (this._addedShortcuts) {
return;
}
this._addedShortcuts = true;
for (let button of this._switcherPanel.querySelectorAll("toolbarbutton[key]")) {
- let keyId = button.getAttribute("key");
- let key = document.getElementById(keyId);
- if (!key) {
- continue;
- }
- button.setAttribute("shortcut", ShortcutUtils.prettifyShortcut(key));
+ this.updateShortcut({button});
}
},
/**
* Change the pref that will trigger a call to setPosition
*/
reversePosition() {
Services.prefs.setBoolPref(this.POSITION_START_PREF, !this._positionStart);
--- a/browser/components/extensions/ext-commands.js
+++ b/browser/components/extensions/ext-commands.js
@@ -8,16 +8,20 @@
ChromeUtils.defineModuleGetter(this, "ExtensionParent",
"resource://gre/modules/ExtensionParent.jsm");
ChromeUtils.defineModuleGetter(this, "ExtensionSettingsStore",
"resource://gre/modules/ExtensionSettingsStore.jsm");
var XUL_NS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
+const EXECUTE_PAGE_ACTION = "_execute_page_action";
+const EXECUTE_BROWSER_ACTION = "_execute_browser_action";
+const EXECUTE_SIDEBAR_ACTION = "_execute_sidebar_action";
+
function normalizeShortcut(shortcut) {
return shortcut ? shortcut.replace(/\s+/g, "") : null;
}
this.commands = class extends ExtensionAPI {
static async onUninstall(extensionId) {
// Cleanup the updated commands. In some cases the extension is installed
// and uninstalled so quickly that `this.commands` hasn't loaded yet. To
@@ -149,23 +153,30 @@ this.commands = class extends ExtensionA
*/
registerKeysToDocument(window, commands) {
let doc = window.document;
let keyset = doc.createElementNS(XUL_NS, "keyset");
keyset.id = `ext-keyset-id-${this.id}`;
if (this.keysetsMap.has(window)) {
this.keysetsMap.get(window).remove();
}
+ let sidebarKey;
commands.forEach((command, name) => {
if (command.shortcut) {
let keyElement = this.buildKey(doc, name, command.shortcut);
keyset.appendChild(keyElement);
+ if (name == EXECUTE_SIDEBAR_ACTION) {
+ sidebarKey = keyElement;
+ }
}
});
doc.documentElement.appendChild(keyset);
+ if (sidebarKey) {
+ window.SidebarUI.updateShortcut({key: sidebarKey});
+ }
this.keysetsMap.set(window, keyset);
}
/**
* Builds a XUL Key element and attaches an onCommand listener which
* emits a command event with the provided name when fired.
*
* @param {Document} doc The XUL document.
@@ -182,21 +193,21 @@ this.commands = class extends ExtensionA
// and it is currently ignored when set to the empty string.
keyElement.setAttribute("oncommand", "//");
/* eslint-disable mozilla/balanced-listeners */
// We remove all references to the key elements when the extension is shutdown,
// therefore the listeners for these elements will be garbage collected.
keyElement.addEventListener("command", (event) => {
let action;
- if (name == "_execute_page_action") {
+ if (name == EXECUTE_PAGE_ACTION) {
action = pageActionFor(this.extension);
- } else if (name == "_execute_browser_action") {
+ } else if (name == EXECUTE_BROWSER_ACTION) {
action = browserActionFor(this.extension);
- } else if (name == "_execute_sidebar_action") {
+ } else if (name == EXECUTE_SIDEBAR_ACTION) {
action = sidebarActionFor(this.extension);
} else {
this.extension.tabManager
.addActiveTabPermission();
this.emit("command", name);
return;
}
if (action) {
@@ -224,17 +235,17 @@ this.commands = class extends ExtensionA
let parts = shortcut.split("+");
// The key is always the last element.
let chromeKey = parts.pop();
// The modifiers are the remaining elements.
keyElement.setAttribute("modifiers", this.getModifiersAttribute(parts));
- if (name == "_execute_sidebar_action") {
+ if (name == EXECUTE_SIDEBAR_ACTION) {
let id = `ext-key-id-${this.id}-sidebar-action`;
keyElement.setAttribute("id", id);
}
if (/^[A-Z]$/.test(chromeKey)) {
// We use the key attribute for all single digits and characters.
keyElement.setAttribute("key", chromeKey);
} else {
--- a/browser/components/extensions/ext-sidebarAction.js
+++ b/browser/components/extensions/ext-sidebarAction.js
@@ -146,17 +146,17 @@ this.sidebarAction = class extends Exten
if (this.browserStyle) {
url += "&browser-style=1";
}
return url;
}
createMenuItem(window, details) {
- let {document} = window;
+ let {document, SidebarUI} = window;
// Use of the broadcaster allows browser-sidebar.js to properly manage the
// checkmarks in the menus.
let broadcaster = document.createElementNS(XUL_NS, "broadcaster");
broadcaster.setAttribute("id", this.id);
broadcaster.setAttribute("autoCheck", "false");
broadcaster.setAttribute("type", "checkbox");
broadcaster.setAttribute("group", "sidebar");
@@ -185,16 +185,17 @@ this.sidebarAction = class extends Exten
toolbarbutton.setAttribute("observes", this.id);
toolbarbutton.setAttribute("class", "subviewbutton subviewbutton-iconic webextension-menuitem");
this.setMenuIcon(toolbarbutton, details);
document.getElementById("mainBroadcasterSet").appendChild(broadcaster);
document.getElementById("viewSidebarMenu").appendChild(menuitem);
let separator = document.getElementById("sidebar-extensions-separator");
separator.parentNode.insertBefore(toolbarbutton, separator);
+ SidebarUI.updateShortcut({button: toolbarbutton});
return menuitem;
}
setMenuIcon(menuitem, details) {
let getIcon = size => IconDetails.escapeUrl(
IconDetails.getPreferredIcon(details.icon, this.extension, size).icon);
--- a/browser/components/extensions/test/browser/browser_ext_commands_update.js
+++ b/browser/components/extensions/test/browser/browser_ext_commands_update.js
@@ -231,8 +231,77 @@ add_task(async function test_update_defi
},
});
await updatedExtension.startup();
await TestUtils.waitForCondition(() => extensionKeyset(extension.id));
// Shortcut is unchanged since it was previously updated.
checkKey(extension.id, "P", "alt shift");
});
+
+add_task(async function updateSidebarCommand() {
+ let extension = ExtensionTestUtils.loadExtension({
+ useAddonManager: "temporary",
+ manifest: {
+ commands: {
+ _execute_sidebar_action: {
+ suggested_key: {
+ default: "Ctrl+Shift+E",
+ },
+ },
+ },
+ sidebar_action: {
+ default_panel: "sidebar.html",
+ },
+ },
+ background() {
+ browser.test.onMessage.addListener(async (msg, data) => {
+ if (msg == "updateShortcut") {
+ await browser.commands.update(data);
+ return browser.test.sendMessage("done");
+ }
+ throw new Error("Unknown message");
+ });
+ },
+ files: {
+ "sidebar.html": `
+ <!DOCTYPE html>
+ <html>
+ <head><meta charset="utf-8"/>
+ <script src="sidebar.js"></script>
+ </head>
+ <body>
+ A Test Sidebar
+ </body></html>
+ `,
+
+ "sidebar.js": function() {
+ window.onload = () => {
+ browser.test.sendMessage("sidebar");
+ };
+ },
+ },
+ });
+ await extension.startup();
+ await extension.awaitMessage("sidebar");
+
+ // Show and hide the switcher panel to generate the initial shortcuts.
+ let switcherShown = promisePopupShown(SidebarUI._switcherPanel);
+ SidebarUI.showSwitcherPanel();
+ await switcherShown;
+ let switcherHidden = promisePopupHidden(SidebarUI._switcherPanel);
+ SidebarUI.hideSwitcherPanel();
+ await switcherHidden;
+
+ let buttonId = `button_${makeWidgetId(extension.id)}-sidebar-action`;
+ let button = document.getElementById(buttonId);
+ let shortcut = button.getAttribute("shortcut");
+ ok(shortcut.endsWith("E"), "The button has the shortcut set");
+
+ extension.sendMessage(
+ "updateShortcut", {name: "_execute_sidebar_action", shortcut: "Ctrl+Shift+M"});
+ await extension.awaitMessage("done");
+
+ shortcut = button.getAttribute("shortcut");
+ ok(shortcut.endsWith("M"), "The button shortcut has been updated");
+
+ await extension.unload();
+});
--- a/browser/components/extensions/test/browser/browser_ext_sidebarAction.js
+++ b/browser/components/extensions/test/browser/browser_ext_sidebarAction.js
@@ -1,23 +1,16 @@
/* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
/* vim: set sts=2 sw=2 et tw=80: */
"use strict";
requestLongerTimeout(2);
let extData = {
manifest: {
- commands: {
- _execute_sidebar_action: {
- suggested_key: {
- default: "Ctrl+Shift+I",
- },
- },
- },
sidebar_action: {
default_panel: "sidebar.html",
},
},
useAddonManager: "temporary",
files: {
"sidebar.html": `
@@ -52,84 +45,89 @@ let extData = {
let isOpen = await browser.sidebarAction.isOpen(arg);
browser.test.assertEq(result, isOpen, "expected value from isOpen");
}
browser.test.sendMessage("done");
});
},
};
+function getExtData(manifestUpdates = {}) {
+ return {
+ ...extData,
+ manifest: {
+ ...extData.manifest,
+ ...manifestUpdates,
+ },
+ };
+}
+
+
async function sendMessage(ext, msg, data = undefined) {
ext.sendMessage({msg, data});
await ext.awaitMessage("done");
}
add_task(async function sidebar_initial_install() {
ok(document.getElementById("sidebar-box").hidden, "sidebar box is not visible");
- let extension = ExtensionTestUtils.loadExtension(extData);
+ let extension = ExtensionTestUtils.loadExtension(getExtData());
await extension.startup();
// Test sidebar is opened on install
await extension.awaitMessage("sidebar");
ok(!document.getElementById("sidebar-box").hidden, "sidebar box is visible");
await extension.unload();
// Test that the sidebar was closed on unload.
ok(document.getElementById("sidebar-box").hidden, "sidebar box is not visible");
});
add_task(async function sidebar_two_sidebar_addons() {
- let extension2 = ExtensionTestUtils.loadExtension(extData);
+ let extension2 = ExtensionTestUtils.loadExtension(getExtData());
await extension2.startup();
// Test sidebar is opened on install
await extension2.awaitMessage("sidebar");
ok(!document.getElementById("sidebar-box").hidden, "sidebar box is visible");
// Test second sidebar install opens new sidebar
- let extension3 = ExtensionTestUtils.loadExtension(extData);
+ let extension3 = ExtensionTestUtils.loadExtension(getExtData());
await extension3.startup();
// Test sidebar is opened on install
await extension3.awaitMessage("sidebar");
ok(!document.getElementById("sidebar-box").hidden, "sidebar box is visible");
await extension3.unload();
// We just close the sidebar on uninstall of the current sidebar.
ok(document.getElementById("sidebar-box").hidden, "sidebar box is not visible");
await extension2.unload();
});
add_task(async function sidebar_empty_panel() {
- let extension = ExtensionTestUtils.loadExtension(extData);
+ let extension = ExtensionTestUtils.loadExtension(getExtData());
await extension.startup();
// Test sidebar is opened on install
await extension.awaitMessage("sidebar");
ok(!document.getElementById("sidebar-box").hidden, "sidebar box is visible in first window");
await sendMessage(extension, "set-panel");
await extension.unload();
});
add_task(async function sidebar_isOpen() {
info("Load extension1");
- let extension1 = ExtensionTestUtils.loadExtension(extData);
+ let extension1 = ExtensionTestUtils.loadExtension(getExtData());
await extension1.startup();
info("Test extension1's sidebar is opened on install");
await extension1.awaitMessage("sidebar");
await sendMessage(extension1, "isOpen", {result: true});
let sidebar1ID = SidebarUI.currentID;
- // Test that the key is set for the extension.
- let button = document.getElementById(`button_${makeWidgetId(extension1.id)}-sidebar-action`);
- ok(button.hasAttribute("key"), "The menu item has a key specified");
- let key = document.getElementById(button.getAttribute("key"));
- ok(key, "The key attribute finds the related key element");
-
info("Load extension2");
- let extension2 = ExtensionTestUtils.loadExtension(extData);
+ let extension2 = ExtensionTestUtils.loadExtension(getExtData());
await extension2.startup();
info("Test extension2's sidebar is opened on install");
await extension2.awaitMessage("sidebar");
await sendMessage(extension1, "isOpen", {result: false});
await sendMessage(extension2, "isOpen", {result: true});
info("Switch back to extension1's sidebar");
@@ -163,8 +161,66 @@ add_task(async function sidebar_isOpen()
info("Close the sidebar in the original window");
SidebarUI.hide();
await sendMessage(extension1, "isOpen", {result: false});
await sendMessage(extension2, "isOpen", {result: false});
await extension1.unload();
await extension2.unload();
});
+
+add_task(async function testShortcuts() {
+ function verifyShortcut(id, commandKey) {
+ // We're just testing the command key since the modifiers have different
+ // icons on different platforms.
+ let button = document.getElementById(`button_${makeWidgetId(id)}-sidebar-action`);
+ ok(button.hasAttribute("key"), "The menu item has a key specified");
+ let key = document.getElementById(button.getAttribute("key"));
+ ok(key, "The key attribute finds the related key element");
+ ok(button.getAttribute("shortcut").endsWith(commandKey),
+ "The shortcut has the right key");
+ }
+
+ let extension1 = ExtensionTestUtils.loadExtension(
+ getExtData({
+ commands: {
+ _execute_sidebar_action: {
+ suggested_key: {
+ default: "Ctrl+Shift+I",
+ },
+ },
+ },
+ }));
+ let extension2 = ExtensionTestUtils.loadExtension(
+ getExtData({
+ commands: {
+ _execute_sidebar_action: {
+ suggested_key: {
+ default: "Ctrl+Shift+E",
+ },
+ },
+ },
+ }));
+
+ await extension1.startup();
+ await extension1.awaitMessage("sidebar");
+
+ // Open and close the switcher panel to trigger shortcut content rendering.
+ let switcherPanelShown = promisePopupShown(SidebarUI._switcherPanel);
+ SidebarUI.showSwitcherPanel();
+ await switcherPanelShown;
+ let switcherPanelHidden = promisePopupHidden(SidebarUI._switcherPanel);
+ SidebarUI.hideSwitcherPanel();
+ await switcherPanelHidden;
+
+ // Test that the key is set for the extension after the shortcuts are rendered.
+ verifyShortcut(extension1.id, "I");
+
+ await extension2.startup();
+ await extension2.awaitMessage("sidebar");
+
+ // Once the switcher panel has been opened new shortcuts should be added
+ // automatically.
+ verifyShortcut(extension2.id, "E");
+
+ await extension1.unload();
+ await extension2.unload();
+});