1144693 - Disable libnotify usage on Linux by default for new-mail notifications (doesn't always work after bug 858919)

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[rsx11m]

Initially noticed with SeaMonkey 2.33 release on x86_64 with KDE4, also confirmed for Earlybird 38.0a2:

> http://forums.mozillazine.org/viewtopic.php?f=5&t=2921717
> http://forums.mozillazine.org/viewtopic.php?p=14076713#p14076713
> http://logs.glob.uno/?c=mozilla%23seamonkey&s=15+Mar+2015&e=15+Mar+2015#c638403

When new mail arrives, Inbox gets bolded in the folder pane and the mail icon in the statusbar (SeaMonkey only) indicates new mail. However, no notification appears on the desktop. Some of these reports state that other notifications like downloads finished or from add-ons don't work. So, this may very well be a Toolkit bug, but filing against MailNews Core/Backend first to see if something is missing specific to the comm-central part.

Bug 858919 allows usage of libnotify on Linux again, if available. The underlying code for nsMessengerUnixIntegration::ShowAlertMessage() in mailnews/base/src/nsMessengerUnixIntegration.cpp#318 attempts to submit the message using the system alert service first; if that fails, it falls back to using ShowNewAlertNotification() instead.

Apparently, sending the alert to libnotify is either silently reported as a success even though it fails for whatever reason, or the resulting failure code isn't correctly interpreted and therefore the built-in alert isn't triggered as a fallback.

Note that manually raising an alert shows it correctly as a libnotify desktop notification: (code courtesy of barbaz)
> Components.classes["@mozilla.org/alerts-service;1"].getService(Components.interfaces.nsIAlertsService).showAlertNotification('chrome://branding/content/icon48.png', 'Test', 'This is a useless notice.');

Comment 1

[rsx11m]

When I get notifications, they are incomplete and only state the account along with a preview of the message text, but neither the sender nor the subject are provided in the notification despite all respective boxes checked in the preference settings.

Comment 2

[rsx11m]

To verify, I've modified mailnews/base/src/nsMessengerUnixIntegration.cpp, line 327, replacing

    if (NS_SUCCEEDED(rv)) {

with

    if (false) {

and indeed got the XUL notification for new mail again. This works for me with both SeaMonkey and Thunderbird trunk builds and is an indication that the alert itself still works. Just the libnotify communication doesn't work and silently fails for some reason.

Comment 3

[rsx11m]

An observation while testing the patch for bug 1144719: When the libnotify alert doesn't work, the "alertfinished" event never happens, thus nsMessengerUnixIntegration::ShowAlertMessage() is left immediately again for any subsequent new-mail notifications. alertsService->ShowAlertNotification() returns though, and with a "SUCCEEDED" rv status.

Since libnotify alerts thus far never worked for me when running my own build, I can't tell what exactly happens when alertsService->ShowAlertNotification() succeeds with the message shown.

Comment 4

[rsx11m]

Attachment: Disable libnotify for the time being

(Quoting rsx11m from bug 1144719 comment #5)
> If the pref is introduced as proposed before bug 1144693 is fixed, the pref
> could be initially set to "false" rather than "true" and thus preventing the
> case that the user doesn't see any notification whatsoever until that issue
> is fixed.

This patch is doing that, now that bug 1144719 attachment 8581187 [details] [diff] [review] has been approved and introduces the preference setting. Whoever is taking this bug, please revert the setting to "true" once libnotify is reliably working.

Comment 5

[rsx11m]

Comment on attachment 8590044 [details] [diff] [review]
Disable libnotify for the time being

I'm not sure if Neil didn't get to it or doesn't want to make the call, thus adding jcranmer as additional reviewer (whoever makes an r+ or r- first).

Comment 6

[Kent James (:rkent)]

I'm confused, this blocks Bug 1144719 that we just pushed, what is the relationship?

Does libnotify fail for all Linux users or only a few?

Comment 7

[rsx11m]

(In reply to Kent James (:rkent) from comment #6)
> I'm confused, this blocks Bug 1144719 that we just pushed, what is the relationship?

Yeah, maybe I've got the dependencies upside-down here. The relationship is that a fix here would reduce the need for a switch, whereas the switch introduced now with attachment 8581187 [details] [diff] [review] allows the user to work around the issue by avoiding libnotify for new-mail notifications.

The proposed patch would only make usage of the built-in notification the default (again) while waiting for a solution to the issue (the cause of which appears unknown at this time).

> Does libnotify fail for all Linux users or only a few?

I can't tell in an absolute number, but I've seen it failing on two (="all" in my access) Linux machines, both running KDE4, and my opening description contains references to respective reports from others. Intermittently a notification makes it, but then it is incomplete per comment #1 (I'd assume that this also affects users where it consistently might work).

Comment 8

[rsx11m]

Any opinion on the default? If it stays "true" then at least the issue should be relnoted and flipping the pref stated as a workaround (I'll do so for SeaMonkey). I see rkent's point, but only people who don't get any notifications will post in the forums, thus it's unknown for how many it works...

Comment 9

[Kent James (:rkent)]

I'm not much aware of the issues, but the justifications in https://bugzilla.mozilla.org/show_bug.cgi?id=858919#c0 don't appear to me to warrant the current risk that notification fails completely for Thunderbird Linux. So with my limited knowledge I'd approve flipping the default. But it would be better if we could get some more comments.

Comment 10

[WaltS48 [:walts48]]

No alert using Thunderbird 38.0b4 on Kubuntu 14.10 or 15.04 with Show an alert enabled, with the Customization of showing the alert for 10 seconds, and all fields enabled.

The Default system sound plays.

I do get alerts with Thunderbird 31.6.0.

Comment 11

[Kent James (:rkent)]

Comment on attachment 8590044 [details] [diff] [review]
Disable libnotify for the time being

Not hearing from Neil or Joshua, I also tried this in a test system (Debian 7 running Thunderbird 30.0b4). I go no alerts with the default use_system_alert true, did get alert with false.

So nobody has supported leaving this at the default, so I'm going to change it for now and approve the patch.

Rsx11m could you please file a followup patch to investigate why this does not work?

I'll land the patch.

Comment 12

[Kent James (:rkent)]

Comment on attachment 8590044 [details] [diff] [review]
Disable libnotify for the time being

http://hg.mozilla.org/releases/comm-aurora/rev/b7a2f5e38172
https://hg.mozilla.org/releases/comm-beta/rev/7b7d20c7a197

Comment 13

[rsx11m]

(In reply to Kent James (:rkent) from comment #11)
> Rsx11m could you please file a followup patch to investigate why this does not work?

Actually, I was thinking of leaving this bug open for the "real" fix, given that all information is already posted here. However, it may be better to retarget this bug to changing the default and clone this into a new bug as we are tracking the status flags here for this purpose already.

Thus, I've filed bug 1162788 for this.

> I'll land the patch.

Thanks. I assume that this will be merged automagically into comm-esr38?

Comment 14

[rsx11m]

Comment on attachment 8590044 [details] [diff] [review]
Disable libnotify for the time being

[Approval Request Comment]
Regression caused by (bug #): bug 858919
User impact if declined: possibly no alerts on Linux by default
Testing completed (on c-c, etc.): checked in on c-c, c-a, c-b
Risk to taking this patch (and alternatives if risky): low

This was pushed to comm-central as https://hg.mozilla.org/comm-central/rev/d39bdd7c8e4c

Requesting approval for a possible SeaMonkey 2.33.2 interim release.
This should be visible in 2.35, I assume no 2.34 will come any more.

Comment 15

[rsx11m]

Closing this bug as FIXED as all work is done for this part except the remaining branch approval.

Comment 16

[Kent James (:rkent)]

Keeping bugs open with patches that land over multiple releases, particularly involving uplifts, cannot be reliably managed by your release managers. A new bug is the proper approach.

Whatever we land into comm-beta will get merged into comm-esr38 when we start using that repo.

Comment 17

[rsx11m]

Ok, thanks!

Comment 18

[Ian Neal]

Won't fix on 2.34, will leave release team to decide against 2.33.x so leaving approval request

Comment 19

[rsx11m]

Comment on attachment 8590044 [details] [diff] [review]
Disable libnotify for the time being

SM 2.33.2 is off the table, and 2.35 has the fix, thus cancelling request.