This API is deprecated. This bug is about totally removing it when the time has come.

Doing a quick grep, it seems the only place that still uses mozNotification.createNotification is share/js/notification_helper.js. A further grep reveals the following apps using it: costcontrol calendar dialer email?? wappush system We should either fix NotificationHelper to use the new Notification API, assuming we can do so without breaking existing functionality. Or file bugs in each of these apps so we can get the eyes of module owners on this issue. This is starting to hurt us, supporting both API's in gecko while we try to add features to Web Notifications. Alex, any ideas on the approach we should take here?

(In reply to Michael Henretty [:mhenretty] from comment #1) > Doing a quick grep, it seems the only place that still uses > mozNotification.createNotification is share/js/notification_helper.js. A > further grep reveals the following apps using it: > > costcontrol > calendar > dialer > email?? > wappush > system > > We should either fix NotificationHelper to use the new Notification API, > assuming we can do so without breaking existing functionality. Or file bugs > in each of these apps so we can get the eyes of module owners on this issue. > This is starting to hurt us, supporting both API's in gecko while we try to > add features to Web Notifications. > > Alex, any ideas on the approach we should take here? Bugs have been filed a long time ago already. Someone needs to push for this.

Those are blocking bug 938541

Thanks for the info. We need this!

We can now remove the API completely. :)

(In reply to Zibi Braniecki [:gandalf] from comment #5) > We can now remove the API completely. :) I think the proper wording is that we can start the API self-destruction process: there may be content using it (apps on the marketplace), so I doubt we can just kill it right now, but rather start deprecating and killing it soon.

Absolutely :) How do we start the process? Can we mark it as deprecated/fire a warning?

Posted the site compatibility doc: https://www.fxsitecompat.com/en-US/docs/2015/navigator-moznotification-will-be-removed/

mozNotification has been pref'd off on desktop since January 2011, and it's no longer used in FxOS since that's no longer in the codebase either. Taking and removing.

Removal passed try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cd7436ee287467de6236f57c3fc76a47419616bb&selectedJob=150970312

Posted the site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2017/navigator-moznotification-has-been-removed/

Comment on attachment 8935979 [details] Bug 952453 - Remove mozNotification API https://reviewboard.mozilla.org/r/206834/#review212654

Comment on attachment 8935980 [details] Bug 952453 - Move all notification tests to dom/notification https://reviewboard.mozilla.org/r/206836/#review212656 Unrelated. Can you move this in a separate bug? And land it. Thanks!

Moving test cleanup to Bug 1424571

Pushed by kmachulis@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/2e682c1841a1 Remove mozNotification; r=baku

Backed out 2 changesets (bug 1424571, bug 952453) for FATAL ERROR PROCESSING MOZBUILD FILE r=backout on a CLOSED TREE https://hg.mozilla.org/integration/mozilla-inbound/rev/f2e83780f1b36281e9ddf74cc2a67e277799f236 https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=c05f6d6df5104d551828f54b86f7bf04ffe10d57&filter-classifiedState=unclassified&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception https://treeherder.mozilla.org/logviewer.html#?job_id=151066271&repo=mozilla-inbound&lineNumber=1828

Missed service worker test updates in bug 1424571. Fixed but rerunning on try first just in case.

Pushed by kmachulis@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/8fd1d4a79a48 Remove mozNotification; r=baku

