Closed Bug 238680 Opened 17 years ago Closed 17 years ago
On a heavily filtered inbox, "mark as read" generates needlessly long IMAP commands
User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7b) Gecko/20040316 Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7b) Gecko/20040316 Using recent releases of SunONE messaging server (6.0 patched), with a large number of client (Moz only) side mail filters (30% - 50% on incoming messages sorted to folders), I've experienced some problems marking messages as read in my inbox. I typically have 20 to 100 unread messages in an inbox of roughly 3000 messages. It appears the filtering/foldering is chewing large holes in the message UID space. When I go to mark the remain messages as read, it typically generates a single IMAP command nearly 20K bytes in length. The command includes large ranges of UID's that very clearly have the "Seen" flag stored already. The Sun mail server currently limits IMAP commands to 8K. I'm not sure if there's a RFC specified limit. Regardless, Moz appears to be getting confused about which messages need to have the Seen flag stored, and which alreday have it stored. Having 10 unread messages should generate a nice short flag store with 10 UID's. I may be able to provide server side telemetry of the IMAP transactions. I first noticed this on Moz 1.6 MacOS X, which returned an error dialog. 1.7b MacOS X fails silently. The flag store not being apparent until the inbox is reselected. Reproducible: Always Steps to Reproduce: 1. 2. 3.
How are you marking the messages read? Select all - mark read? Or Mark all read? I fixed this bug for fetching headers, but not for storing flags, by using the flag state (that's the object that knows what the existing message keys in the folder are)
Status: UNCONFIRMED → NEW
Ever confirmed: true
I reproduce this by right-clicking my inbox in the folder panel, and selecting "mark as read".
this makes it so we only try to store the /SEEN flag on messages we think are unread.
Comment on attachment 144852 [details] [diff] [review] only store /SEEN flag on messages we think are read also removed unused numChanged var.
Attachment #144852 - Flags: superreview?(mscott)
Status: NEW → ASSIGNED
OS: MacOS X → All
Hardware: Macintosh → All
Attachment #144852 - Flags: superreview?(mscott) → superreview+
This fixes the store command to both coalesce ranges using the flag state, and to limit store command lines to 950 bytes. Along the way, I found a bug in the code that limits the command line length in that it didn't correctly say how many entries had been handled - this wasn't a problem for fetch headers, usually, because headers tend to get fetched in ranges anyway, but it could be an issue for downloading for offline use...
Comment on attachment 144861 [details] [diff] [review] imap part of proposed fix The 970 byte command line length limit is somewhat arbitrary - I don't think the RFC says anything about long lines, though I could have sworn it did.
Attachment #144861 - Flags: superreview?(mscott)
Comment on attachment 144861 [details] [diff] [review] imap part of proposed fix will this trailing semi colon cause compiler problems for unix? for (const char *curCharPtr = uidString; curChar && *curCharPtr;) Should messageIdList and idString be nsCAutoStrings? or are they typically going to be larger than 64 character strings?
Attachment #144861 - Flags: superreview?(mscott) → superreview+
that code just moved from nsImapMailFolder.cpp to nsImapUtils.cpp so it should compile fine. It's not actually a trailing ';' - there's just no third clause to the for statement re autostrings - I thought about that, and didn't do it because the bug case has long strings, but I should have made them auto strings because the typical case will have short strings. I'll fix it locally.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
fixed on the tbird 0.6 branch
You need to log in before you can comment on or make changes to this bug.