Closed Bug 1240853 Opened 8 years ago Closed 8 years ago

fixup createCodebasePrincipalFromOrigin to use origin attributes in translation permission handling

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: huseby, Assigned: timhuang)

References

Details

(Whiteboard: [userContextId][OA])

Attachments

(1 obsolete file)

in the file browsers/components/preferences/translation.js there are two callsites that use the createCodebasePrincipalFromOrigin:

> 213   onSiteDeleted: function() {
> 214     let removedSites = this._siteTree.getSelectedItems();
> 215     for (let origin of removedSites) {
> 216       let principal = Services.scriptSecurityManager.createCodebasePrincipalFromOrigin(origin);
> 217       Services.perms.removeFromPrincipal(principal, kPermissionType);
> 218     }
> 219   },

and

> 221   onAllSitesDeleted: function() {
> 222     if (this._siteTree.isEmpty)
> 223       return;
> 224
> 225     let removedSites = this._sites.splice(0, this._sites.length);
> 226     this._siteTree.boxObject.rowCountChanged(0, -removedSites.length);
> 227
> 228     for (let origin of removedSites) {
> 229       let principal = Services.scriptSecurityManager.createCodebasePrincipalFromOrigin(origin);
> 230       Services.perms.removeFromPrincipal(principal, kPermissionType);
> 231     }
> 232
> 233     this.onSiteSelected();
> 234   },

this removes the translate permission from the principal.  this creates a principal from an origin, then internally the origin is pulled back out of the principal when creating the premission object that needs to be removed.  since we are not isolating on user context id for permissions, the fix is, create an origin attributes from the origin (e.g. populateFromOrigin), then create a GlobalContextOriginAttributes from that to force user context id to 0, then call createCodebasePrincipal with the origin and origin attributes.
Assignee: huseby → tihuang
Dave, Could you take a look on this patch? And, who should I ask for reviewing this bug?
Attachment #8734222 - Flags: feedback?(huseby)
I don't think this patch is necessary anymore.  Consider the code:

> Services.perms.removeFromPrincipal(principal, kPermissionType);

That calls the function nsPermissionManager::RemoveFromPrincipal (http://mzl.la/1UL9rRO) which calls nsPermissionManager::AddInternal() (http://mzl.la/1PtZobU) That function uses an instance of the PermissionKey class (http://mzl.la/1PtZwrM) to get the hash string used to look up the permission.  The hash string is based solely on the origin string and *NOT* the origin attributes.

Therefore, it makes no sense to worry about putting the correct origin attributes into the principal that gets passed to the .removeFromPrincipal() function call.  The origin attributes aren't used in there at all.

This bug should be flagged as RESOLVED, WON'T FIX.
Comment on attachment 8734222 [details] [diff] [review]
Fixup createCodebasePrincipalFromOrigin to use origin attributes in translation permission handling.

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

This patch isn't needed IMO.
Attachment #8734222 - Flags: feedback?(huseby) → feedback-
Comment on attachment 8734222 [details] [diff] [review]
Fixup createCodebasePrincipalFromOrigin to use origin attributes in translation permission handling.

According comment 3, mark as obsolete.
Attachment #8734222 - Attachment is obsolete: true
According to comment 3, change the flag into RESOLVED, WON'T FIX.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → WONTFIX
Whiteboard: [userContextId] → [userContextId][OA]
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: