Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?) Assignee
	Description • 9 years ago

      No description provided.

	Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?) Assignee
	Comment 1 • 9 years ago

Bug 1183853 - Rename hasPermission() to permissionState(). r?mt

	Martin Thomson [:mt:]
	Comment 2 • 9 years ago

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?

	Martin Thomson [:mt:]
	Comment 3 • 9 years ago

In case it's not already obvious, I'm not a DOM peer, so my review is not sufficient to land WebIDL changes.

	Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?) Assignee
	Comment 4 • 9 years ago

Comment on attachment 8633697 [details]
MozReview Request: Bug 1183853 - Rename hasPermission() to permissionState(). r?mt

Bug 1183853 - Rename hasPermission() to permissionState(). r?mt

	Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?) Assignee
	Comment 5 • 9 years ago

Olli for DOM peer sign off.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 6 • 9 years ago

Comment on attachment 8633697 [details]
MozReview Request: Bug 1183853 - Rename hasPermission() to permissionState(). r?mt

So we're still missing the PushSubscriptionOptions param.

	Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?) Assignee
	Comment 7 • 9 years ago

I believe :mt is against this and the intention is to drop it from the spec?

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 8 • 9 years ago

oh, I meant the patch is ok, and I was assuming we'll add the param later, if needed.

	Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?) Assignee
	Comment 9 • 9 years ago

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

	Martin Thomson [:mt:]
	Comment 10 • 9 years ago

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.

	Ryan VanderMeulen [:RyanVM]
	Comment 11 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/2fcda90e38a2

	Kohei Yoshino [:kohei]
	Comment 12 • 9 years ago

Site compat doc: https://www.fxsitecompat.com/en-US/docs/2015/pushmanager-haspermission-has-been-removed/

	Kohei Yoshino [:kohei]
	Comment 13 • 9 years ago

Hmm, the Push API has been implemented with Firefox 42, so it should have no compatibility impact. Removing the compat doc.

	Chris Mills [:cmills]
	Comment 14 • 9 years ago

hasPermission() has been marked deprecated:

https://developer.mozilla.org/en-US/docs/Web/API/PushManager/hasPermission

permissionState() has been documented:

https://developer.mozilla.org/en-US/docs/Web/API/PushManager/permissionState

A note has been added to the relevant release notes:

https://developer.mozilla.org/en-US/Firefox/Releases/42

Thanks!