Closed
Bug 905397
Opened 8 years ago
Closed 8 years ago
Handle installed permissions
Categories
(Firefox Graveyard :: Webapp Runtime, defect, P1)
Firefox Graveyard
Webapp Runtime
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 26
People
(Reporter: marco, Assigned: marco)
References
Details
(Whiteboard: DesktopWebRT2)
Attachments
(1 file, 2 obsolete files)
7.39 KB,
patch
|
marco
:
review+
|
Details | Diff | Splinter Review |
Needed after bug 892837 to correctly handle installed permissions.
Attachment #790457 -
Flags: review?(myk)
Assignee | ||
Updated•8 years ago
|
Attachment #790457 -
Attachment is obsolete: true
Attachment #790457 -
Flags: review?(myk)
Assignee | ||
Comment 1•8 years ago
|
||
Comment 2•8 years ago
|
||
Marco: if this blocks bug 892837, then we can't fix that bug until we fix this one. But that bug is currently checkin-needed, while this one isn't ready for commission. Are they really dependent? Or perhaps the dependency relationship goes the other way?
Assignee | ||
Comment 3•8 years ago
|
||
(In reply to Myk Melez [:myk] [@mykmelez] from comment #2) > Marco: if this blocks bug 892837, then we can't fix that bug until we fix > this one. But that bug is currently checkin-needed, while this one isn't > ready for commission. Are they really dependent? Or perhaps the dependency > relationship goes the other way? Yes we can land them separately. This patch completes the permissions support, this is why it's blocking that bug, but actually the patches are "parallel" :)
Updated•8 years ago
|
Priority: -- → P1
Whiteboard: DesktopWebRT2
Comment 4•8 years ago
|
||
Comment on attachment 790497 [details] [diff] [review] Patch Review of attachment 790497 [details] [diff] [review]: ----------------------------------------------------------------- ::: webapprt/ContentPermission.js @@ +26,5 @@ > + .rootTreeItem > + .QueryInterface(Ci.nsIInterfaceRequestor) > + .getInterface(Ci.nsIDOMWindow) > + .QueryInterface(Ci.nsIDOMChromeWindow); > + return chromeWin; Nit: var -> let, but since all you do is return it, you might as well simply return the value instead of assigning it to a variable first. @@ +33,4 @@ > prompt: function(request) { > + // Reuse any remembered permission preferences > + let result = Services.perms. > + testExactPermissionFromPrincipal(request.principal, request.type); Nit: this line exceeds 80 columns and should be wrapped differently, f.e.: let result = Services.perms.testExactPermissionFromPrincipal(request.principal, request.type); @@ +91,5 @@ > + // Persist the choice if the user wants to remember > + Services.perms.addFromPrincipal(request.principal, request.type, action); > + } else { > + // Otherwise allow the permission for the current session > + Services.perms.addFromPrincipal(request.principal, request.type, action, Ci.nsIPermissionManager.EXPIRE_SESSION); Nit: this line exceeds 80 columns and should be wrapped, f.e.: Services.perms.addFromPrincipal(request.principal, request.type, action, Ci.nsIPermissionManager.EXPIRE_SESSION); ::: webapprt/locales/en-US/webapprt/webapp.properties @@ +25,4 @@ > geolocation.remember=Remember my choice > > +# LOCALIZATION NOTE (desktop-notification.title): %S will be replaced with the name of > +# the webapp. Nit: the first of these two lines exceeds 80 columns, so these lines should be rewrapped.
Attachment #790497 -
Flags: review?(myk) → review+
Assignee | ||
Comment 5•8 years ago
|
||
Carrying r+.
Attachment #790497 -
Attachment is obsolete: true
Attachment #792481 -
Flags: review+
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 6•8 years ago
|
||
This landed as 892837. Please double-check your commit messages in the future. Backed out and re-landed. https://hg.mozilla.org/integration/fx-team/rev/a1ba99bda320
Keywords: checkin-needed
Comment 7•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a1ba99bda320
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
Updated•5 years ago
|
Product: Firefox → Firefox Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•