Closed Bug 909012 Opened 9 years ago Closed 9 years ago

pushPermissions doesn't have 'remove' option

Categories

(Testing :: Mochitest, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla26

People

(Reporter: martijn.martijn, Assigned: martijn.martijn)

References

()

Details

Attachments

(1 file, 1 obsolete file)

Attached patch pushPermissionsremove.diff (obsolete) — Splinter Review
pushPrefEnv has the 'clear' option to remove prefs, but pushPermissions doesn't have the 'remove' option, which would be similar to removePermission.

So something like this would remove the pREMOVE permission:
SpecialPowers.pushPermissions([{'type': 'pREMOVE', 'remove': true, 'context': document}], function(){});
This would be equivalent to:
SpecialPowers.pushPermissions([{'type': 'pREMOVE', 'allow': Ci.nsIPermissionManager.UNKNOWN_ACTION, 'context': document}], function(){});

So this 'remove' option doesn't really add any functionality, but I think it's just a bit more logical.

I also added some tests that exercise the 'allow: Ci.nsIPermissionManager.ALLOW_ACTION/Ci.nsIPermissionManager.DENY_ACTION
Attachment #795042 - Flags: review?(jmaher)
Comment on attachment 795042 [details] [diff] [review]
pushPermissionsremove.diff

Review of attachment 795042 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/specialpowers/content/specialpowersAPI.js
@@ +560,5 @@
>       inPermissions is an array of objects where each object has a type, action, context, ex:
>       [{'type': 'SystemXHR', 'allow': 1, 'context': document}, 
>        {'type': 'SystemXHR', 'allow': Ci.nsIPermissionManager.PROMPT_ACTION, 'context': document}]
>  
> +    allow is a boolean and can be true/false or ALLOW_ACTION/DENY_ACTION/PROMPT_ACTION/UNKNOWN_ACTION

this comment is incorrect now, it isn't boolean anymore.

Allow can be a boolean value of true/false or ALLOW_ACTION/DENY_ACTION/PROMPT_ACTION/UNKNOWN_ACTION
Attachment #795042 - Flags: review?(jmaher) → review+
(In reply to Joel Maher (:jmaher) from comment #1)
> this comment is incorrect now, it isn't boolean anymore.
> 
> Allow can be a boolean value of true/false or
> ALLOW_ACTION/DENY_ACTION/PROMPT_ACTION/UNKNOWN_ACTION

Yeah, thanks, that comment was also wrong before this patch, the ALLOW_ACTION/DENY_ACTION/PROMPT_ACTION/UNKNOWN_ACTION values were accepted for some time already.
I'll fix it.
This is on top of the patch from bug 907995 (not that it should matter, afaik).
Attachment #795042 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/bfb91cdacc9a
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.