Bug 1206252 - Part 2: Indicate blocked permissions in the identity box. r=paolo
authorJohann Hofmann <jhofmann@mozilla.com>
Thu, 30 Jun 2016 17:08:16 +0200
changeset 330047 5a23c763032f87bc7530a017096fc389ce3ea3cd
parent 330046 324af61c39291787d7ce7a63f4370424312a81e7
child 330048 d868ba1645e6fbc6671c09f49f331ba4acae9ad6
push id9858
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 14:37:10 +0000
treeherdermozilla-aurora@203106ef6cb6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspaolo
bugs1206252
milestone50.0a1
Bug 1206252 - Part 2: Indicate blocked permissions in the identity box. r=paolo MozReview-Commit-ID: LpJY4RatsHP
browser/base/content/browser.js
browser/base/content/browser.xul
browser/base/content/test/general/browser_permissions.js
browser/modules/SitePermissions.jsm
browser/modules/test/xpcshell/test_SitePermissions.js
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -6642,16 +6642,24 @@ var gIdentityHandler = {
   get _identityIcon () {
     delete this._identityIcon;
     return this._identityIcon = document.getElementById("identity-icon");
   },
   get _permissionList () {
     delete this._permissionList;
     return this._permissionList = document.getElementById("identity-popup-permission-list");
   },
+  get _permissionAnchors () {
+    delete this._permissionAnchors;
+    let permissionAnchors = {};
+    for (let anchor of document.getElementById("blocked-permissions-container").children) {
+      permissionAnchors[anchor.getAttribute("data-permission-id")] = anchor;
+    }
+    return this._permissionAnchors = permissionAnchors;
+  },
 
   /**
    * Handler for mouseclicks on the "More Information" button in the
    * "identity-popup" panel.
    */
   handleMoreInfoClick : function(event) {
     displaySecurityInfo();
     event.stopPropagation();
@@ -6899,17 +6907,42 @@ var gIdentityHandler = {
     }
 
     if (this._isCertUserOverridden) {
       this._identityBox.classList.add("certUserOverridden");
       // Cert is trusted because of a security exception, verifier is a special string.
       tooltip = gNavigatorBundle.getString("identity.identified.verified_by_you");
     }
 
-    if (SitePermissions.hasGrantedPermissions(this._uri)) {
+    let permissionAnchors = this._permissionAnchors;
+
+    // hide all permission icons
+    for (let icon of Object.values(permissionAnchors)) {
+      icon.removeAttribute("showing");
+    }
+
+    // keeps track if we should show an indicator that there are active permissions
+    let hasGrantedPermissions = false;
+
+    // show permission icons
+    for (let permission of SitePermissions.getAllByURI(this._uri)) {
+      if (permission.state === SitePermissions.BLOCK) {
+
+        let icon = permissionAnchors[permission.id];
+        if (icon) {
+          icon.setAttribute("showing", "true");
+        }
+
+      } else if (permission.state === SitePermissions.ALLOW ||
+                 permission.state === SitePermissions.SESSION) {
+        hasGrantedPermissions = true;
+      }
+    }
+
+    if (hasGrantedPermissions) {
       this._identityBox.classList.add("grantedPermissions");
     }
 
     // Push the appropriate strings out to the UI
     this._identityBox.tooltipText = tooltip;
     this._identityIcon.tooltipText = gNavigatorBundle.getString("identity.icon.tooltip");
     this._identityIconLabel.value = icon_label;
     this._identityIconCountryLabel.value = icon_country_label;
@@ -7219,17 +7252,17 @@ var gIdentityHandler = {
   },
 
   updateSitePermissions: function () {
     while (this._permissionList.hasChildNodes())
       this._permissionList.removeChild(this._permissionList.lastChild);
 
     let uri = gBrowser.currentURI;
 
-    for (let permission of SitePermissions.getPermissionsByURI(uri)) {
+    for (let permission of SitePermissions.getPermissionDetailsByURI(uri)) {
       let item = this._createPermissionItem(permission);
       this._permissionList.appendChild(item);
     }
   },
 
   setPermission: function (aPermission, aState) {
     if (aState == SitePermissions.getDefault(aPermission))
       SitePermissions.remove(gBrowser.currentURI, aPermission);
--- a/browser/base/content/browser.xul
+++ b/browser/base/content/browser.xul
@@ -726,16 +726,32 @@
                    align="center"
                    aria-label="&urlbar.viewSiteInfo.label;"
                    onclick="gIdentityHandler.handleIdentityButtonEvent(event);"
                    onkeypress="gIdentityHandler.handleIdentityButtonEvent(event);"
                    ondragstart="gIdentityHandler.onDragStart(event);">
                 <image id="identity-icon"
                        consumeanchor="identity-box"
                        onclick="PageProxyClickHandler(event);"/>
+                <box id="blocked-permissions-container" align="center">
+                  <image data-permission-id="geo" class="notification-anchor-icon geo-icon blocked" role="button"
+                         aria-label="&urlbar.geolocationNotificationAnchor.label;"/>
+                  <image data-permission-id="desktop-notification" class="notification-anchor-icon desktop-notification-icon blocked" role="button"
+                         aria-label="&urlbar.webNotsNotificationAnchor3.label;"/>
+                  <image data-permission-id="camera" class="notification-anchor-icon camera-icon blocked" role="button"
+                         aria-label="&urlbar.webRTCShareDevicesNotificationAnchor.label;"/>
+                  <image data-permission-id="indexedDB" class="notification-anchor-icon indexedDB-icon blocked" role="button"
+                         aria-label="&urlbar.indexedDBNotificationAnchor.label;"/>
+                  <image data-permission-id="microphone" class="notification-anchor-icon microphone-icon blocked" role="button"
+                         aria-label="&urlbar.webRTCShareMicrophoneNotificationAnchor.label;"/>
+                  <image data-permission-id="screen" class="notification-anchor-icon screen-icon blocked" role="button"
+                         aria-label="&urlbar.webRTCShareScreenNotificationAnchor.label;"/>
+                  <image data-permission-id="pointerLock" class="notification-anchor-icon pointerLock-icon blocked" role="button"
+                         aria-label="&urlbar.pointerLockNotificationAnchor.label;"/>
+                </box>
                 <box id="notification-popup-box"
                      hidden="true"
                      tooltiptext=""
                      onmouseover="document.getElementById('identity-icon').classList.add('no-hover');"
                      onmouseout="document.getElementById('identity-icon').classList.remove('no-hover');"
                      align="center">
                   <image id="default-notification-icon" class="notification-anchor-icon" role="button"
                          aria-label="&urlbar.defaultNotificationAnchor.label;"/>
--- a/browser/base/content/test/general/browser_permissions.js
+++ b/browser/base/content/test/general/browser_permissions.js
@@ -6,16 +6,17 @@ var {classes: Cc, interfaces: Ci, utils:
 const PERMISSIONS_PAGE = "http://example.com/browser/browser/base/content/test/general/permissions.html";
 var {SitePermissions} = Cu.import("resource:///modules/SitePermissions.jsm", {});
 
 registerCleanupFunction(function() {
   SitePermissions.remove(gBrowser.currentURI, "install");
   SitePermissions.remove(gBrowser.currentURI, "cookie");
   SitePermissions.remove(gBrowser.currentURI, "geo");
   SitePermissions.remove(gBrowser.currentURI, "camera");
+  SitePermissions.remove(gBrowser.currentURI, "microphone");
 
   while (gBrowser.tabs.length > 1) {
     gBrowser.removeCurrentTab();
   }
 });
 
 add_task(function* testMainViewVisible() {
   let {gIdentityHandler} = gBrowser.ownerGlobal;
@@ -59,25 +60,53 @@ add_task(function* testMainViewVisible()
 add_task(function* testIdentityIcon() {
   let {gIdentityHandler} = gBrowser.ownerGlobal;
   let tab = gBrowser.selectedTab = gBrowser.addTab();
   yield promiseTabLoadEvent(tab, PERMISSIONS_PAGE);
 
   gIdentityHandler.setPermission("geo", SitePermissions.ALLOW);
 
   ok(gIdentityHandler._identityBox.classList.contains("grantedPermissions"),
-    "identity-box signals granted permssions");
+    "identity-box signals granted permissions");
 
   gIdentityHandler.setPermission("geo", SitePermissions.getDefault("geo"));
 
   ok(!gIdentityHandler._identityBox.classList.contains("grantedPermissions"),
-    "identity-box doesn't signal granted permssions");
+    "identity-box doesn't signal granted permissions");
 
   gIdentityHandler.setPermission("camera", SitePermissions.BLOCK);
 
   ok(!gIdentityHandler._identityBox.classList.contains("grantedPermissions"),
-    "identity-box doesn't signal granted permssions");
+    "identity-box doesn't signal granted permissions");
 
   gIdentityHandler.setPermission("cookie", SitePermissions.SESSION);
 
   ok(gIdentityHandler._identityBox.classList.contains("grantedPermissions"),
-    "identity-box signals granted permssions");
+    "identity-box signals granted permissions");
 });
+
+add_task(function* testPermissionIcons() {
+  let {gIdentityHandler} = gBrowser.ownerGlobal;
+  let tab = gBrowser.selectedTab = gBrowser.addTab();
+  yield promiseTabLoadEvent(tab, PERMISSIONS_PAGE);
+
+  gIdentityHandler.setPermission("camera", SitePermissions.ALLOW);
+  gIdentityHandler.setPermission("geo", SitePermissions.BLOCK);
+  gIdentityHandler.setPermission("microphone", SitePermissions.SESSION);
+
+  let geoIcon = gIdentityHandler._identityBox.querySelector("[data-permission-id='geo']");
+  ok(geoIcon.hasAttribute("showing"), "blocked permission icon is shown");
+  ok(geoIcon.classList.contains("blocked"),
+    "blocked permission icon is shown as blocked");
+
+  let cameraIcon = gIdentityHandler._identityBox.querySelector("[data-permission-id='camera']");
+  ok(!cameraIcon.hasAttribute("showing"),
+    "allowed permission icon is not shown");
+
+  let microphoneIcon  = gIdentityHandler._identityBox.querySelector("[data-permission-id='microphone']");
+  ok(!microphoneIcon.hasAttribute("showing"),
+    "allowed permission icon is not shown");
+
+  gIdentityHandler.setPermission("geo", SitePermissions.getDefault("geo"));
+
+  ok(!geoIcon.hasAttribute("showing"),
+    "blocked permission icon is not shown after reset");
+});
--- a/browser/modules/SitePermissions.jsm
+++ b/browser/modules/SitePermissions.jsm
@@ -11,73 +11,72 @@ var gStringBundle =
 
 this.SitePermissions = {
 
   UNKNOWN: Services.perms.UNKNOWN_ACTION,
   ALLOW: Services.perms.ALLOW_ACTION,
   BLOCK: Services.perms.DENY_ACTION,
   SESSION: Components.interfaces.nsICookiePermission.ACCESS_SESSION,
 
+  /* Returns all custom permissions for a given URI, the return
+   * type is 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)
+   *
+   * To receive a more detailed, albeit less performant listing see
+   * SitePermissions.getPermissionDetailsByURI().
+   */
+  getAllByURI: function (aURI) {
+    let result = [];
+    if (!this.isSupportedURI(aURI)) {
+      return result;
+    }
+
+    let permissions = Services.perms.getAllForURI(aURI);
+    while (permissions.hasMoreElements()) {
+      let permission = permissions.getNext();
+
+      // filter out unknown permissions
+      if (gPermissionObject[permission.type]) {
+        result.push({
+          id: permission.type,
+          state: permission.capability,
+        });
+      }
+    }
+
+    return result;
+  },
+
   /* Returns a list of objects representing all permissions that are currently
    * set for the given URI. Each object contains the following keys:
    * - id: the permissionID of the permission
    * - label: the localized label
    * - state: a constant representing the current permission state
    *   (e.g. SitePermissions.ALLOW)
    * - 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
    */
-  getPermissionsByURI: function (aURI) {
-    if (!this.isSupportedURI(aURI)) {
-      return [];
-    }
-
+  getPermissionDetailsByURI: function (aURI) {
     let permissions = [];
-    for (let permission of kPermissionIDs) {
-      let state = this.get(aURI, permission);
-      if (state === this.UNKNOWN) {
-        continue;
-      }
+    for (let {state, id} of this.getAllByURI(aURI)) {
+      let availableStates = this.getAvailableStates(id).map( state => {
+        return { id: state, label: this.getStateLabel(id, state) };
+      });
+      let label = this.getPermissionLabel(id);
 
-      let availableStates = this.getAvailableStates(permission).map( state => {
-        return { id: state, label: this.getStateLabel(permission, state) };
-      });
-      let label = this.getPermissionLabel(permission);
-
-      permissions.push({
-        id: permission,
-        label: label,
-        state: state,
-        availableStates: availableStates,
-      });
+      permissions.push({id, label, state, availableStates});
     }
 
     return permissions;
   },
 
-  /* Returns a boolean indicating whether there are any granted
-   * (meaning allowed or session-allowed) permissions for the given URI.
-   * Will return false for invalid URIs (such as file:// URLs).
-   */
-  hasGrantedPermissions: function (aURI) {
-    if (!this.isSupportedURI(aURI)) {
-      return false;
-    }
-
-    for (let permission of kPermissionIDs) {
-      let state = this.get(aURI, permission);
-      if (state === this.ALLOW || state === this.SESSION) {
-        return true;
-      }
-    }
-    return false;
-  },
-
   /* 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.
    */
   isSupportedURI: function (aURI) {
     return aURI.schemeIs("http") || aURI.schemeIs("https");
   },
 
--- a/browser/modules/test/xpcshell/test_SitePermissions.js
+++ b/browser/modules/test/xpcshell/test_SitePermissions.js
@@ -8,84 +8,79 @@ Components.utils.import("resource://gre/
 
 add_task(function* testPermissionsListing() {
   Assert.deepEqual(SitePermissions.listPermissions().sort(),
     ["camera","cookie","desktop-notification","geo","image",
      "indexedDB","install","microphone","popup"],
     "Correct list of all permissions");
 });
 
-add_task(function* testHasGrantedPermissions() {
-  // check that it returns false on an invalid URI
-  // like a file URI, which doesn't support site permissions
-  let wrongURI = Services.io.newURI("file:///example.js", null, null)
-  Assert.equal(SitePermissions.hasGrantedPermissions(wrongURI), false);
-
-  let uri = Services.io.newURI("https://example.com", null, null)
-  Assert.equal(SitePermissions.hasGrantedPermissions(uri), false);
-
-  // check that ALLOW states return true
-  SitePermissions.set(uri, "camera", SitePermissions.ALLOW);
-  Assert.equal(SitePermissions.hasGrantedPermissions(uri), true);
-
-  // removing the ALLOW state should revert to false
-  SitePermissions.remove(uri, "camera");
-  Assert.equal(SitePermissions.hasGrantedPermissions(uri), false);
-
-  // check that SESSION states return true
-  SitePermissions.set(uri, "microphone", SitePermissions.SESSION);
-  Assert.equal(SitePermissions.hasGrantedPermissions(uri), true);
-
-  // removing the SESSION state should revert to false
-  SitePermissions.remove(uri, "microphone");
-  Assert.equal(SitePermissions.hasGrantedPermissions(uri), false);
-
-  // check that a combination of ALLOW and BLOCK states returns true
-  SitePermissions.set(uri, "geo", SitePermissions.ALLOW);
-  Assert.equal(SitePermissions.hasGrantedPermissions(uri), true);
-
-  // check that a combination of SESSION and BLOCK states returns true
-  SitePermissions.set(uri, "geo", SitePermissions.SESSION);
-  Assert.equal(SitePermissions.hasGrantedPermissions(uri), true);
-
-  // check that only BLOCK states will not return true
-  SitePermissions.remove(uri, "geo");
-  Assert.equal(SitePermissions.hasGrantedPermissions(uri), false);
-
-});
-
-add_task(function* testGetPermissionsByURI() {
+add_task(function* testGetAllByURI() {
   // check that it returns an empty array on an invalid URI
   // like a file URI, which doesn't support site permissions
   let wrongURI = Services.io.newURI("file:///example.js", null, null)
-  Assert.deepEqual(SitePermissions.getPermissionsByURI(wrongURI), []);
+  Assert.deepEqual(SitePermissions.getAllByURI(wrongURI), []);
+
+  let uri = Services.io.newURI("https://example.com", null, null)
+  Assert.deepEqual(SitePermissions.getAllByURI(uri), []);
+
+  SitePermissions.set(uri, "camera", SitePermissions.ALLOW);
+  Assert.deepEqual(SitePermissions.getAllByURI(uri), [
+      { id: "camera", state: SitePermissions.ALLOW }
+  ]);
+
+  SitePermissions.set(uri, "microphone", SitePermissions.SESSION);
+  SitePermissions.set(uri, "desktop-notification", SitePermissions.BLOCK);
+
+  Assert.deepEqual(SitePermissions.getAllByURI(uri), [
+      { id: "camera", state: SitePermissions.ALLOW },
+      { id: "microphone", state: SitePermissions.SESSION },
+      { id: "desktop-notification", state: SitePermissions.BLOCK }
+  ]);
+
+  SitePermissions.remove(uri, "microphone");
+  Assert.deepEqual(SitePermissions.getAllByURI(uri), [
+      { id: "camera", state: SitePermissions.ALLOW },
+      { id: "desktop-notification", state: SitePermissions.BLOCK }
+  ]);
+
+  SitePermissions.remove(uri, "camera");
+  SitePermissions.remove(uri, "desktop-notification");
+  Assert.deepEqual(SitePermissions.getAllByURI(uri), []);
+});
+
+add_task(function* testGetPermissionDetailsByURI() {
+  // check that it returns an empty array on an invalid URI
+  // like a file URI, which doesn't support site permissions
+  let wrongURI = Services.io.newURI("file:///example.js", null, null)
+  Assert.deepEqual(SitePermissions.getPermissionDetailsByURI(wrongURI), []);
 
   let uri = Services.io.newURI("https://example.com", null, null)
 
   SitePermissions.set(uri, "camera", SitePermissions.ALLOW);
   SitePermissions.set(uri, "cookie", SitePermissions.SESSION);
   SitePermissions.set(uri, "popup", SitePermissions.BLOCK);
 
-  let permissions = SitePermissions.getPermissionsByURI(uri);
+  let permissions = SitePermissions.getPermissionDetailsByURI(uri);
 
   let camera = permissions.find(({id}) => id === "camera");
   Assert.deepEqual(camera, {
     id: "camera",
     label: "Use the Camera",
     state: SitePermissions.ALLOW,
     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
   SitePermissions.remove(uri, "camera");
-  permissions = SitePermissions.getPermissionsByURI(uri);
+  permissions = SitePermissions.getPermissionDetailsByURI(uri);
 
   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, {