Data Manager currently has no way to add permissions, only to change or reset/delete them. We should add a possibility to add them, so we can get e.g. the functionality of "Allow popups from this website" implemented correctly.

OK, here's the patch that allows this, adding a box at the bottom of the permissions panel that has an "Add" button, giving way for a host name entry box and a permission type selection, and adding a permission entry to the list when clicked again in that state. For any domain that has no permissions panel yet, the generic "*" domain entry enables that panel to allow adding permissions. The code for loading views gets functionality for opening that add box and prefilling values, ending up in basically the same state as what the "Allow popups from this website" menu entry does right now in SeaMonkey (that's also where this code will be used right away). Of course, a number of tests are added to make sure things work as expected.

This patch depends on the bug 588418 work, so marking that.

Here's the updated patch for the view call separator change in bug 588418.

(In reply to comment #3) > Created attachment 493544 [details] [diff] [review] > v1.1: switch view call separator to | > > Here's the updated patch for the view call separator change in bug 588418. Bitrotted :( applying bug588417-v1.1.diff patching file suite/common/dataman/dataman.js Hunk #2 FAILED at 94 Hunk #3 FAILED at 244 Hunk #4 FAILED at 355 3 out of 7 hunks FAILED -- saving rejects to file suite/common/dataman/dataman.js.rej patching file suite/common/dataman/tests/browser_dataman_callviews.js Hunk #2 FAILED at 64 1 out of 2 hunks FAILED -- saving rejects to file suite/common/dataman/tests/browser_dataman_callviews.js.rej patch failed, unable to continue (try -v) patch failed, rejects left in working dir errors during apply, please fix and refresh bug588417-v1.1.diff

(In reply to comment #4) > Bitrotted :( I don't think so. But it could very easily be needing the patch from bug 613795 to apply cleanly, as it's on top of that in my local system.

Marking dependency because of how my patches are structured.

In bug 588418, I added a few more test cases and that would have needed changes here anyhow, so I switched to a somewhat saner testing infrastructure as well (at first, I hadn't expected that this one would cover that many cases). This patch only contains updates for those changes. The new patch is still on top of bug 613795, btw.

(In reply to comment #7) > Created attachment 496221 [details] [diff] [review] > v1.2: update for test changes > > In bug 588418, I added a few more test cases and that would have needed changes > here anyhow, so I switched to a somewhat saner testing infrastructure as well > (at first, I hadn't expected that this one would cover that many cases). This > patch only contains updates for those changes. > > The new patch is still on top of bug 613795, btw. From first set of testing: Go to permissions tab, select Add, look at type list, but don't actually add anything. Go to another domain and permissions tab again, select add, look at type list, it has the type list repeated. Keep going to different domains, permissions tab, click add, and look at type list, it keeps adding to the list.

Comment on attachment 496221 [details] [diff] [review] v1.2: update for test changes >+++ b/suite/common/dataman/dataman.js >@@ -361,23 +364,50 @@ var gDomains = { > let viewdomain = gDomains.getDomainFromHost(host); > for (let i = 0; i < gDomains.displayedDomains.length; i++) { > if (gDomains.displayedDomains[i].title == viewdomain) { > gDomains.tree.view.selection.select(i); > gDomains.tree.treeBoxObject.ensureRowIsVisible(i); > break; > } > } >+ if (gDomains.selectedDomain.title != viewdomain) { >+ gDomains.tree.view.selection.select(0); >+ gDomains.tree.treeBoxObject.ensureRowIsVisible(0); >+ } Here can we do something like: let selection = 0; for (let i = 0; i < gDomains.displayedDomains.length; i++) { if (gDomains.displayedDomains[i].title == viewdomain) { selection = i; break; } } then if we're adding a permission then check the flag to see if a permissions tab is enabled, if not set selection = 0 then do the { gDomains.tree.view.selection.select(selection); gDomains.tree.treeBoxObject.ensureRowIsVisible(selection); } yield setTimeout(nextStep, 0); etc

If you select a domain with a permission and then change the permission to "Use Default", move to another domain and back again, the permission tab stays enabled even though there no permissions left until you close and reopen datamanager. We should be disabling the tab and, if appropriate, moving to any tab still enabled or to the next domain in the list or "*". I've also noticed when you remove the last cookie from a domain, it removes the domain but leaves you with an empty cookie tab and no domain focussed, I'd expect it to move to either the next domain in the list or to "*".

Comment on attachment 496221 [details] [diff] [review] v1.2: update for test changes r- due to issue in comment 8, as for comment 9 I am okay with that either being done here or spun off into another bug.

This should address both comment #8 and comment #9 - for the latter, I pulled a check for the add param into the domain loading, which is not completely nice, but at least it makes all domain selection be in a single place, which makes it worth that, IMHO.

Comment on attachment 497568 [details] [diff] [review] v1.2.99: clear list on add, loading optimization This needs one more small change, coming in a minute.

I only remembered to actually run the automated tests once I already had submitted the new patch, and they turned up the case of a not-yet-existing domain not having set that domain object yet. This patch covers that as well.

Comment on attachment 497573 [details] [diff] [review] v1.3: clear list on add, loading optimization >+++ b/suite/common/dataman/dataman.js How about setting: let permAdd = (gDataman.viewToLoad[1] && gDataman.viewToLoad[1] == "permissions" && gDataman.viewToLoad[2] && gDataman.viewToLoad[2] == "add"); and then using permAdd in the two if statements? if (permAdd && selectIdx != 0 && (!(viewdomain in gDomains.domainObjects) || !gDomains.domainObjects[viewdomain].hasPermissions)) { selectIdx = 0; // Force * domain as we have a perm panel there. } if (permAdd) { gDataman.debugMsg("Adding permission"); r=me with that addressed/fixed

Applied those changes and pushed as http://hg.mozilla.org/comm-central/rev/76215aaa1e70