Closed Bug 1376944 Opened 7 years ago Closed 1 year ago

notifications.create should return Promise.reject (if notifications.create failed)

Categories

(WebExtensions :: General, defect, P5)

defect

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: kernp25, Unassigned)

Details

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:56.0) Gecko/20100101 Firefox/56.0
Build ID: 20170628030206

Steps to reproduce:

It says [1], notifications.create can fail on some systems, so i think it should return a Promise.reject here (or something else) instead of just returning 'Promise.resolve(notificationId);' [2].

[1] http://searchfox.org/mozilla-central/source/toolkit/components/extensions/ext-notifications.js#37
[2] http://searchfox.org/mozilla-central/source/toolkit/components/extensions/ext-notifications.js#116

Because, otherwise how can we detect if notifications.create succeed or not?

If notifications.create failed for some reason we then can create our own notification and display it to the user.

Because my addon relies on the notification.
This seems like a valid suggestion. Kris, what do you think?
Flags: needinfo?(kmaglione+bmo)
I suppose one problem with doing this would be if Chrome does not do this, in which case we might break some extensions by throwing errors when they are not expected. Redirecting my needinfo to zombie for his ideas.
Flags: needinfo?(kmaglione+bmo) → needinfo?(tomica)
I don't think this is actually a real issue, that comment probably refers to non-supported platforms like Thunderbird or something.

I guess rejecting in that case wouldn't hurt, but probably P5.
Flags: needinfo?(tomica)
Priority: -- → P5
Product: Toolkit → WebExtensions

It seems, that Google is removing the notifications api.

Is this bug now WONTFIX?

Flags: needinfo?(rob)

Even in Chrome, the create method does not return an error if the system does not support notifications, so from the compatibility perspective, there is no need to change anything.

To detect that a notification has closed too soon, use the notifications.onClosed event. I don't think that we need to reject the notifications.create call if the notification was valid, hence I'm fine with WONTFIXing this bug.

Flags: needinfo?(rob)
Severity: normal → S3

This bug should be closed?

Flags: needinfo?(rob)
Status: UNCONFIRMED → RESOLVED
Closed: 1 year ago
Flags: needinfo?(rob)
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.