737646 - no biff notification if notification-daemon not running

Status: RESOLVED FIXED

(Toolkit :: General, defect)

Description

[Wolfgang Rosenauer [:wolfiR]]

This seems to be an old bug but I usually do not notice that stuff.

When mail notifications are enabled in Thunderbird (tested 12 here) I still don't see them in my environment. It works under Gnome where notification-daemon is running and libnotify is used.
It does not work under my window manager (no notification-daemon running).

I think there should be a fallback to the XUL notifications instead. (Firefox doing that for example for download completed notifications)

So I checked the code a bit and found:
http://hg.mozilla.org/releases/comm-beta/annotate/be8909bb020b/mailnews/base/src/nsMessengerUnixIntegration.cpp#l386

The fallback is apparently meant to be ShowNewAlertNotification()?

In that case I think this is not sufficient because in my environment libnotify is likely happily accepting the notification request but is actually not displaying anything because of the not running notification-daemon. Therefore the fallback never is reached.

Comment 1

[Wolfgang Rosenauer [:wolfiR]]

https://mxr.mozilla.org/comm-beta/source/mozilla/toolkit/system/gnome/nsAlertsIconListener.cpp#249
indeed returns success even if no notification-server is running and no notifications are displayed.

As I understand the code (I might miss something) the fallback only works
- if no NS_SYSTEMALERTSERVICE_CONTRACTID is available
  (the mozgnome component is not available or cannot be loaded)
- imgILoader->LoadImage() fails

I'm wondering why the fallback works in Firefox. I'll check what happens there to spot a difference.

Comment 2

[Wolfgang Rosenauer [:wolfiR]]

I have a candidate fix but still testing it. Will attach when I can confirm it works as expected.

Comment 3

[Wolfgang Rosenauer [:wolfiR]]

Attachment: patch

AFAICS notify_get_server_caps() is supposed to return at least one item. If no notification-daemon is running it throws an error message on stderr and returns NULL.
Therefore it makes no sense to proceed trying to use libnotify.

Comment 4

[Wolfgang Rosenauer [:wolfiR]]

Comment on attachment 608299 [details] [diff] [review]
patch

Cancel review request.

While I think the approach is correct this alone does not fix the issue.
The reason is that a second call to notify_get_server_caps() does not return NULL while there still is no notification-daemon running.
Need to investigate first what libnotify is doing here exactly.

Comment 5

[Wolfgang Rosenauer [:wolfiR]]

Attachment: patch #2

I was blind before. libnotify is not to blame but me don't recognizing an if statement.
(There would be another option to uninit libnotify and therefore try again with every alert request in case a notification-daemon becomes available but I think it's fine to record the initial non-working state.)

Comment 6

[Karl Tomlinson (:karlt)]

Comment on attachment 609232 [details] [diff] [review]
patch #2

notify_get_server_caps doesn't distinguish between no server and a server with no optional capabilities, but I guess such a server is rare these days.
This approach looks more appealing than making another synchronous DBus call.

Comment 7

[Wolfgang Rosenauer [:wolfiR]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/9d156f164c8e

Comment 8

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/9d156f164c8e