Bug 1337457 - Handle missing commands[*].suggested_key
Defaulting to "" instead of null because Chrome too defaults to "".
MozReview-Commit-ID: 7pJYCzVR4f6
--- a/browser/components/extensions/ext-commands.js
+++ b/browser/components/extensions/ext-commands.js
@@ -69,40 +69,41 @@ CommandList.prototype = {
* @param {Object} manifest The manifest JSON object.
* @returns {Map<string, object>}
*/
loadCommandsFromManifest(manifest) {
let commands = new Map();
// For Windows, chrome.runtime expects 'win' while chrome.commands
// expects 'windows'. We can special case this for now.
let os = PlatformInfo.os == "win" ? "windows" : PlatformInfo.os;
- for (let name of Object.keys(manifest.commands)) {
- let command = manifest.commands[name];
- let shortcut = command.suggested_key[os] || command.suggested_key.default;
- if (shortcut) {
- commands.set(name, {
- description: command.description,
- shortcut: shortcut.replace(/\s+/g, ""),
- });
- }
+ for (let [name, command] of Object.entries(manifest.commands)) {
+ let suggested_key = command.suggested_key || {};
+ let shortcut = suggested_key[os] || suggested_key.default;
+ shortcut = shortcut ? shortcut.replace(/\s+/g, "") : null,
+ commands.set(name, {
+ description: command.description,
+ shortcut,
+ });
}
return commands;
},
/**
* Registers the commands to a document.
* @param {ChromeWindow} window The XUL window to insert the Keyset.
*/
registerKeysToDocument(window) {
let doc = window.document;
let keyset = doc.createElementNS(XUL_NS, "keyset");
keyset.id = `ext-keyset-id-${this.id}`;
this.commands.forEach((command, name) => {
- let keyElement = this.buildKey(doc, name, command.shortcut);
- keyset.appendChild(keyElement);
+ if (command.shortcut) {
+ let keyElement = this.buildKey(doc, name, command.shortcut);
+ keyset.appendChild(keyElement);
+ }
});
doc.documentElement.appendChild(keyset);
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.
--- a/browser/components/extensions/test/browser/browser_ext_commands_getAll.js
+++ b/browser/components/extensions/test/browser/browser_ext_commands_getAll.js
@@ -21,24 +21,29 @@ add_task(function* () {
"with-platform-info": {
"suggested_key": {
"mac": "Ctrl+Shift+M",
"linux": "Ctrl+Shift+L",
"windows": "Ctrl+Shift+W",
"android": "Ctrl+Shift+A",
},
},
+ "without-suggested-key": {
+ "description": "has no suggested_key",
+ },
+ "without-suggested-key-nor-description": {
+ },
},
},
background: function() {
browser.test.onMessage.addListener((message, additionalScope) => {
browser.commands.getAll((commands) => {
let errorMessage = "getAll should return an array of commands";
- browser.test.assertEq(commands.length, 3, errorMessage);
+ browser.test.assertEq(commands.length, 5, errorMessage);
let command = commands.find(c => c.name == "with-desciption");
errorMessage = "The description should match what is provided in the manifest";
browser.test.assertEq("should have a description", command.description, errorMessage);
errorMessage = "The shortcut should match the default shortcut provided in the manifest";
browser.test.assertEq("Ctrl+Shift+Y", command.shortcut, errorMessage);
@@ -59,16 +64,28 @@ add_task(function* () {
};
command = commands.find(c => c.name == "with-platform-info");
let platformKey = platformKeys[additionalScope.platform];
let shortcut = `Ctrl+Shift+${platformKey}`;
errorMessage = `The shortcut should match the one provided in the manifest for OS='${additionalScope.platform}'`;
browser.test.assertEq(shortcut, command.shortcut, errorMessage);
+ command = commands.find(c => c.name == "without-suggested-key");
+
+ browser.test.assertEq("has no suggested_key", command.description, "The description should match what is provided in the manifest");
+
+ browser.test.assertEq(null, command.shortcut, "The shortcut should be empty if not provided");
+
+ command = commands.find(c => c.name == "without-suggested-key-nor-description");
+
+ browser.test.assertEq(null, command.description, "The description should be empty when it is not provided");
+
+ browser.test.assertEq(null, command.shortcut, "The shortcut should be empty if not provided");
+
browser.test.notifyPass("commands");
});
});
browser.test.sendMessage("ready");
},
});
yield extension.startup();