Bug 1452361 - Don't reset to default permissions for cookies in page info. r=nhnt11, a=RyanVM
authorJohann Hofmann <jhofmann@mozilla.com>
Wed, 09 May 2018 15:39:26 +0200
changeset 470827 f23b9482597f
parent 470826 e303321bec18
child 470828 2bbdeafb0a58
push id9235
push userryanvm@gmail.com
push date2018-05-17 13:49 +0000
treeherdermozilla-beta@39f739efe104 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnhnt11, RyanVM
bugs1452361
milestone61.0
Bug 1452361 - Don't reset to default permissions for cookies in page info. r=nhnt11, a=RyanVM This commit makes the page info window treat cookie permissions a little differently, to reflect that the "default" value for cookies is a combination of two prefs that doesn't strictly map onto the ALLOW/SESSION/DENY system of permissions. I also added some more general pageinfo permissions tests. MozReview-Commit-ID: 80vd61Rv867
browser/base/content/pageinfo/permissions.js
browser/base/content/test/pageinfo/browser.ini
browser/base/content/test/pageinfo/browser_pageinfo_permissions.js
browser/modules/SitePermissions.jsm
--- a/browser/base/content/pageinfo/permissions.js
+++ b/browser/base/content/pageinfo/permissions.js
@@ -58,16 +58,37 @@ function onUnloadPermission() {
 function initRow(aPartId) {
   createRow(aPartId);
 
   var checkbox = document.getElementById(aPartId + "Def");
   var command  = document.getElementById("cmd_" + aPartId + "Toggle");
   var {state, scope} = SitePermissions.get(gPermURI, aPartId);
   let defaultState = SitePermissions.getDefault(aPartId);
 
+  // Since cookies preferences have many different possible configuration states
+  // we don't consider any permission except "no permission" to be default.
+  if (aPartId == "cookie") {
+    state = Services.perms.testPermissionFromPrincipal(gPermPrincipal, "cookie");
+
+    if (state == SitePermissions.UNKNOWN) {
+      checkbox.checked = true;
+      command.setAttribute("disabled", "true");
+      // Don't select any item in the radio group, as we can't
+      // confidently say that all cookies on the site will be allowed.
+      let radioGroup = document.getElementById("cookieRadioGroup");
+      radioGroup.selectedItem = null;
+    } else {
+      checkbox.checked = false;
+      command.removeAttribute("disabled");
+    }
+
+    setRadioState(aPartId, state);
+    return;
+  }
+
   // When flash permission state is "Hide", we show it as "Always Ask" in page info.
   if (aPartId.startsWith("plugin") && state == SitePermissions.PROMPT_HIDE) {
     defaultState == SitePermissions.UNKNOWN ? state = defaultState : state = SitePermissions.PROMPT;
   }
 
   if (state != defaultState) {
     checkbox.checked = false;
     command.removeAttribute("disabled");
@@ -141,27 +162,25 @@ function createRow(aPartId) {
 }
 
 function onCheckboxClick(aPartId) {
   var command  = document.getElementById("cmd_" + aPartId + "Toggle");
   var checkbox = document.getElementById(aPartId + "Def");
   if (checkbox.checked) {
     SitePermissions.remove(gPermURI, aPartId);
     command.setAttribute("disabled", "true");
-    var perm = SitePermissions.getDefault(aPartId);
-    setRadioState(aPartId, perm);
   } else {
     onRadioClick(aPartId);
     command.removeAttribute("disabled");
   }
 }
 
 function onRadioClick(aPartId) {
   var radioGroup = document.getElementById(aPartId + "RadioGroup");
-  var id = radioGroup.selectedItem.id;
+  var id = radioGroup.selectedItem ? radioGroup.selectedItem.id : "#1";
   var permission = parseInt(id.split("#")[1]);
   SitePermissions.set(gPermURI, aPartId, permission);
 }
 
 function setRadioState(aPartId, aValue) {
   var radio = document.getElementById(aPartId + "#" + aValue);
   if (radio) {
     radio.radioGroup.selectedItem = radio;
--- a/browser/base/content/test/pageinfo/browser.ini
+++ b/browser/base/content/test/pageinfo/browser.ini
@@ -8,13 +8,14 @@ support-files =
   image.html
   ../general/audio.ogg
   ../general/moz.png
   ../general/video.ogg
 [browser_pageinfo_images.js]
 [browser_pageinfo_image_info.js]
 uses-unsafe-cpows = true
 skip-if = (os == 'linux' && e10s) # bug 1161699
+[browser_pageinfo_permissions.js]
 [browser_pageinfo_security.js]
 [browser_pageinfo_svg_image.js]
 support-files =
   svg_image.html
   ../general/title_test.svg
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/pageinfo/browser_pageinfo_permissions.js
@@ -0,0 +1,109 @@
+ChromeUtils.import("resource:///modules/SitePermissions.jsm");
+
+const TEST_ORIGIN = "https://example.com";
+
+async function testPermissions(defaultPermission) {
+  await BrowserTestUtils.withNewTab(TEST_ORIGIN, async function(browser) {
+    let pageInfo = BrowserPageInfo(TEST_ORIGIN, "permTab");
+    await BrowserTestUtils.waitForEvent(pageInfo, "load");
+
+    let defaultCheckbox = await TestUtils.waitForCondition(() => pageInfo.document.getElementById("geoDef"));
+    let radioGroup = pageInfo.document.getElementById("geoRadioGroup");
+    let defaultRadioButton = pageInfo.document.getElementById("geo#" + defaultPermission);
+    let blockRadioButton = pageInfo.document.getElementById("geo#2");
+
+    ok(defaultCheckbox.checked, "The default checkbox should be checked.");
+
+    SitePermissions.set(gBrowser.currentURI, "geo", SitePermissions.BLOCK);
+
+    ok(!defaultCheckbox.checked, "The default checkbox should not be checked.");
+
+    defaultCheckbox.checked = true;
+    defaultCheckbox.dispatchEvent(new Event("command"));
+
+    is(SitePermissions.get(gBrowser.currentURI, "geo").state, defaultPermission,
+      "Checking the default checkbox should reset the permission.");
+
+    defaultCheckbox.checked = false;
+    defaultCheckbox.dispatchEvent(new Event("command"));
+
+    is(SitePermissions.get(gBrowser.currentURI, "geo").state, defaultPermission,
+      "Unchecking the default checkbox should pick the default permission.");
+    is(radioGroup.selectedItem, defaultRadioButton,
+      "The unknown radio button should be selected.");
+
+    radioGroup.selectedItem = blockRadioButton;
+    blockRadioButton.dispatchEvent(new Event("command"));
+
+    is(SitePermissions.get(gBrowser.currentURI, "geo").state, SitePermissions.BLOCK,
+      "Selecting a value in the radio group should set the corresponding permission");
+
+    radioGroup.selectedItem = defaultRadioButton;
+    defaultRadioButton.dispatchEvent(new Event("command"));
+
+    is(SitePermissions.get(gBrowser.currentURI, "geo").state, defaultPermission,
+      "Selecting the default value should reset the permission.");
+    ok(defaultCheckbox.checked, "The default checkbox should be checked.");
+
+    pageInfo.close();
+    SitePermissions.remove(gBrowser.currentURI, "geo");
+  });
+}
+
+// Test some standard operations in the permission tab.
+add_task(async function test_geo_permission() {
+  await testPermissions(SitePermissions.UNKNOWN);
+});
+
+// Test some standard operations in the permission tab, falling back to a custom
+// default permission instead of UNKNOWN.
+add_task(async function test_default_geo_permission() {
+  await SpecialPowers.pushPrefEnv({set: [["permissions.default.geo", SitePermissions.ALLOW]]});
+  await testPermissions(SitePermissions.ALLOW);
+});
+
+// Test special behavior for cookie permissions.
+add_task(async function test_cookie_permission() {
+  await BrowserTestUtils.withNewTab(TEST_ORIGIN, async function(browser) {
+    let pageInfo = BrowserPageInfo(TEST_ORIGIN, "permTab");
+    await BrowserTestUtils.waitForEvent(pageInfo, "load");
+
+    let defaultCheckbox = await TestUtils.waitForCondition(() => pageInfo.document.getElementById("cookieDef"));
+    let radioGroup = pageInfo.document.getElementById("cookieRadioGroup");
+    let allowRadioButton = pageInfo.document.getElementById("cookie#1");
+    let blockRadioButton = pageInfo.document.getElementById("cookie#2");
+
+    ok(defaultCheckbox.checked, "The default checkbox should be checked.");
+
+    defaultCheckbox.checked = false;
+    defaultCheckbox.dispatchEvent(new Event("command"));
+
+    is(Services.perms.testPermission(gBrowser.currentURI, "cookie"), SitePermissions.ALLOW,
+      "Unchecking the default checkbox should pick the default permission.");
+    is(radioGroup.selectedItem, allowRadioButton,
+      "The unknown radio button should be selected.");
+
+    radioGroup.selectedItem = blockRadioButton;
+    blockRadioButton.dispatchEvent(new Event("command"));
+
+    is(Services.perms.testPermission(gBrowser.currentURI, "cookie"), SitePermissions.BLOCK,
+      "Selecting a value in the radio group should set the corresponding permission");
+
+    radioGroup.selectedItem = allowRadioButton;
+    allowRadioButton.dispatchEvent(new Event("command"));
+
+    is(Services.perms.testPermission(gBrowser.currentURI, "cookie"), SitePermissions.ALLOW,
+      "Selecting a value in the radio group should set the corresponding permission");
+    ok(!defaultCheckbox.checked, "The default checkbox should not be checked.");
+
+    defaultCheckbox.checked = true;
+    defaultCheckbox.dispatchEvent(new Event("command"));
+
+    is(Services.perms.testPermission(gBrowser.currentURI, "cookie"), SitePermissions.UNKNOWN,
+      "Checking the default checkbox should reset the permission.");
+    is(radioGroup.selectedItem, null, "For cookies, no item should be selected when the checkbox is checked.");
+
+    pageInfo.close();
+    SitePermissions.remove(gBrowser.currentURI, "cookie");
+  });
+});
--- a/browser/modules/SitePermissions.jsm
+++ b/browser/modules/SitePermissions.jsm
@@ -400,18 +400,23 @@ var SitePermissions = {
    * @param {SitePermissions scope} scope (optional)
    *        The scope of the permission. Defaults to SCOPE_PERSISTENT.
    * @param {Browser} browser (optional)
    *        The browser object to set temporary permissions on.
    *        This needs to be provided if the scope is SCOPE_TEMPORARY!
    */
   set(uri, permissionID, state, scope = this.SCOPE_PERSISTENT, browser = null) {
     if (state == this.UNKNOWN || state == this.getDefault(permissionID)) {
-      this.remove(uri, permissionID, browser);
-      return;
+      // Because they are controlled by two prefs with many states that do not
+      // correspond to the classical ALLOW/DENY/PROMPT model, we want to always
+      // allow the user to add exceptions to their cookie rules without removing them.
+      if (permissionID != "cookie") {
+        this.remove(uri, permissionID, browser);
+        return;
+      }
     }
 
     if (state == this.ALLOW_COOKIES_FOR_SESSION && permissionID != "cookie") {
       throw "ALLOW_COOKIES_FOR_SESSION can only be set on the cookie permission";
     }
 
     // Save temporary permissions.
     if (scope == this.SCOPE_TEMPORARY) {
@@ -602,20 +607,20 @@ var gPermissionObject = {
 
   "image": {
     states: [ SitePermissions.ALLOW, SitePermissions.BLOCK ],
   },
 
   "cookie": {
     states: [ SitePermissions.ALLOW, SitePermissions.ALLOW_COOKIES_FOR_SESSION, SitePermissions.BLOCK ],
     getDefault() {
-      if (Services.prefs.getIntPref("network.cookie.cookieBehavior") == 2)
+      if (Services.prefs.getIntPref("network.cookie.cookieBehavior") == Ci.nsICookieService.BEHAVIOR_REJECT)
         return SitePermissions.BLOCK;
 
-      if (Services.prefs.getIntPref("network.cookie.lifetimePolicy") == 2)
+      if (Services.prefs.getIntPref("network.cookie.lifetimePolicy") == Ci.nsICookieService.ACCEPT_SESSION)
         return SitePermissions.ALLOW_COOKIES_FOR_SESSION;
 
       return SitePermissions.ALLOW;
     }
   },
 
   "desktop-notification": {
     exactHostMatch: true,