605972 - Use balloon tips to notify new messages

Status: RESOLVED FIXED

(MailNews Core :: Backend, enhancement)

Description

[[e-mail]]

Since we switched to supporting Windows 2000 or later, we can actually use the balloon notification instead of the pop-up alert.

Comment 1

[[e-mail]]

Attachment: Proof of concept

I don't know whether Thunderbird would find this useful or not, but it affects a core file, so requesting sr.

Comment 2

[David :Bienvenu]

Comment on attachment 484823 [details] [diff] [review]
Proof of concept

I don't know if Thunderbird is interested in this - I'll have to try it out in SM.

Comment 3

[[e-mail]]

Attachment: Untested Thunderbird changes

I don't actually know whether this compiles for Thunderbird.

To test this you will need to add the balloon tip preference via config editor.

Comment 4

[David :Bienvenu]

Comment on attachment 488853 [details] [diff] [review]
Untested Thunderbird changes

thx for the patch, I got this to work for TB. I'll attach a patch that compiles in a minute...

Comment 5

[David :Bienvenu]

Attachment: get to compile in TB

This is pretty much a UI call - it's nice to use the nice OS  widget, but our new mail alert has so much more info (message subject, sender, snippet, and each message is clickable) that I'm pretty sure we wouldn't default to this. I suppose one option is to use the balloon if the user turns off message preview text, subject, and sender.

Comment 6

[[e-mail]]

Comment on attachment 488879 [details] [diff] [review]
get to compile in TB

>-#ifdef MOZ_THUNDERBIRD
>-  nsresult ShowNewAlertNotification(PRBool aUserInitiated);
>+#ifndef MOZ_THUNDERBIRD
>+  nsresult ShowAlertMessage(const nsString& aAlertTitle, const nsString& aAlertText, const nsACString& aFolderURI);
>+#else
>+  nsresult ShowNewAlertNotification(PRBool aUserInitiated, const nsString& aAlertTitle, const nsString& aAlertText);
> #endif
>-
Whoops, I didn't realise that your method needed to be public. Might as well just update the existing lines in place in that case. Or maybe use this:
> #ifdef MOZ_THUNDERBIRD
>-  nsresult ShowNewAlertNotification(PRBool aUserInitiated);
>+  nsresult ShowNewAlertNotification(PRBool aUserInitiated, const nsString& aAlertTitle, const nsString& aAlertText);
>+#else
>+  nsresult ShowAlertMessage(const nsString& aAlertTitle, const nsString& aAlertText, const nsACString& aFolderURI);
> #endif

Comment 7

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

Does anyone happen to have a screenshot of this?  I don't have a windows build available right now.  Thanks.

Comment 8

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

Comment on attachment 488879 [details] [diff] [review]
get to compile in TB

looks pretty cool.  not sure where the text I saw in the screenshot came from but 

- Screenshot -

*Google*
has 2 new messages

I feel like we should change the secondary text to: "2 new messages" for ease of translation and the 'has' reads better overall but I think it distracts from the message a little.  Sound good?

ui-r+ otherwise

Comment 9

[[e-mail]]

Attachment: Tweaked patch

Tweaked the ifdef as above and also added a default value for the pref.

Comment 10

[Karsten Düsterloh]

Comment on attachment 491612 [details] [diff] [review]
Tweaked patch

Seamonkey-Messenger-moa=me, given a Windows-based real review is passed (I don't have Windows).

Comment 11

[Frank Wein [:mcsmurf]]

Comment on attachment 491612 [details] [diff] [review]
Tweaked patch

Patch works fine on Windows XP with a SeaMonkey trunk build

Comment 12

[[e-mail]]

Pushed changeset 740d1cf79d2b to comm-central.