Closed Bug 119344 Opened 23 years ago Closed 23 years ago

add icon to system tray when biff goes off

Categories

(SeaMonkey :: MailNews: Message Display, defect, P1)

x86
Windows 2000
defect

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.9.9

People

(Reporter: mscott, Assigned: mscott)

Details

Attachments

(2 files, 1 obsolete file)

Part of the spec for new mail notification is to place an icon in the system tray (just like 4.x) on windows when new mail is available. I have already filed Bug #119328 to get a new icon to use for this. My current patch uses the icon from nsotify in 4.x (a white mailbox with a red flag over it). Behavior: when biff goes off and we have new mail available for one or more accounts, the icon is placed in the system tray. If you read one of these new messages the icon is removed from the system tray. If you mouse over the icon in the system tray you get the following text "username has # new messages" If multiple accounts have new mail, the tooltip text shows multiple lines of information (one account per line). Up to 128 characters. Windows only allows a tooltip to have 128 characters in it. If you have many accounts with new mail, we'll show as many accounts as will comfortably fit in the tooltip without causing truncation. This is just the first step in our efforts to improve new mail notification. This feature is not done yet nor is it finished. Patch coming up.
setting Sheela as the QA contact. I know there are other bugs out there which kind of talk about the same thing but I didn't want to go there.
Status: NEW → ASSIGNED
Keywords: nsbeta1
QA Contact: esther → sheelar
Target Milestone: --- → mozilla0.9.8
I posted 2 patches. The first patch shows the changes I made to nsMessengerWinIntegration to add support of a system tray icon when biff goes off. Most of the code is localization related stuff to properly con together the tooltip text. We register ourselves as a property flag listener on each root folder. That allows us to get new mail notifications from the folder. We keep an array of weak references to each root folder containing new mail. The tooltip info is generated by iterating over all of these folders and getting the number of new unread messages for each root folder. A separate line of tooltip data is generated for each one. The icon is removed from the system tray when one of the folders says it no longer has new mail (implying the user took action and has read one of the messages). The 2nd part of the patch was a change to nsIMsgFolder::GetNumNewMessages. I added a boolean to make it deep so I could ask the root folder how many new messages this account has. I did not post the patch which shows the following msgBiffIcon.ico // the windows ico file for the icon going into the system tray mail.rc // the resource file which bundles the ico file and gets bundled // into msgbase.dll resource.h // contains the resource ID for the mail biff icon used by // mail.rc
1. You might want to remove + //mBiffIconData.uTimeout = 10; +// mBiffIconData.szInfo = "You've Got mail"; + //mBiffIconData.szInfoTitle = "New Mail Notification"; +// mBiffIconData.dwInfoFlags = NIIF_INFO; from the first patch. 2. Also in the WinIntegration's Init() routine, http://lxr.mozilla.org/seamonkey/source/mailnews/base/src/nsMessengerWinIntegration.cpp#118 we have code that bails out of whole operation, if we detect the timer pref value is negative. 132 if (timerInterval <= 0) { 133 return NS_OK; 134 } Though timer pref;s (mail.windows_xp_integration.unread_count_interval) default value is set to be 5 minutes (http://lxr.mozilla.org/seamonkey/source/modules/libpref/src/win/winpref.js#173), it is possible for some ome to customize this to be a negative value. Then the we simply don't anything on biff front also. So, I think we need couple of changes like, mIntervalTime = 0; (in the constructor) then in Init(), after getting the pref timerInterval (at line 130), if (timerInterval > 0) mIntervalTime = timerInterval * 1000; and later, alter a line in your patch from + if (mSHSetUnreadMailCount && mSHEnumerateUnreadMailAccounts) + mStoreUnreadCounts = PR_TRUE; to + if (mSHSetUnreadMailCount && mSHEnumerateUnreadMailAccounts && mIntervalTime > 0) + mStoreUnreadCounts = PR_TRUE; Does that sound right..? It would have been nicer if we had left a mailbox-with-no-flag instead of completely removing it. When we get nicer icons, I guess we should do that. Finally, system tray notification is a reality. Great & Cool..!! Besides the comments above, patch looks good. r=bhuvan.
I meant patches. Besides my comments, both patches look good. r=bhuvan.
+'ing with enthusiasm.
Keywords: nsbeta1nsbeta1+
Priority: -- → P1
Attachment #64391 - Attachment is obsolete: true
Bhuvan, I left the commented out code in there because those are the fields used on win2k and winxp to use balloon tooltips. Once I figure out how to incorporate those into the build I'm going to use them. I want to leave them for reference reasons right now since this is still a work in progress. Made the other changes you suggested in this patch.
Comment on attachment 64392 [details] [diff] [review] 2nd part of the patch: add a deep version of GetNumNewMessages to nsIMsgFolder r=bhuvan (I had an r=bhuvan on this)
Attachment #64392 - Flags: review+
Comment on attachment 64886 [details] [diff] [review] updated nsMessengerWinIntegration files with Bhuvan's feedback. I had an r=bhuvan on this
Attachment #64886 - Flags: review+
We'll land this in 0.9.9. I didn't want to check it in right before the 098 deadline in case there are problems with the behavior.
Target Milestone: mozilla0.9.8 → mozilla0.9.9
In the win integration patch I removed the extra field I had added to the the mail session folder listener. We were already getting the notification from the root folder I didn't need to get it from the mail session as well. So the following part of the diff is no longer present. I also removed the commented out lines I was using to get balloon tooltips working (which is not part of this patch) - rv = mailSession->AddFolderListener(this, nsIFolderListener::boolPropertyChanged | nsIFolderListener::intPropertyChanged); + rv = mailSession->AddFolderListener(this, nsIFolderListener::boolPropertyChanged | nsIFolderListener::intPropertyChanged | nsIFolderListener::propertyFlagChanged);
Comment on attachment 64392 [details] [diff] [review] 2nd part of the patch: add a deep version of GetNumNewMessages to nsIMsgFolder sr=sspitzer for the first patch.
Attachment #64392 - Flags: superreview+
Comment on attachment 64886 [details] [diff] [review] updated nsMessengerWinIntegration files with Bhuvan's feedback. looks good. I've got a few nits, the only non-nit is the comment containing "msft APIs are wstring friendly" sr=sspitzer once you address them. nsMessengerWinIntegration is a service, so why not make this atom a member variable nsCOMPtr<nsIAtom> mBiffStateAtom; >+ kBiffStateAtom = NS_NewAtom("BiffState"); mBiffStateAtom = getter_AddRefs(NS_NewAtom("BiffState")); >+nsIAtom * nsMessengerWinIntegration::kBiffStateAtom = nsnull; >+ NS_IF_RELEASE(kBiffStateAtom); can be removed >+ nsCOMPtr<nsIAppShellService> appService = do_GetService( "@mozilla.org/appshell/appShellService;1", &rv); >+ if (NS_FAILED(rv)|| (!appService) ) return; You don't need to check appService, as do_GetService() won't return NS_OK without appService being non-null; >+ //mBiffIconData.uTimeout = 10; >+// mBiffIconData.szInfo = "You've Got mail"; >+ //mBiffIconData.szInfoTitle = "New Mail Notification"; >+// mBiffIconData.dwInfoFlags = NIIF_INFO; should these comments be removed? > // return if the timer value is negative or ZERO >- if (timerInterval <= 0) { >- return NS_OK; >- } >- else { >+ if (timerInterval > 0) >+ { the logic is different here, and my old eyes can't tell if you've preserved the behaviour when timerInterval is <= 0 (I think you did, but just double check) > // because we care if the unread total count changes >- rv = mailSession->AddFolderListener(this, nsIFolderListener::boolPropertyChanged | nsIFolderListener::intPropertyChanged); >+ rv = mailSession->AddFolderListener(this, nsIFolderListener::boolPropertyChanged | nsIFolderListener::intPropertyChanged | nsIFolderListener::propertyFlagChanged); the comment should be updated to reflected that we care about unread and new, right? >+nsresult nsMessengerWinIntegration::GetStringBundle(nsIStringBundle **aBundle) >+{ >+ nsresult rv = NS_OK; you don't need to initialize rv (brendan's gives me flack for that) >+ *aBundle = bundle; >+ NS_IF_ADDREF(*aBundle); instead do the old dmose trick: NS_IF_ADDREF(*aBundle = bundle); >+ nsXPIDLString finalText; >+ bundle->FormatStringFromName(NS_ConvertASCIItoUCS2("biffNotification").get(), formatStrings, 3, getter_Copies(finalText)); since "biffNotification" is a constant, do NS_LITERAL_STRING("biffNotification").get(), so that the conversion happens at compile time. >+ >+ asciiFinalText.AssignWithConversion(finalText); since the format comes from a .properties file, isn't this going to be a problem in localized builds? usually, the msft APIs are wstring friendly. or am I missing something? >+ nsresult rv = NS_OK; initialization not needed >+ if (!mStoreUnreadCounts) return NS_OK; // don't do anything here if we aren't storing unread counts... for debugging sake, do waterson nit so you can set a breakpoint on the return; if (!mStoreUnreadCounts) return NS_OK; // don't do anything here if we aren't storing unread counts... >+ if (!mStoreUnreadCounts) return NS_OK; // don't do anything here if we aren't storing unread counts... waterson nit. >+ if (!mStoreUnreadCounts) return NS_OK; // don't do anything here if we aren't storing unread counts... waterson nit. >+ if (!mStoreUnreadCounts) return NS_OK; // don't do anything here if we aren't storing unread counts... waterson nit. >+ if (!mStoreUnreadCounts) return NS_OK; // don't do anything here if we aren't storing unread counts... waterson nit. >+ if (!mStoreUnreadCounts) return NS_OK; // don't do anything here if we aren't storing unread counts... waterson nit.
Attachment #64886 - Flags: superreview+
I just checked this in after incorporating Seth's changes (including using a different shell structure called: NOTIFYICONDATAW which takes a wide string. I already have a separate bug on the UE group for getting a final icon for biff.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Not seeing this working on build 2002012605 in win98. Its not really intended only for win2k as the OS for the bug might indicate is it?
Doesn't work for me either, Win98SE, mozilla trunk builds 20020126, 20020127 and 20020128
I am seeing this for win2000 and win98 win 20020129, But the icon is not clickable to run mail.
2002-01-29-06 build on win98. Mail Notification in system tray is not working on win98. See bug that has been logged by mscott # 122254 which is related and which is spun off of bug bug # 122228
Verifying after testing the following: Tested on imap account, POP account, webmail, aol account in a single profile Tested on each account as default account in a single profile Works when biff goes off as well as when user clicks on get message button. Works with Get all messages on multiple accounts as well. Using build: 2002-01-29-06 WinNT-mailnotification icon appears. Disappears when mouse over- See bug #122351 Win2k-works fine winXP-works fine Mail notification does not work on the following version. This could be resolved when bug 122351 gets resolved. win95 winME win98 Verifying this bug since the other platforms are tracked on a different bug as mentioned above.
Status: RESOLVED → VERIFIED
This bug does not appear to be fixed for me, WIN98 using build 2002020803. Why is it closed if it still is not working for WinME, WIn95, and Win98? I could understand a dependency on some other bug, but the one mentioned in comment #20 says it is about a seemingly unrelated problem.
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: