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

VERIFIED FIXED

Status

SeaMonkey
MailNews: Message Display
VERIFIED FIXED
16 years ago
10 years ago

People

(Reporter: dylang, Assigned: Scott MacGregor)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

16 years ago
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

16 years ago
Should this be two separate bugs?

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

Comment 2

16 years ago
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.

Updated

16 years ago
Summary: New mail indicator out to lunch sometimes. → New mail indicator does not work for non

Comment 3

16 years ago
I will attach an initial patch soon.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: New mail indicator does not work for non → New mail indicator does not work for non-inbox imap folders

Comment 4

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

Comment 5

16 years ago
Created attachment 85806 [details] [diff] [review]
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

16 years ago
looks roughly ok to me.
Keywords: patch, review

Comment 7

16 years ago
David,

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

Updated

16 years ago
Attachment #85806 - Flags: review+

Comment 8

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

r=bienvenu
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=brendan@mozilla.org if you fix these minor issues.

/be
Attachment #85806 - Flags: superreview+

Comment 10

16 years ago
Created attachment 87790 [details] [diff] [review]
v1.1 patch to make notification work for all checked folders

Fixed as per comments from Brendan

Comment 11

16 years ago
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
Attachment #87790 - Flags: superreview+
Attachment #87790 - Flags: review+

Updated

16 years ago
Attachment #85806 - Attachment is obsolete: true

Comment 12

16 years ago
Fix checked in trunk.
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
(Assignee)

Comment 13

16 years ago
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.
OS: Linux → All
Hardware: PC → All
*** Bug 165838 has been marked as a duplicate of this bug. ***
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
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey

Updated

10 years ago
Component: MailNews: Notification → MailNews: Message Display
QA Contact: stephend → search
You need to log in before you can comment on or make changes to this bug.