Bug 1203292 - Replace permissions dropdown with 'x' icon r?paolo draft
authorRicky Chien <ricky060709@gmail.com>
Thu, 19 May 2016 18:18:03 +0800
changeset 392190 2365b49e6e52dec6f1be739d82a09777e9f8e1a7
parent 391305 e0bc88708ffed39aaab1fbc0ac461d93561195de
child 526267 2fd5a83188d8c56ece2258213eb41528da84574c
push id23955
push userbmo:rchien@mozilla.com
push dateSun, 24 Jul 2016 11:20:54 +0000
reviewerspaolo
bugs1203292
milestone50.0a1
Bug 1203292 - Replace permissions dropdown with 'x' icon r?paolo MozReview-Commit-ID: LmdRlgzeW1c
browser/base/content/browser.js
browser/base/content/test/general/browser_permissions.js
browser/themes/shared/controlcenter/panel.inc.css
browser/themes/shared/jar.inc.mn
browser/themes/shared/panel-icons.svg
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -7258,60 +7258,48 @@ var gIdentityHandler = {
     let uri = gBrowser.currentURI;
 
     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);
-    else
-      SitePermissions.set(gBrowser.currentURI, aPermission, aState);
-  },
-
   _createPermissionItem: function (aPermission) {
-    let menulist = document.createElement("menulist");
-    let menupopup = document.createElement("menupopup");
-    for (let state of aPermission.availableStates) {
-      let menuitem = document.createElement("menuitem");
-      menuitem.setAttribute("value", state.id);
-      menuitem.setAttribute("label", state.label);
-      menupopup.appendChild(menuitem);
-    }
-    menulist.appendChild(menupopup);
-    menulist.setAttribute("value", aPermission.state);
-    menulist.setAttribute("oncommand", "gIdentityHandler.setPermission('" +
-                                       aPermission.id + "', this.value)");
-    menulist.setAttribute("id", "identity-popup-permission:" + aPermission.id);
-
-    let label = document.createElement("label");
-    label.setAttribute("flex", "1");
-    label.setAttribute("class", "identity-popup-permission-label");
-    label.setAttribute("control", menulist.getAttribute("id"));
-    label.textContent = aPermission.label;
+    let container = document.createElement("hbox");
+    container.setAttribute("class", "identity-popup-permission-item");
+    container.setAttribute("align", "center");
 
     let img = document.createElement("image");
     let isBlocked = (aPermission.state == SitePermissions.BLOCK) ? " blocked" : "";
     img.setAttribute("class",
       "identity-popup-permission-icon " + aPermission.id + "-icon" + isBlocked);
 
-    let container = document.createElement("hbox");
-    container.setAttribute("align", "center");
+    let nameLabel = document.createElement("label");
+    nameLabel.setAttribute("flex", "1");
+    nameLabel.setAttribute("class", "identity-popup-permission-label");
+    nameLabel.textContent = SitePermissions.getPermissionLabel(aPermission.id);
+
+    let stateLabel = document.createElement("label");
+    stateLabel.setAttribute("flex", "1");
+    stateLabel.setAttribute("class", "identity-popup-permission-state-label");
+    stateLabel.textContent = SitePermissions.getStateLabel(
+      aPermission.id, aPermission.state);
+
+    let button = document.createElement("button");
+    button.setAttribute("class", "identity-popup-permission-remove-button");
+    button.addEventListener("command", () => {
+      this._permissionList.removeChild(container);
+      SitePermissions.remove(gBrowser.currentURI, aPermission.id);
+    });
+
     container.appendChild(img);
-    container.appendChild(label);
-    container.appendChild(menulist);
-
-    // The menuitem text can be long and we don't want the dropdown
-    // to expand to the width of unselected labels.
-    // Need to set this attribute after it's appended, otherwise it gets
-    // overridden with sizetopopup="pref".
-    menulist.setAttribute("sizetopopup", "none");
+    container.appendChild(nameLabel);
+    container.appendChild(stateLabel);
+    container.appendChild(button);
 
     return container;
   }
 };
 
 function getNotificationBox(aWindow) {
   var foundBrowser = gBrowser.getBrowserForDocument(aWindow.document);
   if (foundBrowser)
--- a/browser/base/content/test/general/browser_permissions.js
+++ b/browser/base/content/test/general/browser_permissions.js
@@ -2,17 +2,16 @@
  * Test the Permissions section in the Control Center.
  */
 
 var {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
 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();
   }
@@ -25,88 +24,110 @@ add_task(function* testMainViewVisible()
 
   let permissionsList = document.getElementById("identity-popup-permission-list");
   let emptyLabel = permissionsList.nextSibling;
 
   gIdentityHandler._identityBox.click();
   ok(!is_hidden(emptyLabel), "List of permissions is empty");
   gIdentityHandler._identityPopup.hidden = true;
 
-  gIdentityHandler.setPermission("install", SitePermissions.ALLOW);
+  SitePermissions.set(gBrowser.currentURI, "camera", SitePermissions.ALLOW);
 
   gIdentityHandler._identityBox.click();
   ok(is_hidden(emptyLabel), "List of permissions is not empty");
 
-  let labelText = SitePermissions.getPermissionLabel("install");
+  let labelText = SitePermissions.getPermissionLabel("camera");
   let labels = permissionsList.querySelectorAll(".identity-popup-permission-label");
   is(labels.length, 1, "One permission visible in main view");
   is(labels[0].textContent, labelText, "Correct value");
 
-  let menulists = permissionsList.querySelectorAll("menulist");
-  is(menulists.length, 1, "One permission visible in main view");
-  is(menulists[0].id, "identity-popup-permission:install", "Install permission visible");
-  is(menulists[0].value, "1", "Correct value on install menulist");
-  gIdentityHandler._identityPopup.hidden = true;
+  let img = permissionsList.querySelector("image.identity-popup-permission-icon");
+  ok(img, "There is an image for the permissions");
+  ok(img.classList.contains("camera-icon"), "proper class is in image class");
 
-  let img = menulists[0].parentNode.querySelector("image");
-  ok(img, "There is an image for the permissions");
-  ok(img.classList.contains("install-icon"), "proper class is in image class");
-
-  gIdentityHandler.setPermission("install", SitePermissions.getDefault("install"));
+  SitePermissions.remove(gBrowser.currentURI, "camera");
 
   gIdentityHandler._identityBox.click();
   ok(!is_hidden(emptyLabel), "List of permissions is empty");
   gIdentityHandler._identityPopup.hidden = true;
 });
 
 add_task(function* testIdentityIcon() {
   let {gIdentityHandler} = gBrowser.ownerGlobal;
   let tab = gBrowser.selectedTab = gBrowser.addTab();
   yield promiseTabLoadEvent(tab, PERMISSIONS_PAGE);
 
-  gIdentityHandler.setPermission("geo", SitePermissions.ALLOW);
+  SitePermissions.set(gBrowser.currentURI, "geo", SitePermissions.ALLOW);
 
   ok(gIdentityHandler._identityBox.classList.contains("grantedPermissions"),
     "identity-box signals granted permissions");
 
-  gIdentityHandler.setPermission("geo", SitePermissions.getDefault("geo"));
+  SitePermissions.remove(gBrowser.currentURI, "geo");
+
+  ok(!gIdentityHandler._identityBox.classList.contains("grantedPermissions"),
+    "identity-box doesn't signal granted permissions");
+
+  SitePermissions.set(gBrowser.currentURI, "camera", SitePermissions.BLOCK);
 
   ok(!gIdentityHandler._identityBox.classList.contains("grantedPermissions"),
     "identity-box doesn't signal granted permissions");
 
-  gIdentityHandler.setPermission("camera", SitePermissions.BLOCK);
-
-  ok(!gIdentityHandler._identityBox.classList.contains("grantedPermissions"),
-    "identity-box doesn't signal granted permissions");
-
-  gIdentityHandler.setPermission("cookie", SitePermissions.SESSION);
+  SitePermissions.set(gBrowser.currentURI, "cookie", SitePermissions.SESSION);
 
   ok(gIdentityHandler._identityBox.classList.contains("grantedPermissions"),
     "identity-box signals granted permissions");
 });
 
