Bug 1331172 - Current permission state should use the past tense. r=Paolo
authorDão Gottwald <dao@mozilla.com>
Mon, 16 Jan 2017 20:33:00 +0100
changeset 377550 de03a86224cd02af7166f276554122f628504c10
parent 377549 ef92919b4acabfb3ffd816a63ff93faa85d2057f
child 377551 ee77d1e9cfda6340310a8cac83d952fb25e4137e
push id1419
push userjlund@mozilla.com
push dateMon, 10 Apr 2017 20:44:07 +0000
treeherdermozilla-release@5e6801b73ef6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersPaolo
bugs1331172
milestone53.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1331172 - Current permission state should use the past tense. r=Paolo MozReview-Commit-ID: 7XqHa0xrfsh
browser/base/content/browser.js
browser/base/content/pageinfo/permissions.js
browser/locales/en-US/chrome/browser/sitePermissions.properties
browser/modules/SitePermissions.jsm
browser/modules/test/browser_SitePermissions.js
browser/modules/test/xpcshell/test_SitePermissions.js
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -7407,20 +7407,22 @@ var gIdentityHandler = {
             found = true;
             permission.inUse = true;
             break;
           }
           if (!found) {
             // If the permission item we were looking for doesn't exist,
             // the user has temporarily allowed sharing and we need to add
             // an item in the permissions array to reflect this.
-            let permission =
-              SitePermissions.getPermissionDetails(id, SitePermissions.SCOPE_REQUEST);
-            permission.inUse = true;
-            permissions.push(permission);
+            permissions.push({
+              id,
+              state: SitePermissions.ALLOW,
+              scope: SitePermissions.SCOPE_REQUEST,
+              inUse: true,
+            });
           }
         }
       }
     }
     for (let permission of permissions) {
       let item = this._createPermissionItem(permission);
       this._permissionList.appendChild(item);
     }
