The special powers api needs the ability to check Permissions as well as add and remove permissions. The below is *one* proposal for how to give the tester this ability. SpecialPowers.hasPermission("alarms", document); which returns true or false. SpecialPowers.getPermission("alarms", document); which returns the enumerated value from the permission Manager which is up to the test taker to compare against. enum { UNKNOWN_ACTION = 0U, ALLOW_ACTION = 1U, DENY_ACTION = 2U, PROMPT_ACTION = 3U, }; The 95% usage case I'd imagine is the first instance, for people who want finer grained control they would want to use the second api.

Generally need some feedback on if this approach seems the correct one.

This sounds like you are describing a lot of the functionality provided in the navigator.mozPermissionSettings API.

(In reply to Jason Smith [:jsmith] from comment #2) > This sounds like you are describing a lot of the functionality provided in > the navigator.mozPermissionSettings API. Both SpecialPowers.xxxPermission and mozPermissionSettings use the same API (nsIPermissionManager), but the former works without the "permissions" permissions. (In reply to dclarke@mozilla.com [:onecyrenus] from comment #1) > Generally need some feedback on if this approach seems the correct one. Yes, I think that's the right approach. A lot of SpecialPowers calls are direct copies of their API's, so we might want to use SpecialPowers.testPermission, ala https://developer.mozilla.org/en-US/docs/XPCOM_Interface_Reference/nsIPermissionManager#testPermission%28%29.

Yes I saw that, i was just thinking semantically SpecialPowers.hasPermission("alarms",document) makes a lot more sense than SpecialPowers.testPermission("alarms", document)... The former seems more obvious to return a boolean true / false.. SpecialPowers.testPermission("alarms", PROMPT_ACTION, document) <-- this makes more sense to me than the former.

If I read the interface reference correctly, the API testPermission returns the permission value rather than a boolean. I wouldn't have two functions with the same name and purpose with different signatures--that would be confusing. So if we're going to align, seems like it'd be more like: PROMPT_ACTION == SpecialPowers.testPermission(document, "alarms")

I think there is a bit of confusion there what i'm saying is PERMISSION_STATUS = SpecialPowers.testPermission(document, "alarms") requires you to have a list of permission status'es stored somewhere in order to do the equality. The 90% case is that you just want to check if you have a permission or not. I was merely pointing out that an api with the name test_Permission means you are testing against a known value, whereas get_Permission would be easier to understand when reading the api. test_Permission and passing in the PROMPT_ACTION is also acceptable, but having a function called test_Permission which really returns an encoded value seems semantically incorrect... NS_IMETHODIMP nsPermissionManager::TestExactPermission(nsIURI *aURI, const char *aType, uint32_t *aPermission)

This makes sense, and I'm concerned about (convenient) access to the permissions constants as well. More saying don't align the names unless the functions work the same--basic code comprehension thing. I'd go with hasPermissions for the simple boolean one precisely because the name is different.

er, hasPermission() that is.

Example api usage: ok(SpecialPowers.hasPermission("alarms", document), "yeahh we have permission"); ok(SpecialPowers.testPermission("alarms", SpecialPowers.Ci.nsIPermissionManager.ALLOW_ACTION, document),"Allow Action should return true");

Comment on attachment 719978 [details] [diff] [review] added testPermission / hasPermission api's Review of attachment 719978 [details] [diff] [review]: ----------------------------------------------------------------- r+ with these changes. ::: testing/specialpowers/content/SpecialPowersObserverAPI.js @@ +228,4 @@ > > let secMan = Services.scriptSecurityManager; > let principal = secMan.getAppCodebasePrincipal(this._getURI(msg.url), msg.appId, msg.isInBrowserElement); > + let perm = null; It's a little odd to declare a variable here just so we can use it in two different spots in the switch below. I think it would be better to omit this and just use two differently-named vars in the switch, like hasPerm and testPerm. ::: testing/specialpowers/content/specialpowersAPI.js @@ +1268,5 @@ > + hasPermission: function (type, arg) { > + let [url, appId, isInBrowserElement] = this._getInfoFromPermissionArg(arg); > + > + var msg = { > + 'op': "has", let's using single quotes for consistency @@ +1276,5 @@ > + 'isInBrowserElement': isInBrowserElement > + }; > + > + return this._sendSyncMessage('SPPermissionManager', msg)[0]; > + nit: extra spaces here @@ +1282,5 @@ > + testPermission: function (type, value, arg) { > + let [url, appId, isInBrowserElement] = this._getInfoFromPermissionArg(arg); > + > + var msg = { > + 'op': "test", let's use single quotes for consistency

Updated patch to include comments, should be ready for checkin.

Comment on attachment 720825 [details] [diff] [review] Updated: added testPermission / hasPermission api's Review of attachment 720825 [details] [diff] [review]: ----------------------------------------------------------------- Thanks!

This needs to be nommed so we can resolve the dependency 843893.

https://hg.mozilla.org/integration/mozilla-inbound/rev/22288118672b David, can you please make sure you have hg configured so that your patches contain all the necessary checkin metadata by default? Thanks! https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

Ryan, got it.

This has to get checked into the v1-train, as i believe b2g tests run on the v1-train, and not the trunk .

Comment on attachment 720825 [details] [diff] [review] Updated: added testPermission / hasPermission api's NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] Bug caused by (feature/regressing bug #): 843893 User impact if declined: mochitest run cannot complete successfully Testing completed: tested running mochitests Risk to taking this patch (and alternatives if risky): 0 String or UUID changes made by this patch:

Waiting on review of bug 843893 prior to considering this bug for approval.

(clearing tef? to get this out of the nom list until the review of bug 843893 happens)