Bug 1339952 Sort order of permission prompts
MozReview-Commit-ID: 6ngylPGJ5EE
--- a/browser/base/content/test/webextensions/head.js
+++ b/browser/base/content/test/webextensions/head.js
@@ -114,16 +114,47 @@ function is_visible(element) {
// Hiding a parent element will hide all its children
if (element.parentNode != element.ownerDocument)
return is_visible(element.parentNode);
return true;
}
/**
+ * Check the contents of an individual permission string.
+ * This function is fairly specific to the use here and probably not
+ * suitable for re-use elsewhere...
+ *
+ * @param {string} string
+ * The string value to check (i.e., pulled from the DOM)
+ * @param {string} key
+ * The key in browser.properties for the localized string to
+ * compare with.
+ * @param {string|null} param
+ * Optional string to substitute for %S in the localized string.
+ * @param {string} msg
+ * The message to be emitted as part of the actual test.
+ */
+function checkPermissionString(string, key, param, msg) {
+ let localizedString = param ?
+ gBrowserBundle.formatStringFromName(key, [param], 1) :
+ gBrowserBundle.GetStringFromName(key);
+
+ // If this is a parameterized string and the parameter isn't given,
+ // just do a simple comparison of the text before and after the %S
+ if (localizedString.includes("%S")) {
+ let i = localizedString.indexOf("%S");
+ ok(string.startsWith(localizedString.slice(0, i)), msg);
+ ok(string.endsWith(localizedString.slice(i + 2)), msg);
+ } else {
+ is(string, localizedString, msg);
+ }
+}
+
+/**
* Test that install-time permission prompts work for a given
* installation method.
*
* @param {Function} installFn
* Callable that takes the name of an xpi file to install and
* starts to install it. Should return a Promise that resolves
* when the install is finished or rejects if the install is canceled.
*
@@ -191,17 +222,34 @@ async function testInstallMethod(install
// The icon should come from the extension, don't bother with the precise
// path, just make sure we've got a jar url pointing to the right path
// inside the jar.
ok(icon.startsWith("jar:file://"), "Icon is a jar url");
ok(icon.endsWith("/icon.png"), "Icon is icon.png inside a jar");
is(header.getAttribute("hidden"), "", "Permission list header is visible");
is(ul.childElementCount, 5, "Permissions list has 5 entries");
- // Real checking of the contents here is deferred until bug 1316996 lands
+
+ checkPermissionString(ul.children[0].textContent,
+ "webextPerms.hostDescription.wildcard",
+ "wildcard.domain",
+ "First permission is domain permission");
+ checkPermissionString(ul.children[1].textContent,
+ "webextPerms.hostDescription.oneSite",
+ "singlehost.domain",
+ "Second permission is single host permission");
+ checkPermissionString(ul.children[2].textContent,
+ "webextPerms.description.nativeMessaging", null,
+ "Third permission is nativeMessaging");
+ checkPermissionString(ul.children[3].textContent,
+ "webextPerms.description.tabs", null,
+ "Fourth permission is tabs");
+ checkPermissionString(ul.children[4].textContent,
+ "webextPerms.description.history", null,
+ "Fifth permission is history");
} else if (filename == NO_PERMS_XPI) {
// This extension has no icon, it should have the default
ok(isDefaultIcon(icon), "Icon is the default extension icon");
is(header.getAttribute("hidden"), "true", "Permission list header is hidden");
is(ul.childElementCount, 0, "Permissions list has 0 entries");
}
--- a/browser/modules/ExtensionsUI.jsm
+++ b/browser/modules/ExtensionsUI.jsm
@@ -215,33 +215,17 @@ this.ExtensionsUI = {
result.header = "";
result.text = bundle.formatStringFromName("webextPerms.updateText", [addonName], 1);
result.acceptText = bundle.GetStringFromName("webextPerms.updateAccept.label");
result.acceptKey = bundle.GetStringFromName("webextPerms.updateAccept.accessKey");
}
let perms = info.permissions || {hosts: [], permissions: []};
- result.msgs = [];
- for (let permission of perms.permissions) {
- let key = `webextPerms.description.${permission}`;
- if (permission == "nativeMessaging") {
- let brandBundle = Services.strings.createBundle(BRAND_PROPERTIES);
- let appName = brandBundle.GetStringFromName("brandShortName");
- result.msgs.push(bundle.formatStringFromName(key, [appName], 1));
- } else {
- try {
- result.msgs.push(bundle.GetStringFromName(key));
- } catch (err) {
- // We deliberately do not include all permissions in the prompt.
- // So if we don't find one then just skip it.
- }
- }
- }
-
+ // First classify our host permissions
let allUrls = false, wildcards = [], sites = [];
for (let permission of perms.hosts) {
if (permission == "<all_urls>") {
allUrls = true;
break;
}
let match = /^[htps*]+:\/\/([^/]+)\//.exec(permission);
if (!match) {
@@ -251,16 +235,20 @@ this.ExtensionsUI = {
allUrls = true;
} else if (match[1].startsWith("*.")) {
wildcards.push(match[1].slice(2));
} else {
sites.push(match[1]);
}
}
+ // Format the host permissions. If we have a wildcard for all urls,
+ // a single string will suffice. Otherwise, show domain wildcards
+ // first, then individual host permissions.
+ result.msgs = [];
if (allUrls) {
result.msgs.push(bundle.GetStringFromName("webextPerms.hostDescription.allUrls"));
} else {
// Formats a list of host permissions. If we have 4 or fewer, display
// them all, otherwise display the first 3 followed by an item that
// says "...plus N others"
function format(list, itemKey, moreKey) {
function formatItems(items) {
@@ -278,16 +266,40 @@ this.ExtensionsUI = {
}
format(wildcards, "webextPerms.hostDescription.wildcard",
"webextPerms.hostDescription.tooManyWildcards");
format(sites, "webextPerms.hostDescription.oneSite",
"webextPerms.hostDescription.tooManySites");
}
+ let permissionKey = perm => `webextPerms.description.${perm}`;
+
+ // Next, show the native messaging permission if it is present.
+ const NATIVE_MSG_PERM = "nativeMessaging";
+ if (perms.permissions.includes(NATIVE_MSG_PERM)) {
+ let brandBundle = Services.strings.createBundle(BRAND_PROPERTIES);
+ let appName = brandBundle.GetStringFromName("brandShortName");
+ result.msgs.push(bundle.formatStringFromName(permissionKey(NATIVE_MSG_PERM), [appName], 1));
+ }
+
+ // Finally, show remaining permissions, in any order.
+ for (let permission of perms.permissions) {
+ // Handled above
+ if (permission == "nativeMessaging") {
+ continue;
+ }
+ try {
+ result.msgs.push(bundle.GetStringFromName(permissionKey(permission)));
+ } catch (err) {
+ // We deliberately do not include all permissions in the prompt.
+ // So if we don't find one then just skip it.
+ }
+ }
+
return result;
},
showPermissionsPrompt(browser, strings, icon) {
function eventCallback(topic) {
if (topic == "showing") {
let doc = this.browser.ownerDocument;
doc.getElementById("addon-webext-perm-header").innerHTML = strings.header;