561427 - protocol errors on libnotify-based systems show only empty popup notification windows

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Dan Mosedale (:dmosedale, :dmose)]

+++ This bug was initially created as a clone of Bug #555536 +++

In particular, this is a spin-off of bug 555536 comment 22 and bug 555536 comment 25:

> There's one more issue with using libnotify. If it is installed then
> notifications implemented in bug 123440 appear empty. There's no text in them,
> just the icon. Without libnotify these appear with text. That's surely another
> bug, possibly filed (couldn't find though), but together with this one it
> degrades usefulness of using libnotify noticeably.

https://bug555536.bugzilla.mozilla.org/attachment.cgi?id=440869 has a screenshot (thanks Merike!).

Standard8 has a reproduced this, and has a theory about a band-fix.  If that fix works, we're golden.  However, if it doesn't, we may end up deciding that the best plan is to back out libnotify-based biffing entirely for now, and re-land it after 3.1.  And if that happens, it's much better for that to happen in B2 than in RC1, which is why it's a b2 blocker.

One thing that's not totally clear is whom it effects in what cases.  If it's not extremely prevalent, we could consider taking this off the blocking list.  Adding qawanted in the hopes that a kind Linux soul will explore in more detail exactly in what situation this gets triggered.

Comment 1

[Dan Mosedale (:dmosedale, :dmose)]

Thanks to some sleuthing from Standard8 and asuth, we believe that this bug is actually related to errors from the protocol machinery (including things like "can't connect to server"), not from biffing.  In Tb2 and Tb3, these were presented to the user with modal dialog.  

Additionally, Standard8 believes this can be fixed without a string change, assuming that we can live with the title of the alert being &brandShortName (i.e. "Thunderbird"), which I think we can.

This means, among other things:

1) we won't back out libnotify biffing because of this bug
2) it doesn't block b2, but it does block rc1

Giving to Standard8, as he's already got a patch.

Comment 2

[Mark Banner (:standard8)]

Attachment: Proposed fix

This is the proposed fix for this bug. The main issue was that libnotify requires a title to be specified and we weren't supplying one (growl doesn't need it).

I'm also asking Makoto for feedback as in bug 555536 comment 29 he mentioned that if libnotify isn't supported, the showAlertNotification throws an error. Why it doesn't put up the old-style alert, I'm not quite sure (but would be a core bug). However, I've included a try/catch, so worst-case we'll fallback to a modal prompt, but at least the user will get the error message.

Requesting ui-review from Bryan: I've used brandShortName for the notification title. The modal dialogs that this code replaced used to have a title of "Alert". I didn't want to use "Alert" as libnotify alerts can be from different programs, and I'm not 100% sure displaying the icon is supported, so I thought we'd use "Thunderbird" for 3.1, and if you want something different post 3.1 then we can create a new string on trunk. Thoughts welcome.

Comment 3

[David :Bienvenu]

Comment on attachment 442400 [details] [diff] [review]
Proposed fix

I can't really test this on Linux, but Bryan can, and the code looks OK to me.

Comment 4

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 442400 [details] [diff] [review]
Proposed fix

tried it out, looks good to me

Comment 5

[Mark Banner (:standard8)]

Checked in on trunk: http://hg.mozilla.org/comm-central/rev/3d2e9b49393d

Comment 6

[Mark Banner (:standard8)]

So due to a slight mistake of this patch getting included in the patch for bug 562786, it means that this fix did actually get into Thunderbird 3.1 b2 with these changesets:

http://hg.mozilla.org/releases/comm-1.9.2/rev/8932dbfe9bfd
http://hg.mozilla.org/releases/comm-1.9.2/rev/445885daedf8

Therefore marking as fixed in 3.1b2.

Comment 7

[Jason Haar]

Under Fedora 10, I've just upgraded from the "official" TB-3.0.4 release to TB-3.1 straight off the Mozilla site, and I think I've hit this bug

Ever since the upgrade, I'm seeing this empty popup appearing on occasion on the bottom right hand side of the screen, with a TB logo in it and nothing else

Fedora 10 is old-ish, so I'm wondering if this libnotify issue is still an issue for older versions or something? FC10 uses libnotify-0.4.5-2.fc11.i586

I have "show an alert" disabled - but these popups don't appear to be due to that - I dunno what they are of course as they're empty!

mail.biff "show_tray_icon" and "show_alert" are false - any idea how I can disable the darn things? TB-3.1 appears to be totally fine - I have no idea what the popups are trying to tell me...

Comment 8

[Jason Haar]

FWIW, I also upgraded to FF-3.6.6 and I am getting successful popups from that application - so my comments about this being a libnotify fault are probably incorrect.

Comment 9

[Christopher Aillon (sabbatical, not receiving bugmail)]

It looks like it got checked into the release branch only, but not the tip of comm-1.9.2 so this is still around...  Reopening.

http://hg.mozilla.org/releases/comm-1.9.2/file/8df4f8b5a46c/mail/components/activity/modules/alertHook.js

Comment 11

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: Better approach

IMHO, this is a better patch.  Since the Linux backend doesn't permit an empty title, it makes little sense to permit that from the toolkit code for clients to keep making this mistake.

Comment 12

[Jason Haar]

FYI editing modules/activity/alertHook.js and ensuring the title field wasn't empty definitely fixed the problem for me - and I got to see the popups I've been missing. So this is definitely the right track

Jason

Comment 13

[Jonathan Kamens]

(In reply to comment #11)
> Created an attachment (id=455994) [details]
> Better approach

Yes, I agree that this is a good way to fix this problem.

Comment 14

[Mark Banner (:standard8)]

Christopher, thanks for spotting this. I actually found that the problem was that

http://hg.mozilla.org/releases/comm-1.9.2/rev/3b34e3d4519c

accidentally backed out the alertHook.js changes from this bug.

Given that the gecko for the 3.1.1 version of Thunderbird is already fixed, and we'll want this in 3.1.1, I've made the decision to re-land the existing patch for Thunderbird and have pushed that to the nightly builds here:

http://hg.mozilla.org/releases/comm-1.9.2/rev/524f27904bd0

Therefore I'm going to re-mark this as fixed - although that revision doesn't yet guarantee entry into 3.1.1 (but it does to later 3.1 version), I'm going to use bug 575385 to ensure that it does get there and to highlight the fact we've fixed it.

The core patch sounds like a good idea, I would recommend that we move it to a core bug and get it submitted, then we can do a follow up to remove the brandShortName parts of this patch. If you can cc me on bugs filed, that would be useful.

Thanks again.