1237911 - permission creation needs to use default user context id

Status: RESOLVED WONTFIX

(Core :: Permission Manager, defect)

Description

[Dave Huseby [:huseby]]

in the file browser/components/preferences/permissions.js in the function addPermission, there are a pair of calls to createCodebasePrincipal that need to be changed to use origin attributes with the default user context id.  the solution is to just call the ChromeUtils function for creating a default user context origin attribute from the origin and then use that.

Comment 1

[Dave Huseby [:huseby]]

Attachment: Bug_1237911.patch

this just makes sure that the principal used when adding a permission uses the default user context id.

Comment 2

[Dave Huseby [:huseby]]

After groking this code, it's really only testing to see if the URI is a valid canonical origin.  The default {} origin attribute passed into the createCodebasePrincipal call is correct in this case since the resulting principal won't be used for anything other than checking the URI.  Closing as WON'T FIX.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Really? The code at [1] does a full principal.equals comparison, which will compare OriginAttributes.

[1] http://mxr.mozilla.org/mozilla-central/source/browser/components/preferences/permissions.js#120

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Reopening based on comment 3. Please re-close if I'm wrong.

Comment 5

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

(In reply to Jonas Sicking (:sicking) from comment #3)
> Really? The code at [1] does a full principal.equals comparison, which will
> compare OriginAttributes.
> 
> [1]
> http://mxr.mozilla.org/mozilla-central/source/browser/components/preferences/
> permissions.js#120

And the this._permissions[i] is added from [2], 
whose principal is created from Comment 0

So I also think in this file we always use DEFAULT_USER_CONTEXT_ID as OriginAttributes.

[2] http://mxr.mozilla.org/mozilla-central/source/browser/components/preferences/permissions.js#432

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Hmm.. Yeah. I guess that since permissions always use the default user context this is fine. It'll mean that all other parts of the OriginAttribute (other than the userContextId) might still be wrong.

However to get them right I suspect that we'll need more UI. So with the current UI I think this code is likely fine as-is.