@@ -7471,17 +7473,17 @@ var gIdentityHandler = {
     stateLabel.setAttribute("class", "identity-popup-permission-state-label");
     let {state, scope} = aPermission;
     // If the user did not permanently allow this device but it is currently
     // used, set the variables to display a "temporarily allowed" info.
     if (state != SitePermissions.ALLOW && aPermission.inUse) {
       state = SitePermissions.ALLOW;
       scope = SitePermissions.SCOPE_REQUEST;
     }
-    stateLabel.textContent = SitePermissions.getStateLabel(state, scope);
+    stateLabel.textContent = SitePermissions.getCurrentStateLabel(state, scope);
 
     let button = document.createElement("button");
     button.setAttribute("class", "identity-popup-permission-remove-button");
     let tooltiptext = gNavigatorBundle.getString("permissions.remove.tooltip");
     button.setAttribute("tooltiptext", tooltiptext);
     button.addEventListener("command", () => {
 	  let browser = gBrowser.selectedBrowser;
       // Only resize the window if the reload hint was previously hidden.
--- a/browser/base/content/pageinfo/permissions.js
+++ b/browser/base/content/pageinfo/permissions.js
@@ -130,17 +130,17 @@ function createRow(aPartId) {
   controls.appendChild(spacer);
 
   let radiogroup = document.createElement("radiogroup");
   radiogroup.setAttribute("id", radiogroupId);
   radiogroup.setAttribute("orient", "horizontal");
   for (let state of SitePermissions.getAvailableStates(aPartId)) {
     let radio = document.createElement("radio");
     radio.setAttribute("id", aPartId + "#" + state);
-    radio.setAttribute("label", SitePermissions.getStateLabel(state));
+    radio.setAttribute("label", SitePermissions.getMultichoiceStateLabel(state));
     radio.setAttribute("command", commandId);
     radiogroup.appendChild(radio);
   }
   controls.appendChild(radiogroup);
 
   row.appendChild(controls);
 
   document.getElementById("permList").appendChild(row);
--- a/browser/locales/en-US/chrome/browser/sitePermissions.properties
+++ b/browser/locales/en-US/chrome/browser/sitePermissions.properties
@@ -1,18 +1,34 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
-allow = Allow
-allowForSession = Allow for Session
-allowTemporarily = Allow Temporarily
-block = Block
-blockTemporarily = Block Temporarily
-alwaysAsk = Always Ask
+# LOCALIZATION NOTE (state.current.allowed,
+#                    state.current.allowedForSession,
+#                    state.current.allowedTemporarily,
+#                    state.current.blockedTemporarily,
+#                    state.current.blocked):
+# This label is used to display active permission states in the site
+# identity popup (which does not have a lot of screen space).
+state.current.allowed = Allowed
+state.current.allowedForSession = Allowed for Session
+state.current.allowedTemporarily = Allowed Temporarily
+state.current.blockedTemporarily = Blocked Temporarily
+state.current.blocked = Blocked
+
+# LOCALIZATION NOTE (state.multichoice.alwaysAsk,
+#                    state.multichoice.allow,
+#                    state.multichoice.allowForSession,
+#                    state.multichoice.block):
+# Used to label permission state checkboxes in the page info dialog.
+state.multichoice.alwaysAsk = Always Ask
+state.multichoice.allow = Allow
+state.multichoice.allowForSession = Allow for Session
+state.multichoice.block = Block
 
 permission.cookie.label = Set Cookies
 permission.desktop-notification2.label = Receive Notifications
 permission.image.label = Load Images
 permission.camera.label = Use the Camera
 permission.microphone.label = Use the Microphone
 permission.screen.label = Share the Screen
 permission.install.label = Install Add-ons
--- a/browser/modules/SitePermissions.jsm
+++ b/browser/modules/SitePermissions.jsm
@@ -179,44 +179,16 @@ this.SitePermissions = {
         });
       }
     }
 
     return result;
   },
 
   /**
-   * Returns detailed information on the specified permission.
-   *
-   * @param {String} id
-   *        The permissionID of the permission.
-   * @param {SitePermissions scope} scope
-   *        The current scope of the permission.
-   * @param {SitePermissions state} state (optional)
-   *        The current state of the permission.
-   *        Will default to the default state if omitted.
-   *
-   * @return {Object} an object with the keys:
-   *           - id: the permissionID of the permission
-   *           - label: the localized label
-   *           - state: the passed in state argument
-   *           - scope: the passed in scope argument
-   *           - availableStates: an array of all available states for that permission,
-   *             represented as objects with the keys:
-   *             - id: the state constant
-   *             - label: the translated label of that state
-   */
-  getPermissionDetails(id, scope, state = this.getDefault(id)) {
-    let availableStates = this.getAvailableStates(id).map(val => {
-      return { id: val, label: this.getStateLabel(val) };
-    });
-    return {id, label: this.getPermissionLabel(id), state, scope, availableStates};
-  },
-
-  /**
    * Returns all custom permissions for a given browser.
    *
    * To receive a more detailed, albeit less performant listing see
    * SitePermissions.getAllPermissionDetailsForBrowser().
    *
    * @param {Browser} browser
    *        The browser to fetch permission for.
    *
@@ -244,21 +216,27 @@ this.SitePermissions = {
 
   /**
    * Returns a list of objects with detailed information on all permissions
    * that are currently set for the given browser.
    *
    * @param {Browser} browser
    *        The browser to fetch permission for.
    *
-   * @return {Array} a list of objects. See getPermissionDetails for the content of each object.
+   * @return {Array<Object>} a list of objects with the keys:
+   *           - id: the permissionID of the permission
+   *           - state: a constant representing the current permission state
+   *             (e.g. SitePermissions.ALLOW)
+   *           - scope: a constant representing how long the permission will
+   *             be kept.
+   *           - label: the localized label
    */
   getAllPermissionDetailsForBrowser(browser) {
     return this.getAllForBrowser(browser).map(({id, scope, state}) =>
-      this.getPermissionDetails(id, scope, state));
+      ({id, scope, state, label: this.getPermissionLabel(id)}));
   },
 
   /**
    * Checks whether a UI for managing permissions should be exposed for a given
    * URI. This excludes file URIs, for instance, as they don't have a host,
    * even though nsIPermissionManager can still handle them.
    *
    * @param {nsIURI} uri
@@ -484,39 +462,62 @@ this.SitePermissions = {
   },
 
   /**
    * Returns the localized label for the given permission state, to be used in
    * a UI for managing permissions.
    *
    * @param {SitePermissions state} state
    *        The state to get the label for.
+   *
+   * @return {String|null} the localized label or null if an
+   *         unknown state was passed.
+   */
+  getMultichoiceStateLabel(state) {
+    switch (state) {
+      case this.UNKNOWN:
+        return gStringBundle.GetStringFromName("state.multichoice.alwaysAsk");
+      case this.ALLOW:
+        return gStringBundle.GetStringFromName("state.multichoice.allow");
+      case this.ALLOW_COOKIES_FOR_SESSION:
+        return gStringBundle.GetStringFromName("state.multichoice.allowForSession");
+      case this.BLOCK:
+        return gStringBundle.GetStringFromName("state.multichoice.block");
+      default:
+        return null;
+    }
+  },
+
+  /**
+   * Returns the localized label for a permission's current state.
+   *
+   * @param {SitePermissions state} state
+   *        The state to get the label for.
    * @param {SitePermissions scope} scope (optional)
    *        The scope to get the label for.
    *
-   * @return {String} the localized label.
+   * @return {String|null} the localized label or null if an
+   *         unknown state was passed.
    */
