Closed
Bug 1180871
Opened 9 years ago
Closed 9 years ago
Since Bug 1170200 the API for nsIPermissionManager::Remove() accepts a URI instead of a string.
Categories
(SeaMonkey :: Passwords & Permissions, defect)
Tracking
(seamonkey2.39+ fixed, seamonkey2.40 affected, seamonkey2.41 fixed)
RESOLVED
FIXED
seamonkey2.41
People
(Reporter: philip.chee, Assigned: neil)
References
Details
User Story
Change all callers in /suite/ of nsIPermissionManager::Remove() to use a URI instead of a string. Also change call callers in /suite/ of nsIPermission::GetHost() to use the principal.
Attachments
(2 files)
15.11 KB,
patch
|
philip.chee
:
review+
iannbugzilla
:
feedback+
iannbugzilla
:
approval-comm-aurora+
iannbugzilla
:
approval-comm-beta+
iannbugzilla
:
approval-comm-release+
|
Details | Diff | Splinter Review |
8.15 KB,
patch
|
ewong
:
approval-comm-release+
|
Details | Diff | Splinter Review |
No description provided.
Reporter | ||
Updated•9 years ago
|
User Story: (updated)
Reporter | ||
Updated•9 years ago
|
Assignee | ||
Updated•9 years ago
|
User Story: (updated)
Assignee | ||
Updated•9 years ago
|
User Story: (updated)
Assignee | ||
Comment 1•9 years ago
|
||
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.
Attachment #8672112 -
Flags: review?(philip.chee)
Attachment #8672112 -
Flags: review?(iann_bugzilla)
Assignee | ||
Updated•9 years ago
|
status-seamonkey2.39:
--- → affected
status-seamonkey2.40:
--- → affected
status-seamonkey2.41:
--- → affected
tracking-seamonkey2.39:
--- → ?
Reporter | ||
Comment 2•9 years ago
|
||
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
Attachment #8672112 -
Flags: review?(philip.chee) → review+
Reporter | ||
Comment 3•9 years ago
|
||
> @@ -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
Reporter | ||
Updated•9 years ago
|
Assignee: nobody → neil
Status: NEW → ASSIGNED
Comment 4•9 years ago
|
||
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 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.
Attachment #8672112 -
Flags: review?(iann_bugzilla) → feedback+
Assignee | ||
Comment 6•9 years ago
|
||
(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.
Assignee | ||
Comment 7•9 years ago
|
||
Pushed comm-central changeset 553176734705.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.41
Assignee | ||
Comment 8•9 years ago
|
||
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
Attachment #8672112 -
Flags: approval-comm-release?
Attachment #8672112 -
Flags: approval-comm-beta?
Attachment #8672112 -
Flags: approval-comm-aurora?
Attachment #8672112 -
Flags: approval-comm-release?
Attachment #8672112 -
Flags: approval-comm-release+
Attachment #8672112 -
Flags: approval-comm-beta?
Attachment #8672112 -
Flags: approval-comm-beta+
Attachment #8672112 -
Flags: approval-comm-aurora?
Attachment #8672112 -
Flags: approval-comm-aurora+
Comment 9•9 years ago
|
||
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•9 years ago
|
||
[Triage Comment] This was for the c-r checkin. (Removed the dataman code).
Attachment #8682396 -
Flags: approval-comm-release+
Updated•9 years ago
|
Comment 11•9 years ago
|
||
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
You need to log in
before you can comment on or make changes to this bug.
Description
•