Bug 1340501 Fix sideloading notification with no promptable permissions draft
authorAndrew Swan <aswan@mozilla.com>
Thu, 23 Feb 2017 19:05:22 -0800
changeset 491116 dafcbd379ab005bc6df2981755a0ca07cc9062fd
parent 490976 e150eaff1f83e4e4a97d1e30c57d233859efe9cb
child 547461 6647fd765ce98e604865ec9cc7c5fff87ddde478
push id47316
push useraswan@mozilla.com
push dateWed, 01 Mar 2017 18:52:48 +0000
bugs1340501
milestone54.0a1
Bug 1340501 Fix sideloading notification with no promptable permissions MozReview-Commit-ID: JduSAhhYWwR
browser/base/content/test/webextensions/browser_extension_sideloading.js
browser/base/content/test/webextensions/head.js
browser/locales/en-US/chrome/browser/browser.properties
browser/modules/ExtensionsUI.jsm
--- a/browser/base/content/test/webextensions/browser_extension_sideloading.js
+++ b/browser/base/content/test/webextensions/browser_extension_sideloading.js
@@ -201,18 +201,20 @@ add_task(function* () {
   is(gBrowser.currentURI.spec, "about:addons", "Foreground tab is at about:addons");
 
   const VIEW = "addons://list/extension";
   let win = gBrowser.selectedBrowser.contentWindow;
   ok(!win.gViewController.isLoading, "about:addons view is fully loaded");
   is(win.gViewController.currentViewId, VIEW, "about:addons is at extensions list");
 
   // Check the contents of the notification, then choose "Cancel"
-  let icon = panel.getAttribute("icon");
-  is(icon, ICON_URL, "Permissions notification has the addon icon");
+  checkNotification(panel, ICON_URL, [
+    ["webextPerms.hostDescription.allUrls"],
+    ["webextPerms.description.history"],
+  ]);
 
   let disablePromise = promiseSetDisabled(mock1);
   panel.secondaryButton.click();
 
   let value = yield disablePromise;
   is(value, true, "Addon should remain disabled");
 
   let [addon1, addon2, addon3, addon4] = yield AddonManager.getAddonsByIDs([ID1, ID2, ID3, ID4]);
@@ -237,19 +239,20 @@ add_task(function* () {
 
   // Again we should be at the extentions list in about:addons
   is(gBrowser.currentURI.spec, "about:addons", "Foreground tab is at about:addons");
 
   win = gBrowser.selectedBrowser.contentWindow;
   ok(!win.gViewController.isLoading, "about:addons view is fully loaded");
   is(win.gViewController.currentViewId, VIEW, "about:addons is at extensions list");
 
-  // Check the notification contents, this time accept the install
-  icon = panel.getAttribute("icon");
-  is(icon, DEFAULT_ICON_URL, "Permissions notification has the default icon");
+  // Check the notification contents.
+  checkNotification(panel, DEFAULT_ICON_URL, []);
+
+  // This time accept the install.
   disablePromise = promiseSetDisabled(mock2);
   panel.button.click();
 
   value = yield disablePromise;
   is(value, false, "Addon should be set to enabled");
 
   [addon1, addon2, addon3, addon4] = yield AddonManager.getAddonsByIDs([ID1, ID2, ID3, ID4]);
   is(addon1.userDisabled, true, "Addon 1 should still be disabled");
@@ -280,16 +283,17 @@ add_task(function* () {
   ok(is_visible(item._enableBtn), "Enable button is visible for sideloaded extension");
   ok(is_hidden(item._disableBtn), "Disable button is not visible for sideloaded extension");
 
   // When clicking enable we should see the permissions notification
   popupPromise = promisePopupNotificationShown("addon-webext-permissions");
   BrowserTestUtils.synthesizeMouseAtCenter(item._enableBtn, {},
                                            gBrowser.selectedBrowser);
   panel = yield popupPromise;
+  checkNotification(panel, DEFAULT_ICON_URL, [["webextPerms.hostDescription.allUrls"]]);
 
   // Accept the permissions
   disablePromise = promiseSetDisabled(mock3);
   panel.button.click();
   value = yield disablePromise;
   is(value, false, "userDisabled should be set on addon 3");
 
   addon3 = yield AddonManager.getAddonByID(ID3);
@@ -307,16 +311,17 @@ add_task(function* () {
   win = yield BrowserOpenAddonsMgr(`addons://detail/${encodeURIComponent(ID4)}`);
   let button = win.document.getElementById("detail-enable-btn");
 
   // When clicking enable we should see the permissions notification
   popupPromise = promisePopupNotificationShown("addon-webext-permissions");
   BrowserTestUtils.synthesizeMouseAtCenter(button, {},
                                            gBrowser.selectedBrowser);
   panel = yield popupPromise;
+  checkNotification(panel, DEFAULT_ICON_URL, [["webextPerms.hostDescription.allUrls"]]);
 
   // Accept the permissions
   disablePromise = promiseSetDisabled(mock4);
   panel.button.click();
   value = yield disablePromise;
   is(value, false, "userDisabled should be set on addon 4");
 
   addon4 = yield AddonManager.getAddonByID(ID4);
--- a/browser/base/content/test/webextensions/head.js
+++ b/browser/base/content/test/webextensions/head.js
@@ -145,16 +145,60 @@ function checkPermissionString(string, k
     ok(string.startsWith(localizedString.slice(0, i)), msg);
     ok(string.endsWith(localizedString.slice(i + 2)), msg);
   } else {
     is(string, localizedString, msg);
   }
 }
 
 /**
+ * Check the contents of a permission popup notification
+ *
+ * @param {Window} panel
+ *        The popup window.
+ * @param {string|regexp|function} checkIcon
+ *        The icon expected to appear in the notification.  If this is a
+ *        string, it must match the icon url exactly.  If it is a
+ *        regular expression it is tested against the icon url, and if
+ *        it is a function, it is called with the icon url and returns
+ *        true if the url is correct.
+ * @param {array} permissions
+ *        The expected entries in the permissions list.  Each element
+ *        in this array is itself a 2-element array with the string key
+ *        for the item (e.g., "webextPerms.description.foo") and an
+ *        optional formatting parameter.
+ */
+function checkNotification(panel, checkIcon, permissions) {
+  let icon = panel.getAttribute("icon");
+  let ul = document.getElementById("addon-webext-perm-list");
+  let header = document.getElementById("addon-webext-perm-intro");
+
+  if (checkIcon instanceof RegExp) {
+    ok(checkIcon.test(icon), "Notification icon is correct");
+  } else if (typeof checkIcon == "function") {
+    ok(checkIcon(icon), "Notification icon is correct");
+  } else {
+    is(icon, checkIcon, "Notification icon is correct");
+  }
+
+  is(ul.childElementCount, permissions.length, `Permissions list has ${permissions.length} entries`);
+  if (permissions.length == 0) {
+    is(header.getAttribute("hidden"), "true", "Permissions header is hidden");
+  } else {
+    is(header.getAttribute("hidden"), "", "Permissions header is visible");
+  }
+
+  for (let i in permissions) {
+    let [key, param] = permissions[i];
+    checkPermissionString(ul.children[i].textContent, key, param,
+                          `Permission number ${i + 1} is correct`);
+  }
+}
+
+/**
  * 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.
  *
@@ -208,54 +252,29 @@ async function testInstallMethod(install
         },
       };
       AddonManager.addInstallListener(listener);
     });
 
     let installMethodPromise = installFn(filename);
 
     let panel = await promisePopupNotificationShown("addon-webext-permissions");
-    let icon = panel.getAttribute("icon");
-
-    let ul = document.getElementById("addon-webext-perm-list");
-    let header = document.getElementById("addon-webext-perm-intro");
-
     if (filename == PERMS_XPI) {
       // 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");
-
-      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");
+      checkNotification(panel, /^jar:file:\/\/.*\/icon\.png$/, [
+        ["webextPerms.hostDescription.wildcard", "wildcard.domain"],
+        ["webextPerms.hostDescription.oneSite", "singlehost.domain"],
+        ["webextPerms.description.nativeMessaging"],
+        ["webextPerms.description.tabs"],
+        ["webextPerms.description.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");
+      checkNotification(panel, isDefaultIcon, []);
     }
 
     if (cancel) {
       panel.secondaryButton.click();
       try {
         await installMethodPromise;
       } catch (err) {}
     } else {
--- a/browser/locales/en-US/chrome/browser/browser.properties
+++ b/browser/locales/en-US/chrome/browser/browser.properties
@@ -57,16 +57,17 @@ webextPerms.sideloadMenuItem=%1$S added 
 
 # LOCALIZATION NOTE (webextPerms.sideloadHeader)
 # This string is used as a header in the webextension permissions dialog
 # when the extension is side-loaded.
 # %S is replaced with the localized name of the extension being installed.
 # Note, this string will be used as raw markup. Avoid characters like <, >, &
 webextPerms.sideloadHeader=%S added
 webextPerms.sideloadText2=Another program on your computer installed an add-on that may affect your browser. Please review this add-on’s permissions requests and choose to Enable or Cancel (to leave it disabled).
+webextPerms.sideloadTextNoPerms=Another program on your computer installed an add-on that may affect your browser. Please choose to Enable or Cancel (to leave it disabled).
 
 webextPerms.sideloadEnable.label=Enable
 webextPerms.sideloadEnable.accessKey=E
 webextPerms.sideloadCancel.label=Cancel
 webextPerms.sideloadCancel.accessKey=C
 
 # LOCALIZATION NOTE (webextPerms.updateMenuItem)
 # %S will be replaced with the localized name of the extension which
--- a/browser/modules/ExtensionsUI.jsm
+++ b/browser/modules/ExtensionsUI.jsm
@@ -187,42 +187,16 @@ this.ExtensionsUI = {
   },
 
   // Create a set of formatted strings for a permission prompt
   _buildStrings(info) {
     let result = {};
 
     let bundle = Services.strings.createBundle(BROWSER_PROPERTIES);
 
-    let name = this._sanitizeName(info.addon.name);
-    let addonName = `<span class="addon-webext-name">${name}</span>`;
-
-    result.header = bundle.formatStringFromName("webextPerms.header", [addonName], 1);
-    result.text = "";
-    result.listIntro = bundle.GetStringFromName("webextPerms.listIntro");
-
-    result.acceptText = bundle.GetStringFromName("webextPerms.add.label");
-    result.acceptKey = bundle.GetStringFromName("webextPerms.add.accessKey");
-    result.cancelText = bundle.GetStringFromName("webextPerms.cancel.label");
-    result.cancelKey = bundle.GetStringFromName("webextPerms.cancel.accessKey");
-
-    if (info.type == "sideload") {
-      result.header = bundle.formatStringFromName("webextPerms.sideloadHeader", [addonName], 1);
-      result.text = bundle.GetStringFromName("webextPerms.sideloadText2");
-      result.acceptText = bundle.GetStringFromName("webextPerms.sideloadEnable.label");
-      result.acceptKey = bundle.GetStringFromName("webextPerms.sideloadEnable.accessKey");
-      result.cancelText = bundle.GetStringFromName("webextPerms.sideloadCancel.label");
-      result.cancelKey = bundle.GetStringFromName("webextPerms.sideloadCancel.accessKey");
-    } else if (info.type == "update") {
-      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: []};
 
     // First classify our host permissions
     let allUrls = false, wildcards = [], sites = [];
     for (let permission of perms.hosts) {
       if (permission == "<all_urls>") {
         allUrls = true;
         break;
@@ -290,16 +264,45 @@ this.ExtensionsUI = {
       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.
       }
     }
 
+    // Now figure out all the rest of the notification text.
+    let name = this._sanitizeName(info.addon.name);
+    let addonName = `<span class="addon-webext-name">${name}</span>`;
+
+    result.header = bundle.formatStringFromName("webextPerms.header", [addonName], 1);
+    result.text = "";
+    result.listIntro = bundle.GetStringFromName("webextPerms.listIntro");
+
+    result.acceptText = bundle.GetStringFromName("webextPerms.add.label");
+    result.acceptKey = bundle.GetStringFromName("webextPerms.add.accessKey");
+    result.cancelText = bundle.GetStringFromName("webextPerms.cancel.label");
+    result.cancelKey = bundle.GetStringFromName("webextPerms.cancel.accessKey");
+
+    if (info.type == "sideload") {
+      result.header = bundle.formatStringFromName("webextPerms.sideloadHeader", [addonName], 1);
+      let key = result.msgs.length == 0 ?
+                "webextPerms.sideloadTextNoPerms" : "webextPerms.sideloadText2";
+      result.text = bundle.GetStringFromName(key);
+      result.acceptText = bundle.GetStringFromName("webextPerms.sideloadEnable.label");
+      result.acceptKey = bundle.GetStringFromName("webextPerms.sideloadEnable.accessKey");
+      result.cancelText = bundle.GetStringFromName("webextPerms.sideloadCancel.label");
+      result.cancelKey = bundle.GetStringFromName("webextPerms.sideloadCancel.accessKey");
+    } else if (info.type == "update") {
+      result.header = "";
+      result.text = bundle.formatStringFromName("webextPerms.updateText", [addonName], 1);
+      result.acceptText = bundle.GetStringFromName("webextPerms.updateAccept.label");
+      result.acceptKey = bundle.GetStringFromName("webextPerms.updateAccept.accessKey");
+    }
+
     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;