146559 - New mail indicator does not work for non-inbox imap folders

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[dylang]

Now that bug 18266 is landed, I've been using this feature.  However, unless I
have new mail in the INBOX specifically, the new mail indicator won't go "you
have new mail" properly.

I've also noticed that between several browser windows all from the same process
that Mozilla will have different flags shown (perhaps have all windows sync from
same var is not happening correctly?).

Comment 1

[Joonas]

Should this be two separate bugs?

Reporter: please consider using bugzilla helper to file bugs. Steps to reproduce
would be useful.

Comment 2

[dylang]

Steps to reproduce:
 Have some NS windows open.
 Have mail arrive to yourself.
 Open some new ones.
 Note that the indicator on the new ones is incorrect.

Or, if you want the "no subfolder detection" bug, just have new mail sen to the
subfolders.  The new IMAP code will find it, but the littl flag won't be updated
correctly.

Comment 3

[Ere Maijala (slow)]

I will attach an initial patch soon.

Comment 4

[Ere Maijala (slow)]

Then again, not today. (my patch doesn't work in all situations)

Comment 5

[Ere Maijala (slow)]

Attachment: v1 patch to make notification work for all checked folders

This is a patch to make new mail notification work for all folders. It ensures
that nsImapIncomingServer knows that biff is being performing. Biff status is
set for each nsImapMailFolder when biff is triggered.

Comment 6

[David :Bienvenu]

looks roughly ok to me.

Comment 7

[Ere Maijala (slow)]

David,

If it's ok, could you r= it? Thanks.

Comment 8

[David :Bienvenu]

Comment on attachment 85806 [details] [diff] [review]
v1 patch to make notification work for all checked folders

r=bienvenu

Comment 9

[Brendan Eich [:brendan]]

Comment on attachment 85806 [details] [diff] [review]
v1 patch to make notification work for all checked folders

>@@ -3587,8 +3588,16 @@
>   aRootFolder->GetFlags(&flags);
>   if ((forceAllFolders && !(flags & MSG_FOLDER_FLAG_INBOX)) || (flags & MSG_FOLDER_FLAG_CHECK_NEW))
>   {
>-  // Get new messages for this folder. 
>-  aRootFolder->UpdateFolder(aWindow);
>+    // Get new messages for this folder.
>+    aRootFolder->SetGettingNewMessages(PR_TRUE);
>+    if (performingBiff)
>+    {
>+      nsresult rv;
>+      nsCOMPtr<nsIMsgImapMailFolder> imapFolder = do_QueryInterface(aRootFolder, &rv);
>+      if (NS_SUCCEEDED(rv) && imapFolder)

No need for rv here, just null-test imapFolder after do_QI.

>@@ -2165,8 +2166,15 @@
>     nsCOMPtr<nsIImapIncomingServer> imapServer;
>     rv = GetImapIncomingServer(getter_AddRefs(imapServer));
>  
>+    PRBool performingBiff = PR_FALSE;
>+
>     if (NS_SUCCEEDED(rv) && imapServer)

Existing check of both rv and imapServer, could simplify?

>+    {
>       imapServer->GetDownloadBodiesOnGetNewMail(&m_downloadingFolderForOfflineUse);
>+      nsCOMPtr<nsIMsgIncomingServer> incomingServer = do_QueryInterface(imapServer, &rv);
>+      if (NS_SUCCEEDED(rv) && incomingServer)

Ditto first comment above.

>@@ -2190,13 +2198,17 @@
>       rv = rootFolder->GetFoldersWithFlag(MSG_FOLDER_FLAG_INBOX, 1, &numFolders, getter_AddRefs(inbox));
>       if (inbox)

Note existing code doesn't test NS_FAILED(rv) here, only inbox.  That's cool if
the interface guarantees success means non-null out param.

>       {
>+        nsCOMPtr<nsIMsgImapMailFolder> imapFolder = do_QueryInterface(inbox, &rv);
>+        if (NS_SUCCEEDED(rv) && imapFolder)

Ditto.

sr=[e-mail] if you fix these minor issues.

/be

Comment 10

[Ere Maijala (slow)]

Attachment: v1.1 patch to make notification work for all checked folders

Fixed as per comments from Brendan

Comment 11

[Ere Maijala (slow)]

Comment on attachment 87790 [details] [diff] [review]
v1.1 patch to make notification work for all checked folders

Carrying over r= and sr= from previous version

Comment 12

[Ere Maijala (slow)]

Fix checked in trunk.

Comment 13

[Scott MacGregor]

This bug actually introduced a regression. Now, every time the user hits the get
new mail button we are setting the "performing biff" flag. This causes our biff
notifications (animated alert & system tray icon) on windows to show up! Please
see Bug #153500 which tracks this regression.

Comment 14

[[e-mail] (gone - use [e-mail] instead)]

*** Bug 165838 has been marked as a duplicate of this bug. ***

Comment 15

[[e-mail] (gone - use [e-mail] instead)]

Testcase:

1) Enable the folder checking via either Edit | Folder Properties or
context-clicking the folder name in the Folder Pane.  Select the checkbox 'Check
this folder for new messages'.
2) Change biff (new mail notification) to perform at 1-minute intervals.  You'll
have to restart for the new setting to take place.  (See bug 24594.)
3) Setup a mail filter which sends mail to a designated folder. (I sent all
subject of 'test' -> 'Test Messages' folder.
4) Send a test message which matches the criteria set in 3).
5) Wait the specified interval set in 2).
6) If you're on a Windows-based platform, we'll (by default) show an alert in
the system tray. On all platforms, we'll bold the folder, place a (1) after the
folder name, change the folder and server (account) icons to include green
arrows for each.  

Tested with the following:

Mac OS X 10.2.3 - 2003-01-02-08
Windows 2000 - 2003-01-02-05
RedHat 8.0 Linux - 2003-01-02-08

Verified FIXED