1180871 - Since Bug 1170200 the API for nsIPermissionManager::Remove() accepts a URI instead of a string.

Status: RESOLVED FIXED

(SeaMonkey :: Passwords & Permissions, defect)

Comment 1

[[e-mail]]

Attachment: Proposed patch

This may fix some other bugs such as the Data Manager one, but right now these are just mechanical conversions so I may have some typos or omissions.

Comment 2

[Philip Chee]

Comment on attachment 8672112 [details] [diff] [review]
Proposed patch

> +++ b/suite/browser/pageinfo/pageInfo.js
 In Firefox I see:

 function onLoadPermission()
 .....
-    hostText.value = gPermURI.host;
+    hostText.value = gPermURI.prePath;

https://bugzilla.mozilla.org/show_bug.cgi?id=1173523#c24

> +++ b/suite/common/dataman/dataman.xml

> -              Services.perms.remove(this.host, this.type);
> +              Services.perms.remove(Services.io.newURI("http://" + this.host, null, null), this.type);
What happens to https?

> +++ b/suite/common/pref/pref-offlineapps.js

> -    let usage = _getOfflineAppUsage(perm.host);
> +    let usage = _getOfflineAppUsage(perm);
>      let row = document.createElement("listitem");
> -    row.setAttribute("host", perm.host);
> +    row.setAttribute("host", perm.principal.URI.host);

That reminds me, we should port Bug 1173523 - Part 7: Update advanced.js to use new API for nsIPermission
https://hg.mozilla.org/mozilla-central/rev/88daaf531d7c#l1.40
https://hg.mozilla.org/mozilla-central/rev/88daaf531d7c#l1.99

Comment 3

[Philip Chee]

> @@ -1225,21 +1225,21 @@ var gPerms = {
>      this.addHost = document.getElementById("permHost");
>      this.addType = document.getElementById("permType");
>      this.addButton = document.getElementById("permAddButton");
 
>      let enumerator = Services.perms.enumerator;
>      while (enumerator.hasMoreElements()) {
>        let nextPermission = enumerator.getNext();
>        nextPermission = nextPermission.QueryInterface(Components.interfaces.nsIPermission);
> -      let rawHost = nextPermission.host.replace(/^\./, "");
> +      let rawHost = nextPermission.principal.URI.host.replace(/^\./, "");
>        if (gDomains.hostMatchesSelected(rawHost)) {
>          let permElem = document.createElement("richlistitem");
>          permElem.setAttribute("type", nextPermission.type);


> -        permElem.setAttribute("host", nextPermission.host);
> +        permElem.setAttribute("host", nextPermission.principal.URI.host);
Frank-Rainer Grahl has this change in his patch over at Bug 1188348

Comment 4

[Frank-Rainer Grahl (:frg)]

Neil,

could you look over my patch in Bug 1188348 and integrate it in yours. It also covers the old cookieviewer. 

I did the remove in dataviewer.js a little differently. The removeFromPrincipal seemed to be looking a little better in my eyes.

Your missed one host assignment in yours:  

>> -        permElem.setAttribute("host", nextPermission.host);
>> +        permElem.setAttribute("host", nextPermission.principal.URI.host);

FRG

Comment 5

[Ian Neal]

Comment on attachment 8672112 [details] [diff] [review]
Proposed patch

>+++ b/suite/common/dataman/dataman.js
>@@ -1225,17 +1225,17 @@ var gPerms = {
>     while (enumerator.hasMoreElements()) {
>       let nextPermission = enumerator.getNext();
>       nextPermission = nextPermission.QueryInterface(Components.interfaces.nsIPermission);
>-      let rawHost = nextPermission.host.replace(/^\./, "");
>+      let rawHost = nextPermission.principal.URI.host.replace(/^\./, "");
>       if (gDomains.hostMatchesSelected(rawHost)) {
>         let permElem = document.createElement("richlistitem");
>         permElem.setAttribute("type", nextPermission.type);
>         permElem.setAttribute("host", nextPermission.host);
As mentioned by FRG, this will need fixing.

As mentioned by Ratty:
That reminds me, we should port Bug 1173523 - Part 7: Update advanced.js to use new API for nsIPermission
https://hg.mozilla.org/mozilla-central/rev/88daaf531d7c#l1.40
https://hg.mozilla.org/mozilla-central/rev/88daaf531d7c#l1.99

f+ as I would like to see the new patch.

Comment 6

[[e-mail]]

(In reply to Ian Neal from comment #5)
> As mentioned by FRG, this will need fixing.
I won't land the datamanager changes; neither set is ready anyway.

> As mentioned by Ratty:
> That reminds me, we should port Bug 1173523 - Part 7: Update advanced.js to
> use new API for nsIPermission
We need to implement the nsILoadContextInfo changes first anyway. I think it would be better to land and uplift this work done so far, and then fix the other bustage.

Comment 7

[[e-mail]]

Pushed comm-central changeset 553176734705.

Comment 8

[[e-mail]]

Comment on attachment 8672112 [details] [diff] [review]
Proposed patch

[Approval Request Comment]
Regression caused by (bug #): 1170200
User impact if declined: Various functions broken
String changes made by this patch: None

Comment 9

[Edmund Wong (:ewong)]

From the recent merge, c-a is not needed since it's been uplifted from
c-c.  So it's only c-b and c-r that needs the patch.

Comment 10

[Edmund Wong (:ewong)]

Attachment: patch as checked in to c-r

[Triage Comment]

This was for the c-r checkin. (Removed the dataman code).

Comment 11

[Edmund Wong (:ewong)]

Comment on attachment 8682396 [details] [diff] [review]
patch as checked in to c-r

Pushed to comm-release:
https://hg.mozilla.org/releases/comm-release/rev/34ba48147772