Bug 1510960 - Fix the problem when the user removes all permissions from cookies permission and then try to add the same before clicking on "Save Changes". r=johannh
authorCarolina Jimenez Gomez <carolina.jimenez.g@gmail.com>
Mon, 01 Apr 2019 12:41:51 +0000
changeset 467455 5d5338fc46c89bc61ed12fb07b71b0c9093997c2
parent 467454 360e565d9175b08cbec70e1bdf5e5a14dbeb0713
child 467456 3b7bf999ceb61ab93638fe7afc0c5f36670e0e27
push id35799
push usercbrindusan@mozilla.com
push dateTue, 02 Apr 2019 08:35:12 +0000
treeherdermozilla-central@ea0977445697 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjohannh
bugs1510960
milestone68.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 1510960 - Fix the problem when the user removes all permissions from cookies permission and then try to add the same before clicking on "Save Changes". r=johannh The problem was that it only apply changes when the user clicks on save button, so, some times there is a permission that has to be added but is also in the permissions to delete, so, the solution is just delete all the permissions it has to delete first and then add the permissions that has to be added. r=johannh Differential Revision: https://phabricator.services.mozilla.com/D23952
browser/components/preferences/in-content/tests/browser.ini
browser/components/preferences/in-content/tests/browser_cookie_exceptions_addRemove.js
browser/components/preferences/permissions.js
--- a/browser/components/preferences/in-content/tests/browser.ini
+++ b/browser/components/preferences/in-content/tests/browser.ini
@@ -30,16 +30,17 @@ skip-if = (os == 'win' && (processor == 
 [browser_search_subdialogs_within_preferences_8.js]
 [browser_search_subdialogs_within_preferences_site_data.js]
 [browser_bug795764_cachedisabled.js]
 [browser_bug1018066_resetScrollPosition.js]
 [browser_bug1020245_openPreferences_to_paneContent.js]
 [browser_bug1184989_prevent_scrolling_when_preferences_flipped.js]
 support-files =
   browser_bug1184989_prevent_scrolling_when_preferences_flipped.xul
+[browser_cookie_exceptions_addRemove.js]
 [browser_cert_export.js]
 [browser_engines.js]
 [browser_change_app_handler.js]
 skip-if = os != "win" # Windows-specific handler application selection dialog
 [browser_checkspelling.js]
 [browser_cloud_storage.js]
 [browser_connection.js]
 [browser_connection_bug388287.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/preferences/in-content/tests/browser_cookie_exceptions_addRemove.js
@@ -0,0 +1,241 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+const PERMISSIONS_URL = "chrome://browser/content/preferences/permissions.xul";
+
+async function openCookiesDialog(doc) {
+  let cookieExceptionsButton = doc.getElementById("cookieExceptions");
+  ok(cookieExceptionsButton, "cookieExceptionsButton found");
+  let dialogPromise = promiseLoadSubDialog(PERMISSIONS_URL);
+  cookieExceptionsButton.click();
+  let dialog = await dialogPromise;
+  return dialog;
+}
+
+function checkCookiesDialog(dialog) {
+  ok(dialog, "dialog loaded");
+  let buttonIds = ["removePermission", "removeAllPermissions", "btnApplyChanges"];
+
+  for (let buttonId of buttonIds) {
+    let button = dialog.document.getElementById(buttonId);
+    ok(button, `${buttonId} found`);
+  }
+
+  let cancelButton =
+    dialog.document.getElementsByClassName("actionButtons")[1].children[0];
+
+  is(cancelButton.getAttribute("label"), "Cancel", "cancelButton found");
+}
+
+function addNewPermission(websiteAddress, dialog) {
+  let url = dialog.document.getElementById("url");
+  let buttonDialog = dialog.document.getElementById("btnBlock");
+  let permissionsBox = dialog.document.getElementById("permissionsBox");
+  let currentPermissions = permissionsBox.itemCount;
+
+  url.value = websiteAddress;
+  url.dispatchEvent(new Event("input", {bubbles: true}));
+  is(buttonDialog.hasAttribute("disabled"), false,
+     "When the user add an url the button should be clickable");
+  buttonDialog.click();
+
+  is(permissionsBox.itemCount, currentPermissions + 1,
+     "Website added in url should be in the list");
+}
+
+async function cleanList(dialog) {
+  let removeAllButton = dialog.document.getElementById("removeAllPermissions");
+  if (!removeAllButton.hasAttribute("disabled")) removeAllButton.click();
+}
+
+function addData(websites, dialog) {
+  for (let website of websites) {
+    addNewPermission(website, dialog);
+  }
+}
+
+function deletePermission(permission, dialog) {
+  let permissionsBox = dialog.document.getElementById("permissionsBox");
+  let elements = permissionsBox.getElementsByAttribute("origin", permission);
+  is(elements.length, 1, "It should find only one entry");
+  permissionsBox.selectItem(elements[0]);
+  let removePermissionButton = dialog.document.getElementById("removePermission");
+  is(removePermissionButton.hasAttribute("disabled"), false,
+     "The button should be clickable to remove selected item");
+  removePermissionButton.click();
+}
+
+function save(dialog) {
+  let saveButton = dialog.document.getElementById("btnApplyChanges");
+  saveButton.click();
+}
+
+function cancel(dialog) {
+  let cancelButton =
+    dialog.document.getElementsByClassName("actionButtons")[1].children[0];
+  is(cancelButton.getAttribute("label"), "Cancel", "cancelButton found");
+  cancelButton.click();
+}
+
+async function checkExpected(expected, doc) {
+  let dialog = await openCookiesDialog(doc);
+  let permissionsBox = dialog.document.getElementById("permissionsBox");
+
+  is(permissionsBox.itemCount, expected.length,
+     `There should be ${expected.length} elements in the list`);
+
+  for (let website of expected) {
+    let elements = permissionsBox.getElementsByAttribute("origin", website);
+    is(elements.length, 1, "It should find only one entry");
+  }
+  return dialog;
+}
+
+async function runTest(test, websites, doc) {
+  let dialog = await openCookiesDialog(doc);
+  checkCookiesDialog(dialog);
+
+  if (test.needPreviousData) {
+    addData(websites, dialog);
+    save(dialog);
+    dialog = await openCookiesDialog(doc);
+  }
+
+  for (let step of test.steps) {
+    switch (step) {
+      case "addNewPermission":
+        addNewPermission(test.newData, dialog);
+        break;
+      case "deletePermission":
+        deletePermission(test.newData, dialog);
+        break;
+      case "deleteAllPermission":
+        await cleanList(dialog);
+        break;
+      case "save":
+        save(dialog);
+        break;
+      case "cancel":
+        cancel(dialog);
+        break;
+      case "openPane":
+        dialog = await openCookiesDialog(doc);
+        break;
+      default:
+        // code block
+    }
+  }
+  dialog = await checkExpected(test.expected, doc);
+  await cleanList(dialog);
+  save(dialog);
+}
+
+add_task(async function checkPermissions() {
+  await openPreferencesViaOpenPreferencesAPI("panePrivacy", {leaveOpen: true});
+  let win = gBrowser.selectedBrowser.contentWindow;
+  let doc = win.document;
+  let websites = ["http://test1.com", "http://test2.com"];
+
+  let tests = [{
+    "needPreviousData": false,
+    "newData": "https://mytest.com",
+    "steps": ["addNewPermission", "save"],
+    "expected": ["https://mytest.com"], // when open the pane again it should find this in the list
+  },
+  {
+    "needPreviousData": false,
+    "newData": "https://mytest.com",
+    "steps": ["addNewPermission", "cancel"],
+    "expected": [],
+  },
+  {
+    "needPreviousData": false,
+    "newData": "https://mytest.com",
+    "steps": ["addNewPermission", "deletePermission", "save"],
+    "expected": [],
+  },
+  {
+    "needPreviousData": false,
+    "newData": "https://mytest.com",
+    "steps": ["addNewPermission", "deletePermission", "cancel"],
+    "expected": [],
+  },
+  {
+    "needPreviousData": false,
+    "newData": "https://mytest.com",
+    "steps": ["addNewPermission", "save", "openPane", "deletePermission", "save"],
+    "expected": [],
+  },
+  {
+    "needPreviousData": false,
+    "newData": "https://mytest.com",
+    "steps": ["addNewPermission", "save", "openPane", "deletePermission", "cancel"],
+    "expected": ["https://mytest.com"],
+  },
+  {
+    "needPreviousData": false,
+    "newData": "https://mytest.com",
+    "steps": ["addNewPermission", "deleteAllPermission", "save"],
+    "expected": [],
+  },
+  {
+    "needPreviousData": false,
+    "newData": "https://mytest.com",
+    "steps": ["addNewPermission", "deleteAllPermission", "cancel"],
+    "expected": [],
+  },
+  {
+    "needPreviousData": false,
+    "newData": "https://mytest.com",
+    "steps": ["addNewPermission", "save", "openPane", "deleteAllPermission", "save"],
+    "expected": [],
+  },
+  {
+    "needPreviousData": false,
+    "newData": "https://mytest.com",
+    "steps": ["addNewPermission", "save", "openPane", "deleteAllPermission", "cancel"],
+    "expected": ["https://mytest.com"],
+  },
+  {
+    "needPreviousData": true,
+    "newData": "https://mytest.com",
+    "steps": ["deleteAllPermission", "save"],
+    "expected": [],
+  },
+  {
+    "needPreviousData": true,
+    "newData": "https://mytest.com",
+    "steps": ["deleteAllPermission", "cancel"],
+    "expected": websites,
+  },
+  {
+    "needPreviousData": true,
+    "newData": "https://mytest.com",
+    "steps": ["addNewPermission", "save"],
+    "expected": (function() {
+      let result = websites.slice();
+      result.push("https://mytest.com");
+      return result;
+    }()),
+  },
+  {
+    "needPreviousData": true,
+    "newData": "https://mytest.com",
+    "steps": ["addNewPermission", "cancel"],
+    "expected": websites,
+  },
+  {
+    "needPreviousData": false,
+    "newData": "https://mytest.com",
+    "steps": ["addNewPermission", "save", "openPane", "deleteAllPermission", "addNewPermission",  "save"],
+    "expected": ["https://mytest.com"],
+  }];
+
+  for (let test of tests) {
+    await runTest(test, websites, doc);
+  }
+
+  gBrowser.removeCurrentTab();
+});
--- a/browser/components/preferences/permissions.js
+++ b/browser/components/preferences/permissions.js
@@ -370,24 +370,24 @@ var gPermissionManager = {
   },
 
   onApplyChanges() {
     // Stop observing permission changes since we are about
     // to write out the pending adds/deletes and don't need
     // to update the UI
     this.uninit();
 
+    for (let p of this._permissionsToDelete.values()) {
+      Services.perms.removeFromPrincipal(p.principal, p.type);
+    }
+
     for (let p of this._permissionsToAdd.values()) {
       Services.perms.addFromPrincipal(p.principal, p.type, p.capability);
     }
 
-    for (let p of this._permissionsToDelete.values()) {
-      Services.perms.removeFromPrincipal(p.principal, p.type);
-    }
-
     window.close();
   },
 
   buildPermissionsList(sortCol) {
     // Clear old entries.
     let oldItems = this._list.querySelectorAll("richlistitem");
     for (let item of oldItems) {
       item.remove();