-  getStateLabel(state, scope = null) {
+  getCurrentStateLabel(state, scope = null) {
     switch (state) {
-      case this.UNKNOWN:
-        return gStringBundle.GetStringFromName("alwaysAsk");
       case this.ALLOW:
         if (scope && scope != this.SCOPE_PERSISTENT)
-          return gStringBundle.GetStringFromName("allowTemporarily");
-        return gStringBundle.GetStringFromName("allow");
+          return gStringBundle.GetStringFromName("state.current.allowedTemporarily");
+        return gStringBundle.GetStringFromName("state.current.allowed");
       case this.ALLOW_COOKIES_FOR_SESSION:
-        return gStringBundle.GetStringFromName("allowForSession");
+        return gStringBundle.GetStringFromName("state.current.allowedForSession");
       case this.BLOCK:
         if (scope && scope != this.SCOPE_PERSISTENT)
-          return gStringBundle.GetStringFromName("blockTemporarily");
-        return gStringBundle.GetStringFromName("block");
+          return gStringBundle.GetStringFromName("state.current.blockedTemporarily");
+        return gStringBundle.GetStringFromName("state.current.blocked");
       default:
         return null;
     }
-  }
+  },
 };
 
 var gPermissionObject = {
   /* Holds permission ID => options pairs.
    *
    * Supported options:
    *
    *  - exactHostMatch
--- a/browser/modules/test/browser_SitePermissions.js
+++ b/browser/modules/test/browser_SitePermissions.js
@@ -33,68 +33,47 @@ add_task(function* testGetAllPermissionD
   let permissions = SitePermissions.getAllPermissionDetailsForBrowser(tab.linkedBrowser);
 
   let camera = permissions.find(({id}) => id === "camera");
   Assert.deepEqual(camera, {
     id: "camera",
     label: "Use the Camera",
     state: SitePermissions.ALLOW,
     scope: SitePermissions.SCOPE_PERSISTENT,
-    availableStates: [
-      { id: SitePermissions.UNKNOWN, label: "Always Ask" },
-      { id: SitePermissions.ALLOW, label: "Allow" },
-      { id: SitePermissions.BLOCK, label: "Block" },
-    ]
   });
 
-  // check that removed permissions (State.UNKNOWN) are skipped
+  // Check that removed permissions (State.UNKNOWN) are skipped.
   SitePermissions.remove(uri, "camera");
   permissions = SitePermissions.getAllPermissionDetailsForBrowser(tab.linkedBrowser);
 
   camera = permissions.find(({id}) => id === "camera");
   Assert.equal(camera, undefined);
 
-  // check that different available state values are represented
-
   let cookie = permissions.find(({id}) => id === "cookie");
   Assert.deepEqual(cookie, {
     id: "cookie",
     label: "Set Cookies",
     state: SitePermissions.ALLOW_COOKIES_FOR_SESSION,
     scope: SitePermissions.SCOPE_PERSISTENT,
-    availableStates: [
-      { id: SitePermissions.ALLOW, label: "Allow" },
-      { id: SitePermissions.ALLOW_COOKIES_FOR_SESSION, label: "Allow for Session" },
-      { id: SitePermissions.BLOCK, label: "Block" },
-    ]
   });
 
   let popup = permissions.find(({id}) => id === "popup");
   Assert.deepEqual(popup, {
     id: "popup",
     label: "Open Pop-up Windows",
     state: SitePermissions.BLOCK,
     scope: SitePermissions.SCOPE_PERSISTENT,
-    availableStates: [
-      { id: SitePermissions.ALLOW, label: "Allow" },
-      { id: SitePermissions.BLOCK, label: "Block" },
-    ]
   });
 
   let geo = permissions.find(({id}) => id === "geo");
   Assert.deepEqual(geo, {
     id: "geo",
     label: "Access Your Location",
     state: SitePermissions.ALLOW,
     scope: SitePermissions.SCOPE_SESSION,
-    availableStates: [
-      { id: SitePermissions.UNKNOWN, label: "Always Ask" },
-      { id: SitePermissions.ALLOW, label: "Allow" },
-      { id: SitePermissions.BLOCK, label: "Block" },
-    ]
   });
 
   SitePermissions.remove(uri, "cookie");
   SitePermissions.remove(uri, "popup");
   SitePermissions.remove(uri, "geo");
 
   yield BrowserTestUtils.removeTab(gBrowser.selectedTab);
 });
--- a/browser/modules/test/xpcshell/test_SitePermissions.js
+++ b/browser/modules/test/xpcshell/test_SitePermissions.js
@@ -46,8 +46,24 @@ add_task(function* testGetAllByURI() {
   SitePermissions.remove(uri, "desktop-notification");
   Assert.deepEqual(SitePermissions.getAllByURI(uri), []);
 
   // XXX Bug 1303108 - Control Center should only show non-default permissions
   SitePermissions.set(uri, "addon", SitePermissions.BLOCK);
   Assert.deepEqual(SitePermissions.getAllByURI(uri), []);
   SitePermissions.remove(uri, "addon");
 });
+
+add_task(function* testGetAvailableStates() {
+  Assert.deepEqual(SitePermissions.getAvailableStates("camera"),
+                   [ SitePermissions.UNKNOWN,
+                     SitePermissions.ALLOW,
+                     SitePermissions.BLOCK ]);
+
+  Assert.deepEqual(SitePermissions.getAvailableStates("cookie"),
+                   [ SitePermissions.ALLOW,
+                     SitePermissions.ALLOW_COOKIES_FOR_SESSION,
+                     SitePermissions.BLOCK ]);
+
+  Assert.deepEqual(SitePermissions.getAvailableStates("popup"),
+                   [ SitePermissions.ALLOW,
+                     SitePermissions.BLOCK ]);
+});