Closed Bug 1262542 Opened 4 years ago Closed 4 years ago

WebExtension notifications seem to fail on Nightly

Categories

(WebExtensions :: Untriaged, defect, major)

defect
Not set
major

Tracking

(firefox48 verified)

VERIFIED FIXED
mozilla48
Iteration:
48.3 - Apr 25
Tracking Status
firefox48 --- verified

People

(Reporter: cmills, Assigned: bsilverberg)

Details

Attachments

(2 files)

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+
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)
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.
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.
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.
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.
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 :)
https://hg.mozilla.org/mozilla-central/rev/48fac32381ec
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
(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.
Attached video 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)
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
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.