Closed Bug 1241278 Opened 4 years ago Closed 4 years ago

`Notification.requestPermission()` should return a promise

Categories

(Core :: DOM: Push Notifications, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox46 --- fixed
firefox47 --- fixed

People

(Reporter: Lina, Assigned: Lina)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, site-compat)

Attachments

(1 file, 1 obsolete file)

Per https://notifications.spec.whatwg.org/#dom-notification-permission, the callback argument is deprecated.

`Notification.requestPermission` should return a promise fulfilled with the permission, invoking the callback if given.
Assignee: nobody → kcambridge
Status: NEW → ASSIGNED
Attachment #8710151 - Flags: review?(amarchesini) → review+
Comment on attachment 8710151 [details]
MozReview Request: Bug 1241278 - Change `Notification.requestPermission()` to return a promise. r=baku

https://reviewboard.mozilla.org/r/31671/#review28459

Do we have any plan to remove the callback completely? In case, file a bug as follow up.

::: dom/notification/Notification.cpp:254
(Diff revision 1)
>    {

MOZ_ASSERT(aPromise)

::: dom/notification/Notification.cpp:650
(Diff revision 1)
>  NotificationPermissionRequest::CallCallback()

rename this to ResolvePromise()
Comment on attachment 8710151 [details]
MozReview Request: Bug 1241278 - Change `Notification.requestPermission()` to return a promise. r=baku

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/31671/diff/1-2/
Attachment #8710151 - Attachment description: MozReview Request: Bug 1241278 - Change `Notification.requestPermission()` to return a promise. r?baku → MozReview Request: Bug 1241278 - Change `Notification.requestPermission()` to return a promise. r=baku
Blocks: 1241543
Keywords: leave-open
Comment on attachment 8710545 [details]
MozReview Request: Bug 1241278 - Record telemetry for notification permission callback usage. r?vladan

https://reviewboard.mozilla.org/r/31791/#review28965

::: toolkit/components/telemetry/Histograms.json:10019
(Diff revision 1)
> +    "kind": "count",

This will generate a histogram of the # of times the callback version of this API was called per Firefox session.

Due to the nature of count histograms, the 0 bucket will be omitted, so you will actually get a distribution of the # of times this version of the function was called per session (among sessions that called this version of the function at least once).

Will that be enough to gauge its popularity? Would it be better to count these with a boolean histogram, so that you end up with a ratio of sessions that used the callback version vs the promise version?

::: toolkit/components/telemetry/Histograms.json:10020
(Diff revision 1)
> +    "description": "Usage of the deprecated Notification.requestPermission() callback argument"

add an alert_emails field
Attachment #8710545 - Flags: review?(vladan.bugzilla)
https://reviewboard.mozilla.org/r/31791/#review28965

> This will generate a histogram of the # of times the callback version of this API was called per Firefox session.
> 
> Due to the nature of count histograms, the 0 bucket will be omitted, so you will actually get a distribution of the # of times this version of the function was called per session (among sessions that called this version of the function at least once).
> 
> Will that be enough to gauge its popularity? Would it be better to count these with a boolean histogram, so that you end up with a ratio of sessions that used the callback version vs the promise version?

You're right; I'd like to know which version is used, not the number of callback uses. Thank you!
Comment on attachment 8710545 [details]
MozReview Request: Bug 1241278 - Record telemetry for notification permission callback usage. r?vladan

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/31791/diff/1-2/
Attachment #8710545 - Flags: review?(vladan.bugzilla)
Attachment #8710151 - Attachment is obsolete: true
Attachment #8710545 - Flags: review?(vladan.bugzilla) → review+
Comment on attachment 8710545 [details]
MozReview Request: Bug 1241278 - Record telemetry for notification permission callback usage. r?vladan

https://reviewboard.mozilla.org/r/31791/#review29479
https://hg.mozilla.org/mozilla-central/rev/76f015b2176f
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Thanks for the help in updating the MDN docs, Kit!

I've expanded the details a little bit, to include browser compat information, and showing the older syntax alongside the promise just for completeness:

https://developer.mozilla.org/en-US/docs/Web/API/Notification/requestPermission

I've also updated compat info and code syntax in other places:

https://developer.mozilla.org/en-US/docs/Web/API/Notifications_API
https://developer.mozilla.org/en-US/docs/Web/API/notification
https://developer.mozilla.org/en-US/docs/Web/API/Notifications_API/Using_the_Notifications_API

I've also updated my silly emogotchi demo to use the new syntax:

http://mdn.github.io/emogotchi/

And finally, I've added a note to the relevant release notes:
*ahem*

  ...

And finally, I've added a note to the relevant release notes:

https://developer.mozilla.org/en-US/Firefox/Releases/47#Others
You need to log in before you can comment on or make changes to this bug.