WebExtension notifications seem to fail on Nightly

VERIFIED FIXED in Firefox 48

Status

()

Toolkit
WebExtensions: Untriaged
--
major
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: cmills, Assigned: bsilverberg)

Tracking

unspecified
mozilla48
Points:
---
Bug Flags:
blocking-webextensions +

Firefox Tracking Flags

(firefox48 verified)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

I've been testing Will's i18n example:

https://github.com/mdn/webextensions-examples/tree/master/notify-link-clicks-i18n

This seems to work fine on Fx Dev Edition and Chrome regular/Canary, but on Nightly (48.0a1 (2016-04-05), I tested it on Mac and Windows) it fails.

The extension installs fine, and appears ok in about:addons

It looks like it is the notifications.create that fails, and you get “Error: Incorrect argument types for notifications.create.”
Flags: needinfo?(bob.silverberg)
We should make sure this is addressed before the next merge.
Severity: normal → major
Flags: blocking-webextensions+
(Assignee)

Comment 2

2 years ago
I'll get on this right away. I'll check first whether it's an issue with the sample extension or with the notifications API.
Assignee: nobody → bob.silverberg
Status: NEW → ASSIGNED
Iteration: --- → 48.3 - Apr 18
Flags: needinfo?(bob.silverberg)
(Assignee)

Comment 3

2 years ago
The problem is that, now that we've added a schema for notifications, all arguments are being enforced. You need to specify a `notificationId` as the first argument to `notifications.create()`, and that is missing from the example extension code.

I have issued a pull request [1] to fix this.

[1] https://github.com/mdn/webextensions-examples/pull/52
According to Chrome's docs, the notification ID should be optional.
(Assignee)

Comment 5

2 years ago
Well, it is optional, but it's also the first argument, so how does one go about passing in the second argument (`options`) without providing something for the first argument? You can pass an empty string, or null, but we need to pass something, don't we?
Nope, optional arguments can be left out entirely.
(Assignee)

Comment 7

2 years ago
Oh, I see it's actually not marked as optional in the schema, but it should have been. I'll fix that, which might negate the need for the pull request.
(Assignee)

Comment 8

2 years ago
Created attachment 8738696 [details]
MozReview Request: Bug 1262542 - WebExtension notifications seem to fail on Nightly, r?kmag

Review commit: https://reviewboard.mozilla.org/r/44655/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/44655/
Attachment #8738696 - Flags: review?(kmaglione+bmo)
Comment on attachment 8738696 [details]
MozReview Request: Bug 1262542 - WebExtension notifications seem to fail on Nightly, r?kmag

https://reviewboard.mozilla.org/r/44655/#review41403
Attachment #8738696 - Flags: review?(kmaglione+bmo) → review+
Just adding Will so he is aware of this bug.
(Assignee)

Updated

2 years ago
Keywords: checkin-needed
(In reply to Bob Silverberg [:bsilverberg] from comment #9)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=51300344a2c4

You seriously need to start pushing to Try on top of m-c. Having Try pushes full of random fx-team bustage makes looking at the results much more difficult than it should have to be. Also, can you please update your hg commit information with your full name instead of just bsilverberg? That's the usual practice :)

Comment 14

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/48fac32381ec
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
(Assignee)

Comment 15

2 years ago
(In reply to Ryan VanderMeulen [:RyanVM] from comment #12)
> (In reply to Bob Silverberg [:bsilverberg] from comment #9)
> > https://treeherder.mozilla.org/#/jobs?repo=try&revision=51300344a2c4
> 
> You seriously need to start pushing to Try on top of m-c. Having Try pushes
> full of random fx-team bustage makes looking at the results much more
> difficult than it should have to be. Also, can you please update your hg
> commit information with your full name instead of just bsilverberg? That's
> the usual practice :)

Sorry Ryan, I was not aware of either of these things. Thanks for letting me know. I've updated my username in .hgrc and I'll start pushing to try on top if m-c. I generally work just with fx-team, and I thought that's what the rest of my team did as well, but I understand that seeing the results against m-c with fewer failures would be much nicer.
Created attachment 8740396 [details]
video-1460464173.mp4

I was able to reproduce the initial issue on Firefox 48.0a1 (2016-04-06) using Windows 10 64-bit.
Tested on Firefox 48.0a1 (2016-04-11) across all platforms and I noticed the following:

- Windows 10 64-bit: a series of notifications are displayed in real time and in a chronological order: http://i.imgur.com/C8Ood2h.jpg

- Mac OS X 10.11: only one notification is displayed at a time, and it is updated according to accessed link in real time: http://i.imgur.com/z585TFp.png

- Ubuntu 14.04 32-bit: only one notification is displayed at a time, but it is not updated in real time, it changes after about 10 seconds. See the attached screencast.



Any thoughts about these discrepancies?
Flags: needinfo?(bob.silverberg)
(Assignee)

Comment 17

2 years ago
Thanks for reviewing this Vasilica. Because the notifications are displayed using the operating system, and not Firefox itself, I assume that these discrepancies are not unexpected. Kris, do you have any opinions about whether any of these represent bugs, or whether each behaviour is expected and acceptable?
Flags: needinfo?(bob.silverberg) → needinfo?(kmaglione+bmo)
This is expected. The notifications follow the standard behavior of the notification systems on a given platform.
Flags: needinfo?(kmaglione+bmo)
Thanks guys for the additional info!
Based on Comment 16, Comment 17 and Comment 18, I am marking this bug as Verified.
Status: RESOLVED → VERIFIED
status-firefox48: fixed → verified
You need to log in before you can comment on or make changes to this bug.