+add_task(function* testCancelPermission() {
+  let {gIdentityHandler} = gBrowser.ownerGlobal;
+  let tab = gBrowser.selectedTab = gBrowser.addTab();
+  yield promiseTabLoadEvent(tab, PERMISSIONS_PAGE);
+
+  let permissionsList = document.getElementById("identity-popup-permission-list");
+  let emptyLabel = permissionsList.nextSibling;
+
+  gIdentityHandler._identityBox.click();
+
+  SitePermissions.set(gBrowser.currentURI, "geo", SitePermissions.ALLOW);
+  SitePermissions.set(gBrowser.currentURI, "geo", SitePermissions.getDefault("geo"));
+
+  ok(is_hidden(emptyLabel), "List of permissions is not empty");
+
+  let cancelButtons = permissionsList
+    .querySelectorAll(".identity-popup-permission-remove-button");
+
+  cancelButtons[0].click();
+  let labels = permissionsList.querySelectorAll(".identity-popup-permission-label");
+  is(labels.length, 1, "One permission should be removed");
+  cancelButtons[1].click();
+  labels = permissionsList.querySelectorAll(".identity-popup-permission-label");
+  is(labels.length, 0, "One permission should be removed");
+
+  gIdentityHandler._identityPopup.hidden = true;
+});
+
 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);
+  SitePermissions.set(gBrowser.currentURI, "camera", SitePermissions.ALLOW);
+  SitePermissions.set(gBrowser.currentURI, "geo", SitePermissions.BLOCK);
+  SitePermissions.set(gBrowser.currentURI, "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"));
+  SitePermissions.set(gBrowser.currentURI, "geo", SitePermissions.getDefault("geo"));
 
   ok(!geoIcon.hasAttribute("showing"),
     "blocked permission icon is not shown after reset");
 });
