Closed
Bug 889770
Opened 11 years ago
Closed 11 years ago
permissions.js onRadioClick tries to pass an nsIURI to nsIPermissionManager::remove, needs to be the host string instead
Categories
(Firefox :: Page Info Window, defect)
Firefox
Page Info Window
Tracking
()
RESOLVED
FIXED
Firefox 25
| Tracking | Status | |
|---|---|---|
| firefox23 | --- | unaffected |
| firefox24 | --- | fixed |
People
(Reporter: dao, Assigned: dao)
References
Details
(Keywords: regression)
Attachments
(1 file)
|
(deleted),
patch
|
benjamin
:
review+
bajaj
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
>--- a/browser/base/content/pageinfo/permissions.js
>+++ b/browser/base/content/pageinfo/permissions.js
>@@ -183,24 +183,25 @@ function onPluginRadioClick(aEvent) {
> function onRadioClick(aPartId)
> {
> var permissionManager = Components.classes[PERMISSION_CONTRACTID]
> .getService(nsIPermissionManager);
>
> var radioGroup = document.getElementById(aPartId + "RadioGroup");
> var id = radioGroup.selectedItem.id;
> var permission = id.split('#')[1];
>- permissionManager.add(gPermURI, aPartId, permission);
>+ if (permission == UNKNOWN) {
>+ permissionManager.remove(gPermURI, aPartId);
>+ } else {
>+ permissionManager.add(gPermURI, aPartId, permission);
>+ }
> if (aPartId == "indexedDB" &&
> (permission == ALLOW || permission == BLOCK)) {
> permissionManager.remove(gPermURI.host, "indexedDB-unlimited");
> }
>- if (aPartId == "fullscreen" && permission == UNKNOWN) {
>- permissionManager.remove(gPermURI.host, "fullscreen");
>- }
> }
|
Assignee | |
Comment 1•11 years ago
|
||
|
|
Assignee | |
Updated•11 years ago
|
status-firefox23:
--- → unaffected
status-firefox24:
--- → affected
|
|
Assignee | |
Updated•11 years ago
|
Summary: permissions.js onRadioClick tries to pass an nsIURI to nsIPermissionManager::remove, needs to be a string instead → permissions.js onRadioClick tries to pass an nsIURI to nsIPermissionManager::remove, needs to be the host string instead
|
||
Comment 2•11 years ago
|
||
Comment on attachment 770695 [details] [diff] [review]
patch
Hrm. Does this UI work when the page is a data: URI? See bug 887773. It seems like this entire file is using gPermURI when it perhaps really ought to be using a principal. If it were using a principal, then perms.addFromPrincipal(UNKNOWN_ACTION) would be better here.
But since that's probably beyond the scope of what we can do here, this is ok.
Attachment #770695 -
Flags: review?(benjamin) → review+
|
|
Assignee | |
Comment 3•11 years ago
|
||
(In reply to Benjamin Smedberg [:bsmedberg] from comment #2)
> Comment on attachment 770695 [details] [diff] [review]
> patch
>
> Hrm. Does this UI work when the page is a data: URI? See bug 887773.
The permissions tab is only shown for http(s) URIs. We can probably change that, but as you say it's beyond this bug's scope.
Keywords: checkin-needed
|
|
Assignee | |
Comment 4•11 years ago
|
||
Keywords: checkin-needed
|
||
Comment 5•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 25
|
|
Assignee | |
Comment 6•11 years ago
|
||
Comment on attachment 770695 [details] [diff] [review]
patch
[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 875454
User impact if declined: permission isn't removed when it should be when clicking some radio elements in the page info window's permissions tab
Testing completed (on m-c, etc.): landed on m-c
Risk to taking this patch (and alternatives if risky): none
String or IDL/UUID changes made by this patch: none
Attachment #770695 -
Flags: approval-mozilla-aurora?
|
||
Updated•11 years ago
|
Attachment #770695 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
|
|
Assignee | |
Comment 7•11 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•