Closed
Bug 1295877
Opened 8 years ago
Closed 8 years ago
Put Permissions API .revoke() behind a pref
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla51
Tracking | Status | |
---|---|---|
firefox51 | --- | fixed |
People
(Reporter: marcosc, Assigned: marcosc)
Details
(Keywords: dev-doc-complete, site-compat)
Attachments
(1 file, 1 obsolete file)
13.33 KB,
patch
|
Details | Diff | Splinter Review |
It seems we prematurely shipped the .revoke() method on the Permissions API before it was stable or deciding if we even wanted it in the platform. For those that don't know it: navigator.permission.revoke() allows a site to self-revoke a permission after a user has granted that permission. For example, a user may grant foo.com access to geolocation, but upon signing out of a site, a site might call .revoke({name:"geolocation"}) so that the next user to log into the site doesn't automatically get access to geolocation (because permissions are bound to origin). A few folks (who can chime in) working on the standard have raised concerns about the design, so we would like to suggest we put it behind a pref for now. Particularly, using in-browser user profiles can handle the above use case without a site taking away a user's decision. There seems to be consensus that .query() is useful, so that one can remain.
Updated•8 years ago
|
Keywords: dev-doc-needed,
site-compat
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → mcaceres
Assignee | ||
Comment 1•8 years ago
|
||
Birunthan, because the .revoke() API is now behind a pref, I had to refactor the tests to use an iframe's window object. I also cleaned up the tests, as there were lots of unused variables, and things upsetting jshint.
Attachment #8781861 -
Flags: review?(birunthan)
Assignee | ||
Comment 2•8 years ago
|
||
Comment on attachment 8781861 [details] [diff] [review] Puts .revoke() behind pref. Refactors tests. Bobby, can you kindly review the IDL change?
Attachment #8781861 -
Flags: superreview?(bobbyholley)
Assignee | ||
Comment 3•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a31af707fca1
Assignee | ||
Comment 4•8 years ago
|
||
Comment on attachment 8781861 [details] [diff] [review] Puts .revoke() behind pref. Refactors tests. Ah, my git blame archeology was off, :marco actually implemented .revoke().
Attachment #8781861 -
Flags: review?(birunthan) → review?(mcastelluccio)
Comment 5•8 years ago
|
||
Comment on attachment 8781861 [details] [diff] [review] Puts .revoke() behind pref. Refactors tests. Review of attachment 8781861 [details] [diff] [review]: ----------------------------------------------------------------- r=me on the webidl change assuming there continues to be no objection to it on the dev-platform thread. Do we need an "Intent to unship" or did we never actually ship it?
Attachment #8781861 -
Flags: superreview?(bobbyholley) → superreview+
Assignee | ||
Comment 6•8 years ago
|
||
(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #5) > Do we need an "Intent to unship" or did we never actually ship it? I was unsure, does pref'ing in off count as "unship"? We did ship it.
Flags: needinfo?(bobbyholley)
Assignee | ||
Comment 7•8 years ago
|
||
*it
Comment 8•8 years ago
|
||
I'd think so, but looking back I think the existing dev-platform thread is probably sufficient.
Flags: needinfo?(bobbyholley)
Assignee | ||
Updated•8 years ago
|
Attachment #8781861 -
Flags: review?(mcastelluccio) → review?(amarchesini)
Updated•8 years ago
|
Attachment #8781861 -
Flags: review?(amarchesini) → review+
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 9•8 years ago
|
||
Assignee | ||
Comment 10•8 years ago
|
||
Attachment 8785787 [details] [diff] obsoletes attachment 8781861 [details] [diff] [review]. Forgot to check the box.
Updated•8 years ago
|
Attachment #8781861 -
Attachment is obsolete: true
Comment 11•8 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/19c4f264ce37 Put Permissions API .revoke() behind a pref. r=baku, sr=bholley
Keywords: checkin-needed
Comment 12•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/19c4f264ce37
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Comment 13•8 years ago
|
||
Posted the site compatibility doc: https://www.fxsitecompat.com/en-CA/docs/2016/permissions-revoke-has-been-disabled/
Comment 14•8 years ago
|
||
Pages updated: https://developer.mozilla.org/en-US/docs/Web/API/Permissions/revoke https://developer.mozilla.org/en-US/docs/Web/API/Permissions https://developer.mozilla.org/en-US/docs/Web/API/Permissions_API/Using_the_Permissions_API https://developer.mozilla.org/en-US/Firefox/Releases/51 This should be all there is. Please let me know if you find anything else that needs to be done. Thanks!
Keywords: dev-doc-needed → dev-doc-complete
Comment 15•7 years ago
|
||
Marcos, why did you add |debugger;| in the test? https://hg.mozilla.org/mozilla-central/rev/19c4f264ce37#l3.237
Flags: needinfo?(mcaceres)
Assignee | ||
Comment 16•7 years ago
|
||
> Marcos, why did you add |debugger;| in the test?
I forgot to remove that before we merged. Please feel free to remove.
Flags: needinfo?(mcaceres)
Comment 17•7 years ago
|
||
Pushed by VYV03354@nifty.ne.jp: https://hg.mozilla.org/integration/mozilla-inbound/rev/349dbf222bac Remove accidentally added |debugger;|. r=marcosc
Comment 18•7 years ago
|
||
(In reply to Marcos Caceres [:marcosc] from comment #16) > > Marcos, why did you add |debugger;| in the test? > > I forgot to remove that before we merged. Please feel free to remove. Thanks, pushed a remove commit.
Comment 19•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/349dbf222bac
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•