1144719 - Allow the user to decide whether or not to use libnotify for new-mail alerts on Linux

Status: RESOLVED FIXED

(MailNews Core :: Backend, enhancement)

Description

[rsx11m]

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.

Bug 1144693 issues aside, there is no reason why to force the user to depend on libnotify for new-mail (and possibly other) alerts. On Windows, there is a dedicated preference ("mail.biff.show_balloon") to use the system's mechanism of icon ballons for new-mail notifications. Similarly, a preference on Mac OSX ("mail.biff.animate_dock_icon") enables or disables that desktop effect.

There may be reasons for a user to revert to the built-in XUL notification instead: Be it configurability for the time it is shown (thus being able to read it before it disappears again), or simply to have a dedicated style for such alerts raising better attention to it.

Thus, suggestion to provide a preference on Linux ("mail.biff.use_system_alert" defaulting to "true") which would determine in nsMessengerUnixIntegration::ShowAlertMessage() whether or not the if() statement for using the system alert service and returning if it was successful is entered. With that pref set to "false" the fallback ShowNewAlertNotification() would be used instead.

Comment 1

[flying sheep]

> configurability for the time it is shown

there are dozens of providers for the notification API. users can always choose one that can be configured how they want it.

you’re still right: why *not* addd this option?

Comment 2

[rsx11m]

(In reply to flying sheep from comment #1)
> there are dozens of providers for the notification API. users can always
> choose one that can be configured how they want it.

Thanks, guess I just didn't find that option yet in the KDE preferences for the notifications...

Anyway, I still rather like the distinct XUL notification for new mail, thus I'll go ahead and work out a patch for adding the preference (proof of concept works per bug 1144693 comment #2).

Comment 3

[rsx11m]

Attachment: WIP

Minimal patch, with room for improvement. The comments in the prefs should probably be converted into real #ifdef clauses, thus we don't offer those prefs for platforms which aren't using them. Also, if useSystemAlert is false, no need to obtain alertsService in the first place.

Comment 4

[rsx11m]

Attachment: Proposed patch

This patch resolves a typo in the pref definition of the WIP version and won't check for the libnotify service being available if the pref is false anyway. Also "#ifdef"s the pref (and some other platform-specific prefs as a drive-by fix).

As said, without this pref, users won't be able to get notified for new mail by the built-in alert at all and have to rely on libnotify to do the job.

Comment 5

[rsx11m]

Attachment: Dummy patch for bug 1144693

This patch is only needed for testing and goes on top of attachment 8581187 [details] [diff] [review].

Since the libnotify alerts may not be working right now, this patch is adding some crude stderr debug output to see when the notification was triggered. Also, it works around bug 1144693 comment #3 by explicitly calling AlertFinished() upon success of alertsService->ShowAlertNotification().

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.

Comment 6

[rsx11m]

Since Neil approved it and Joshua didn't voice any opinion, I assume that this is ready to land.
Push for comm-central, please.

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/comm-central/rev/45710af4129c

Comment 8

[rsx11m]

Comment on attachment 8581187 [details] [diff] [review]
Proposed patch

[Approval Request Comment]
Regression caused by (bug #): Bug 858919
User impact if declined: no or uncertain new-mail notifications on Linux
Testing completed (on c-c, etc.): works fine on trunk
Risk to taking this patch (and alternatives if risky): low, default behavior unchanged pending bug 1144693 attachment 8590044 [details] [diff] [review]

Requesting approval-comm-release for possible SeaMonkey 2.33.2 and/or 2.34.

Comment 9

[Kent James (:rkent)]

Comment on attachment 8581187 [details] [diff] [review]
Proposed patch

https://hg.mozilla.org/releases/comm-aurora/rev/415721b0657e
https://hg.mozilla.org/releases/comm-beta/rev/de98ea1bf621

comm-release is SeaMonkey only, so I did not do the approval or uplift for that.

Comment 10

[rsx11m]

(In reply to Kent James (:rkent) from comment #9)
> comm-release is SeaMonkey only, so I did not do the approval or uplift for that.

Thanks, Ian?

Comment 11

[Ian Neal]

Depends on whether there is another 2.33.x

Comment 12

[Edmund Wong (:ewong)]

(In reply to Ian Neal from comment #11)
> Depends on whether there is another 2.33.x

The likelihood is no.  Right now 2.35 is release and I'm wondering
if it's even likely we do a 2.35 beta; but I suppose that's Callek's
decision.

Comment 13

[rsx11m]

If we can do a 2.35 in a reasonable time frame, that's certainly preferable. I've left the requests (here and for the dependent patch in bug 1144693) up in case we can't.

Comment 14

[rsx11m]

Comment on attachment 8581187 [details] [diff] [review]
Proposed patch

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