--- a/browser/themes/shared/controlcenter/panel.inc.css
+++ b/browser/themes/shared/controlcenter/panel.inc.css
@@ -357,18 +357,18 @@ description#identity-popup-content-verif
 
 #identity-popup-permission-list {
   /* Offset the padding set on #identity-popup-permissions-content so that it
      shows up just below the section. The permission icons are 16px wide and
      should be right aligned with the section icon. */
   margin-inline-start: calc(-1em - 16px);
 }
 
-#identity-popup-permission-list menulist {
-  min-width: 60px;
+.identity-popup-permission-item {
+  min-height: 24px;
 }
 
 #identity-popup-permission-list:not(:empty) {
   margin-top: 5px;
 }
 
 #identity-popup-permission-list:not(:empty) + description {
   display: none;
@@ -376,10 +376,53 @@ description#identity-popup-content-verif
 
 .identity-popup-permission-icon {
   width: 16px;
   height: 16px;
 }
 
 .identity-popup-permission-label {
   margin-inline-start: 1em;
-  word-wrap: break-word;
+}
+
+.identity-popup-permission-state-label {
+  text-align: end;
+  opacity: 0.6;
+}
+
+.identity-popup-permission-remove-button {
+  -moz-appearance: none;
+  margin: 0;
+  border-width: 0;
+  border-radius: 50%;
+  min-width: 0;
+  padding: 2px;
+}
+
+.identity-popup-permission-remove-button > .button-box {
+  border-width: 0;
+  padding: 0;
 }
+
+.identity-popup-permission-remove-button > .button-box > .button-icon {
+  margin: 0;
+  width: 16px;
+  height: 16px;
+  list-style-image: url(chrome://browser/skin/panel-icons.svg#cancel);
+  filter: url(chrome://browser/skin/filters.svg#fill);
+  fill: #999;
+}
+
+.identity-popup-permission-remove-button > .button-box > .button-text {
+  display: none;
+}
+
+.identity-popup-permission-remove-button:hover {
+  background-color: #999;
+}
+
+.identity-popup-permission-remove-button:hover > .button-box > .button-icon {
+  fill: #fff;
+}
+
+.identity-popup-permission-remove-button:hover:active {
+  background-color: #808080;
+}
--- a/browser/themes/shared/jar.inc.mn
+++ b/browser/themes/shared/jar.inc.mn
@@ -67,16 +67,17 @@
   skin/classic/browser/identity-mixed-active-loaded.svg        (../shared/identity-block/identity-mixed-active-loaded.svg)
   skin/classic/browser/info.svg                                (../shared/info.svg)
   skin/classic/browser/notification-icons.svg                  (../shared/notification-icons.svg)
   skin/classic/browser/tracking-protection-16.svg              (../shared/identity-block/tracking-protection-16.svg)
   skin/classic/browser/tracking-protection-disabled-16.svg     (../shared/identity-block/tracking-protection-disabled-16.svg)
   skin/classic/browser/newtab/close.png                        (../shared/newtab/close.png)
   skin/classic/browser/newtab/controls.svg                     (../shared/newtab/controls.svg)
   skin/classic/browser/newtab/whimsycorn.png                   (../shared/newtab/whimsycorn.png)
+  skin/classic/browser/panel-icons.svg                         (../shared/panel-icons.svg)
   skin/classic/browser/preferences/in-content/favicon.ico      (../shared/incontentprefs/favicon.ico)
   skin/classic/browser/preferences/in-content/icons.svg        (../shared/incontentprefs/icons.svg)
   skin/classic/browser/preferences/in-content/search.css       (../shared/incontentprefs/search.css)
   skin/classic/browser/fxa/default-avatar.svg                  (../shared/fxa/default-avatar.svg)
   skin/classic/browser/fxa/logo.png                            (../shared/fxa/logo.png)
   skin/classic/browser/fxa/logo@2x.png                         (../shared/fxa/logo@2x.png)
   skin/classic/browser/fxa/sync-illustration.png               (../shared/fxa/sync-illustration.png)
   skin/classic/browser/fxa/sync-illustration@2x.png            (../shared/fxa/sync-illustration@2x.png)
new file mode 100644
--- /dev/null
+++ b/browser/themes/shared/panel-icons.svg
@@ -0,0 +1,8 @@
+<?xml version="1.0"?>
+<!-- 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/. -->
+<svg xmlns="http://www.w3.org/2000/svg"
+     width="32" height="32" viewBox="0 0 32 32">
+  <path id="cancel" d="m 6,9.5 6.5,6.5 -6.5,6.5 3.5,3.5 6.5,-6.5 6.5,6.5 3.5,-3.5 -6.5,-6.5 6.5,-6.5 -3.5,-3.5 -6.5,6.5 -6.5,-6.5 z" />
+</svg>