permissions.js onRadioClick tries to pass an nsIURI to nsIPermissionManager::remove, needs to be the host string instead

RESOLVED FIXED in Firefox 24

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: dao, Assigned: dao)

Tracking

({regression})

Trunk
Firefox 25
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox23 unaffected, firefox24 fixed)

Details

Attachments

(1 attachment)

>--- 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");
>-  }  
> }
Posted patch patchSplinter Review
Assignee: nobody → dao
Status: NEW → ASSIGNED
Attachment #770695 - Flags: review?(benjamin)
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
Blocks: 889835
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+
(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
https://hg.mozilla.org/mozilla-central/rev/3b63ac78d593
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 25
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?
Attachment #770695 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.