Closed Bug 238680 Opened 17 years ago Closed 17 years ago

On a heavily filtered inbox, "mark as read" generates needlessly long IMAP commands

Categories

(MailNews Core :: Networking: IMAP, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: rvassar, Assigned: Bienvenu)

References

Details

Attachments

(2 files)

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.
fixed .
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Blocks: 240476
fixed on the tbird 0.6 branch
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.