853104 - new mail notifications aren't using libnotify anymore

Status: RESOLVED FIXED

(Toolkit Graveyard :: Notifications and Alerts, defect)

Description

[Magnus Melin [:mkmelin]]

Looks like we suddenly using the fallback notifications, and not the libnotify ones. This is a recent regression, trunk from 2013-03-16 works fine, at least 2013-03-20 doesn't. 

I'd see these checkins (due to bug 782211, which is likely the cause).
https://hg.mozilla.org/comm-central/rev/6770dd37290f
https://hg.mozilla.org/comm-central/rev/026db2140214

Comment 1

[Mark Banner (:standard8)]

The only changes we made were to adapt to the core API changes. Therefore I think this is most likely a regression in core following bug 782211.

Comment 2

[Mark Banner (:standard8)]

Should probably be tracked as it is a regression.

Comment 3

[Marco Castelluccio [:marco]]

I think this was done on purpose (see bug 782211 comment 22 and following, and bug 782211 comment 97 and following)

Comment 4

[Marco Castelluccio [:marco]]

(In reply to Marco Castelluccio [:marco] from comment #3)
> I think this was done on purpose (see bug 782211 comment 22 and following,
> and bug 782211 comment 97 and following)

Sorry, bug 782211 comment 14, not 22.

Comment 5

[Alex Keybl [:akeybl]]

wchen - can you confirm comment 4, and move this into the TB product if we can't make core changes to support them in FF22?

Comment 6

[Mark Banner (:standard8)]

(In reply to Alex Keybl [:akeybl] from comment #5)
> wchen - can you confirm comment 4, and move this into the TB product if we
> can't make core changes to support them in FF22?

There are already bugs to implement notification center support on Mac (bug 852648) and Windows 8 notifications (bug 852917). I would treat this in the same fashion as those two bugs - as a core bug, not product specific.

Comment 7

[Alex Keybl [:akeybl]]

I'm trying to understand whether bug 782211 was landed assuming resolution of bug 852648 and bug 852917 in FF22, or whether engineering was hoping TB would make a comm-specific backout and diverge from Firefox.

Comment 8

[William Chen [:wchen]]

(In reply to Alex Keybl [:akeybl] from comment #5)
> wchen - can you confirm comment 4, and move this into the TB product if we
> can't make core changes to support them in FF22?

I can confirm that removing libnotify was intentional. It is important that notifications work on all platforms, not just the ones that had libnotify installed. In addition, not all libnotify implementations support necessary features to implement the updated alerts service API (being able to click and close them).

(In reply to Alex Keybl [:akeybl] from comment #7)
> I'm trying to understand whether bug 782211 was landed assuming resolution
> of bug 852648 and bug 852917 in FF22, or whether engineering was hoping TB
> would make a comm-specific backout and diverge from Firefox.

The idea was to use the XUL notifications and add back support for native notifications if they could support the updated alerts service API. There was no intent for a comm backout.

Comment 9

[Alex Keybl [:akeybl]]

mbanner - sounds like something you all need to change before Thunderbird 24 in that case, and not something we'll track for Firefox/Gecko release of 22.

Comment 10

[Philip Chee]

As I understand it the original design of nsMessengerUnixIntegration::ShowAlertMessage() in nsMessengerUnixIntegration.cpp is:

1. First try the system alerts service. Before Bug 782211, this would succeed only if libnotify was installed.
2. If the system alerts service fails then fallback to a mailnews specific XUL alert.

I think what happens after Bug 782211 is that the system alerts service never fails because all OS native implementations of nsIAlertsService were removed (including libnotify) and replaced by a XUL implementation of the W3C desktop notifications.

This means that we never fallback to the mailnews XUL alerts. Someone on *nix needs to test this but when I redesigned the mailnews specific XUL alerts someone on Linux confirmed this behaviour pre Bug 782211.

Currently there is a SeaMonkey specific pref "mail.biff.show_new_alert" that controls whether to show the system alert or the mailnews XUL alert. Currently, this only works on Windows and only in SeaMonkey (#ifndef MOZ_THUNDERBIRD).

It would trivial to make nsMessengerUnixIntegration understand this pref as well, and also to extend this to Thunderbird by removing a few IFDEF/IFNDEFs

Comment 11

[Philip Chee]

q.v. Bug 858919 - Web Notifications support does not integrate with libnotify

Comment 12

[Mark Banner (:standard8)]

I think all the necessary folks are cc'ed, so no extra TB specific work necessary.

Comment 13

[Mark Banner (:standard8)]

(In reply to Philip Chee from comment #10)
> This means that we never fallback to the mailnews XUL alerts. Someone on
> *nix needs to test this but when I redesigned the mailnews specific XUL
> alerts someone on Linux confirmed this behaviour pre Bug 782211.

That's not quite right, we're either using the core alerts service, or we're using the fallback that mailnews has. Either way, we're not using libnotify - the native system - as that's the bit that got removed from gecko.

Comment 14

[Wolfgang Rosenauer [:wolfiR]]

What is the status of this bug?
People are recognizing now with TB 24 that notifications have regressed (on Linux here).
From the original removal bug I don't see it coming back to Gecko. So are there plans to recover the libnotify based notifications within mailnews?

Comment 15

[Marco Castelluccio [:marco]]

(In reply to Wolfgang Rosenauer [:wolfiR] from comment #14)
> What is the status of this bug?
> People are recognizing now with TB 24 that notifications have regressed (on
> Linux here).
> From the original removal bug I don't see it coming back to Gecko. So are
> there plans to recover the libnotify based notifications within mailnews?

There's a discussion going on on dev-platform.

Comment 16

[Magnus Melin [:mkmelin]]

Tb24 from ubuntu does use libnotify notifications. Ubuntu applying some own patches?

Comment 17

[Mike Conley (:mconley) (:⚙️)]

(In reply to Magnus Melin from comment #16)
> Tb24 from ubuntu does use libnotify notifications. Ubuntu applying some own
> patches?

It's possible that the Ubuntu modification add-on is wired in to use libnotify. I seem to recall that being in there.

Comment 18

[Brad Laue]

Ubuntu are simply reverting the removal of the libnotify support - you can find the patch in:

https://launchpad.net/ubuntu/precise/+source/thunderbird/1:24.1.0+build1-0ubuntu0.12.04.1/+files/thunderbird_24.1.0+build1-0ubuntu0.12.04.1.debian.tar.gz

Comment 19

[flying sheep]

the standard way of doing notifications on linux is through DBUS: https://developer.gnome.org/notification-spec/

libnotify is a pretty thin wrapper around the DBUS interface (the only thing not “thin” about it is its use of GObject)

although i think linking against libnotify and requiring on linux is a valid path (it’s already installed on my otherwise pretty much KDE-only system), if we want to go the most compatible road possible, we should do what the old bug 404738 wanted: directly use DBUS to show notifications, i.e. reinvent libdbus without GObject.

Funnily bugs proposing exactly this (bug 750141 and bug 404738) were closed wontfix due to bug 783765 implementing notifications using libdbus! nobody back than had any kind of objections, so my questions:

1. do we really want to re-program libdbus? it’s no big library, but nevertheless it’s reinventing the wheel.
2. who is in charge of those gnome-y decisions to remove features without adequate replacements? please let’s not do that and only remove things if we really don’t need them or really have something better.

PS: how to do DBUS from Firefox? is there some nsIDBUSService?
PPS: could you point me to the discussion on dev-platform?

Comment 20

[Philip Chee]

Ideally Bug 858919 (Web Notifications support does not integrate with libnotify) will fix things for Thunderbird and other applications. Failing that you would need to implement a nsIAlertsService component, register it and make it replace the toolkit nsIAlertsService.

The IDL:
http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/components/alerts/nsIAlertsService.idl

An example implementation:
http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/components/alerts/nsXULAlerts.cpp

Of course your replacement component should push notifications to libnotify instead.

Comment 21

[Marco Castelluccio [:marco]]

(In reply to Philip Chee from comment #20)
> Ideally Bug 858919 (Web Notifications support does not integrate with
> libnotify) will fix things for Thunderbird and other applications. Failing
> that you would need to implement a nsIAlertsService component, register it
> and make it replace the toolkit nsIAlertsService.
> 
> The IDL:
> http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/components/alerts/
> nsIAlertsService.idl
> 
> An example implementation:
> http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/components/alerts/
> nsXULAlerts.cpp
> 
> Of course your replacement component should push notifications to libnotify
> instead.

We already had a nsIAlertsService implementation using libnotify. It was removed in bug 782211.

Comment 22

[Wolfgang Rosenauer [:wolfiR]]

FWIW, there is a Firefox addon written in plain JS using libnotify again. This could be rather easy made to work with TB probably. https://addons.mozilla.org/de/firefox/addon/gnotifier

Comment 23

[Merike Sell (:merike)]

(In reply to Wolfgang Rosenauer [:wolfiR] from comment #22)
> FWIW, there is a Firefox addon written in plain JS using libnotify again.
> This could be rather easy made to work with TB probably.
> https://addons.mozilla.org/de/firefox/addon/gnotifier

Thanks for mentioning that. It seems to work just fine at first glance after modifying install.rdf Thunderbird version only :)

Comment 24

[flying sheep]

(In reply to Wolfgang Rosenauer [:wolfiR] from comment #22)
> FWIW, there is a Firefox addon written in plain JS using libnotify again.
> This could be rather easy made to work with TB probably.
> https://addons.mozilla.org/de/firefox/addon/gnotifier

it works fine if it *happens* to be loaded before all other addons.

it doesn’t work if some other addon creates a reference to XUL notification service (e.g. using `const {notify} = require('sdk/notifications')` at the top of some file)

since addon load order isn’t controllable, GNotifier is and will always be inadequate (as much as i like it)

(In reply to Marco Castelluccio [:marco] from comment #21)
> We already had a nsIAlertsService implementation using libnotify. It was
> removed in bug 782211.

and the reason was what i called “GNOMEy”:

(In reply to William Chen [:wchen] from comment #13)
> I also removed libnotify support because I can't find
> documentation about all the capabilities, so it's possible that libnotify
> could work but I just can't find it in the documentation.
> 
> The native notification systems need to be able to close notifications and
> have a callback on showing, clicking and closing a notification.

no! that’s not how you do things! “i’m to lazy to dig in bad documentation, so i just throw out the superior implementation that i can’t get to work fully, and rely on the home-cooked inferior fallback”

this needs to be reverted, like ubuntu did.

Comment 25

[Karl Tomlinson (:karlt)]

(In reply to flying sheep from comment #19)
> 1. do we really want to re-program libdbus? it’s no big library, but
> nevertheless it’s reinventing the wheel.

DBus usage in Gecko is a bit of a mess at the moment.
The first usage was nsDBusService, but since then there have been many
additional uses that didn't find this suitable.  Perhaps it could have been
made suitable, or perhaps it is not necessary.

Whatever approach is used for DBus in the GTK port, it needs to hook into the
GLib event loop.  dbus_connection_setup_with_g_main() from dbus-glib provides
that.  GIO now provides GDBusConnection and IIUC that is now considered the
GLib way to do this.  Both of these use GObject of course.

Unless there is good reason to do so, I'd prefer to use the Glib higher level
methods than implement Gecko's own equivalent, just so that the APIs are more
conventional and so familiar.

Unfortunately the build machines that Mozilla uses have GLib 2.22, but
GDBusConnection is only available from 2.26.  We could probably make a case
for updating to GLib 2.26 packages from CentOS 6.5, but things move slowly
here.  GTK3 requirements will at least bump the GLib build package.

All in all, libnotify seems the easiest solution here.

If the removal of the libnotify support can be reverted without causing too
much pain, then that is the way to go.

Do Ubuntu also revert in Firefox, or only Thunderbird?
It should be possible to add a build-time switch for Thunderbird if this is
only practical in Thunderbird.

Comment 26

[Mike Hommey [:glandium]]

(In reply to Karl Tomlinson (:karlt, back May 5) from comment #25)
> Unfortunately the build machines that Mozilla uses have GLib 2.22, but
> GDBusConnection is only available from 2.26.  We could probably make a case
> for updating to GLib 2.26 packages from CentOS 6.5, but things move slowly
> here.  GTK3 requirements will at least bump the GLib build package.

Note it's not only a build machine requirement. At the moment, we still support running mozilla.org builds on debian 6 and rhel 6, which both come with glib < 2.26.

Comment 27

[flying sheep]

(In reply to Karl Tomlinson (:karlt, back May 5) from comment #25)
> Do Ubuntu also revert in Firefox, or only Thunderbird?

IDK

> It should be possible to add a build-time switch for Thunderbird if this is
> only practical in Thunderbird.

It isn’t only practical there. Firefox’ download-completed notification is pretty much the same usecase (simple notification most definitely supported by any native approach). Someone with the ability to do so should probably remove the bug title’s first two words.

Comment 28

[Wolfgang Rosenauer [:wolfiR]]

(In reply to Karl Tomlinson (:karlt, back May 5) from comment #25)
> All in all, libnotify seems the easiest solution here.
> 
> If the removal of the libnotify support can be reverted without causing too
> much pain, then that is the way to go.
> 
> Do Ubuntu also revert in Firefox, or only Thunderbird?
> It should be possible to add a build-time switch for Thunderbird if this is
> only practical in Thunderbird.

Another comment about the practicality question:
Native notifications are apparently practical anyway otherwise we wouldn't support them on Mac and Windows. The only valid reason I read before was that libnotify didn't/doesn't (not sure what is true atm) support everything required for proper web notification implementation.
If that's the case we should work upstream with libnotify people to support what we need and short term recover libnotify support for everything but "proper web notifications".

Comment 29

[flying sheep]

(In reply to Wolfgang Rosenauer [:wolfiR] from comment #28)
> If that's the case we should work upstream with libnotify people to support
> what we need and short term recover libnotify support for everything but
> "proper web notifications".

100% agree to the second part. mainly because the code is already there, we just need to revert a commit.

the first part should be handled in bug 858919

Comment 30

[Karl Tomlinson (:karlt)]

(In reply to Mike Hommey [:glandium] from comment #26)
> Note it's not only a build machine requirement. At the moment, we still
> support running mozilla.org builds on debian 6 and rhel 6, which both come
> with glib < 2.26.

I had assumed that RHEL user would be or could update to 6.5.
I don't know whether or not that is a reasonable assumption.

I hadn't noticed that debian squeeze (oldstable) was on 2.24.2.

Comment 31

[Matthew N. [:MattN]]

Bulk move to Toolkit::Notifications and Alerts

Filter on notifications-and-alerts-component.

Comment 32

[Philip Chee]

(In reply to Magnus Melin from comment #0)
> Looks like we suddenly using the fallback notifications, and not the
> libnotify ones. This is a recent regression, trunk from 2013-03-16 works
> fine, at least 2013-03-20 doesn't. 

Bug 858919 is fixed. Anyone on *nix want to test if the mail notifications are using libnotify again?

Components.classes['@mozilla.org/alerts-service;1'] .getService(Components.interfaces.nsIAlertsService) .showAlertNotification("https://www.google.com.au/images/google_favicon_128.png", "summary", "body");

Comment 33

[Magnus Melin [:mkmelin]]

Yes, alerts are back to libnotify again now.

Comment 34

[berturion]

Hello, I am on Linux Mint 17.2 64bit, cinnamon v2.6.13, thunderbird v38.2.0.
Is it normal that I have to install this add-on https://addons.mozilla.org/eN-US/thunderbird/addon/gnome-integration/ in order to see notifications on my desktop ?
Is that confirm that this bug is fixed ?
Thanks.

Comment 35

[rsx11m]

It was supposed to work out of the box; however, bug 1144719 followed by bug 1144693 disabled it by default for new-mail notifications due to bug 1152773 and bug 1162788. In short, notifications don't show up consistently and may be omitted, and if they show, they don't contain all information.

Flipping mail.biff.use_system_alert in the Config Editor to "true" enables it for new-mail alerts, otherwise the built-in XUL alerts are used.

Comment 36

[rsx11m]

[adding those as blocking for the sake of better documentation.]

Comment 37

[antonio.malcolm]

Flipping mail.biff.use_system_alert in the Config Editor to "true" does not work, when using libnotify (on Openbox, Void Linux).

Comment 38

[antonio.malcolm]

Should probably also mention I'm using Dunst, for the actual notifications. Dunst only requires the use of libnotify for notifications.

Comment 39

[rsx11m]

That's either a new regression or specific to your setup. What is "does not work" implying: you don't get a notification at all then (which is bug 1162788) or still the built-in notification?

Comment 40

[antonio.malcolm]

1162788 might cover it- except I get no notifications, at all. It isn't a "not always working" scenario, it's a "doesn't work, at all" scenario. All other apps installed, which use libnotify, work as expected. Being that I'm using Openbox, libnotify, and dunst, my setup is fairly simple and straight-forward. I can send along my dunst config file, if you think that'll help (but I don't think it would be useful).