Closed Bug 1183853 Opened 4 years ago Closed 4 years ago

Rename hasPermission() to permissionState()

Categories

(Core :: DOM: Push Notifications, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla42
Tracking Status
firefox42 --- fixed

People

(Reporter: nsm, Assigned: nsm)

References

Details

(Keywords: dev-doc-complete)

Attachments

(1 file)

Bug 1183853 - Rename hasPermission() to permissionState(). r?mt
Attachment #8633697 - Flags: review?(martin.thomson)
Comment on attachment 8633697 [details]
MozReview Request: Bug 1183853 - Rename hasPermission() to permissionState(). r?mt

https://reviewboard.mozilla.org/r/13247/#review11885

Ship It!

::: dom/push/Push.js:316
(Diff revision 1)
>      }.bind(this));

Would you mind changing this to an => function?
Attachment #8633697 - Flags: review?(martin.thomson) → review+
In case it's not already obvious, I'm not a DOM peer, so my review is not sufficient to land WebIDL changes.
Comment on attachment 8633697 [details]
MozReview Request: Bug 1183853 - Rename hasPermission() to permissionState(). r?mt

Bug 1183853 - Rename hasPermission() to permissionState(). r?mt
Attachment #8633697 - Flags: review+ → review?(bugs)
Olli for DOM peer sign off.
Comment on attachment 8633697 [details]
MozReview Request: Bug 1183853 - Rename hasPermission() to permissionState(). r?mt

So we're still missing the PushSubscriptionOptions param.
Attachment #8633697 - Flags: review?(bugs) → review+
I believe :mt is against this and the intention is to drop it from the spec?
Flags: needinfo?(martin.thomson)
oh, I meant the patch is ok, and I was assuming we'll add the param later, if needed.
url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/2fcda90e38a23b489b4fd5281cf79dc745ea2117
changeset:  2fcda90e38a23b489b4fd5281cf79dc745ea2117
user:       Nikhil Marathe <nsm.nikhil@gmail.com>
date:       Tue Jul 14 14:27:32 2015 -0700
description:
Bug 1183853 - Rename hasPermission() to permissionState(). r=mt,smaug
Yeah, I'm still against it, but only really the specific option that Google have implemented.  People developing against chrome will pass a dictionary; we shouldn't choke on that.
Flags: needinfo?(martin.thomson)
https://hg.mozilla.org/mozilla-central/rev/2fcda90e38a2
Assignee: nobody → nsm.nikhil
Status: NEW → RESOLVED
Closed: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Hmm, the Push API has been implemented with Firefox 42, so it should have no compatibility impact. Removing the compat doc.
Keywords: site-compat
You need to log in before you can comment on or make changes to this bug.