Comment on attachment 8935979 [details] Bug 952453 - Remove mozNotification API https://reviewboard.mozilla.org/r/206834/#review215938 Hello, I would recommend that this bug get backed out because of the below issues. In the future please get review from peers for all relevant modules as I suspect they would have caught this issue (in this case Cocoa ones). Unfortunately reviewboard doesn't show the contents of deleted tests but I looked at them and don't think many should have been deleted just because they use navigator.mozNotification rather than window.Notification since some of the behaviour being tested was about shared code. Some of the deleted tests didn't even use mozNotification. * test_system_principal.xul wasn't even using mozNotification but wasn't in any .ini file but it should be added to a chrome.ini instead of deleted. * test_notification_tag.html is still relevant AFAICT and wasn't using mozNotification * test_leak_windowClose.html is probably stil relevant if switched to window.Notification * etc. I do see that the tests were not being run (bug 1043918) due to a mistake in bug 899574 but that's something that should be fixed IMO. Our test coverage of notifications is already quite weak IMO and I'm worried that we're making it worse. ::: widget/cocoa/OSXNotificationCenter.mm (Diff revision 1) > -- (void)userNotificationCenter:(id<FakeNSUserNotificationCenter>)center > - didDeliverNotification:(id<FakeNSUserNotification>)notification > -{ > - > -} > - None of the Cocoa could should have been removed as it's not specific to the mozNotification API, it's used for the standard `new window.Notification` API and possibly push notifications. This has caused bug 1427996 but also regressed behaviour that each of the delegate methods was providing (close events, showing notifications when the application is focused, handling of the alternate actions dropdown to open prefs or disable for the site).

Ok, was looking at this right now anyways, I'll back it out now and follow up here.

Try run for backout happening at https://treeherder.mozilla.org/#/jobs?repo=try&revision=e1e57aacf77f67ffa13aeef083c7e1694b9898c1. After that passes, I'll land backout on inbound, then will remove cocoa changes, add a "this is used indirectly, don't delete" comment on it so no one tries this again without asking, and fix up the tests. Will keep :MattN as reviewer for now since they're aware of the situation.

Pushed by kmachulis@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/6500c02faddf Backing out 8fd1d4a79a48 due to notification bustage on MacOS

Ok, updated tests that were deleted in the first version of the patch in this bug: - test_basic_notification - already covered by dom/notifications/test/mochitest/test_notification_basics.html - test_basic_notification_click - already covered by dom/notifications/test/mochitest/test_notification_basics.html - test_leak_windowClose - leaks would be triggered in test_notification_basics, so calling it already covered. - test_system_principal - moved to dom/notifications/test/browser/test_notification_system_principal.html - test_notification_tag - moved to dom/notifications/test/mochitest/test_notification_tag.html

Comment on attachment 8935979 [details] Bug 952453 - Remove mozNotification API https://reviewboard.mozilla.org/r/206834/#review216318 Note that you may want/need to re-add :baku as a reviewer (the r+ should carry over) since I think there may be hooks to ensure DOM review on test_interfaces.js but I may be misremembering. Thanks ::: dom/notification/test/mochitest/test_notification_tag.html:1 (Diff revision 2) > +<!DOCTYPE HTML> Nit: should this have been an `hg mv` too?

Since I'm using git-cinnabar and git doesn't like move + change, I just split this into 2 patches. We move in the first one, change in the second.

Pushed by kmachulis@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/781a6bc83dde Remove mozNotification API; r=mattn r=baku https://hg.mozilla.org/integration/mozilla-inbound/rev/e0895bd719ee Fix test_notification_tag to point to correct location for create_notification.html; r=mattn

Went ahead and pushed this, as the patch division is no different than the originally reviewed code, and I'd really like to get Mac user's notifications working again. :)

Comment on attachment 8940355 [details] Bug 952453 - Fix test_notification_tag to point to correct location for create_notification.html; https://reviewboard.mozilla.org/r/210624/#review216982 Thanks

https://hg.mozilla.org/mozilla-central/rev/781a6bc83dde https://hg.mozilla.org/mozilla-central/rev/e0895bd719ee

I've documented this by archiving the property page and adding a note to the Fx59 rel notes: https://developer.mozilla.org/en-US/docs/Archive/API/Navigator/mozNotification https://developer.mozilla.org/en-US/Firefox/Releases/59#APIs_2 We never documented the DesktopNotification interface. Let me know if that looks OK. Thanks!

lgtm, thanks!