User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US) AppleWebKit/533.4 (KHTML, like Gecko) Chrome/5.0.375.70 Safari/533.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:188.8.131.52) Gecko/20100608 Thunderbird/3.1
Undo after a delete reports an error from the IMAP UID command:
Error in IMAP command UID: Invalid UID messageset.
Steps to Reproduce:
. Select a message in INBOX or other folder (except Trash)
2. Delete the message.
3. Do Undo (^Z)
4. A notification window rolls up, saying (for example):
"The current operation on 'Inbox' did not succeed. The mail server for account firstname.lastname@example.org responded: Error in IMAP command UID: Invalid UID messageset."
My IMAP server is secure.emailsrvr.com, which is Rackspace's commercial Noteworthy IMAP service.
I could give you a test account on that service if you'd like; contact me.
Created attachment 450960 [details]
imap.log showing the error (search for "Error")
This error happens in 3.1RC2; operating on the same mailbox in 3.0.4 does not cause the problem.
This is reminiscent of bug 569588 (same initial scenario), but is not a crash.
Dan can you find out when this regressed ?
(In reply to comment #4)
> Dan can you find out when this regressed ?
I tried rc1, and then bisected. I ended up testing all the releases:
3.1b2 error as reported
3.1rc1 error as reported
3.1rc2 error as reported
** ALSO, I accidentally confirmed the crash in bug 569588 while testing. As that bug says, the following scenario causes a crash:
1. Delete message
2. QUICKLY do undo (^Z)
In this scenario, I did not see the IMAP UID error notification. However, I was able to reliably reproduce the bug 569588 crash all the way back to Thunderbird 3.0.2, so it is not clear how/if it is related to this IMAP error bug. The IMAP error bug happens when the undo is not done quite so quickly (perhaps there is time for a round-trip to the IMAP server).
Dan can you try to finegrain the bisect using nightlies so we end up with a shorter regression window ?
(In reply to comment #6)
> Dan can you try to finegrain the bisect using nightlies so we end up with a
> shorter regression window ?
Sure. I assume you mean the nightlies here:
but which branch should I be testing? There's -comm-1.92, comm-1.9.2-i10n, etc. Thanks.
I will be able to do this some time this evening Eastern time.
Yes -comm-1.9.2 is the one to use.
I was able to reproduce this - we're using the message id and not the uid in the command to clear the deleted flag. So someone (probably me) got confused about what a string contained...
(In reply to comment #9)
> I was able to reproduce this ...
Would bisecting still be helpful? I was just about to start doing that.
(In reply to comment #10)
> Would bisecting still be helpful? I was just about to start doing that.
It might be helpful, but it might not be needed either. I'll spend a few minutes investigating and let you know if I get stumped.
I think it was the patch in bug 525646 that caused the regression. bisection won't be helpful at this point, but thanks for offering.
Created attachment 451184 [details] [diff] [review]
this fixes it - running the unit tests to make sure it doesn't regress anything. But they passed w/ the bug in :-)
I think what's happening here is that the regression was from a long time ago, but we never saw a ui manifestation of it because the undo runs w/o a message window. Thus, the error from the server was not displayed. But now that we have a non-modal alert mechanism, even urls run without a msgWindow cause the error to get displayed to the user. It's unfortunate that this is in 3.1, but it's probably not worth respinning for.
Some imap servers may not generate an alert in this case, since they might just ignore bad UID's in the STORE command.
Might the "response" be different depending on server? With undo I get
The current operation on <folder> did not succeed. The server for account <name> responded: Bogus sequence in UID FETCH: UID sequence syntax error.
forgot to mention...in 3.1 (not sure about 3.2) this causes this UI to freeze for a few seconds
I need to figure out why our unit test didn't catch this, other than the fact that the undo actually works, despite the error.
fakeserver ignores invalid uids, which it should not, I don't think, so I'll have to patch fakeserver.
Created attachment 452145 [details] [diff] [review]
fix to server and test case to catch this error
marking blocking 3.1.1
(In reply to comment #14)
> But now that we have
> a non-modal alert mechanism, even urls run without a msgWindow cause the error
> to get displayed to the user.
You're right, I couldn't understand it at first, but yes, I did miss that when we swapped to non-modal.
fixed on trunk
Adding text in bug summary, for ease of tracking and search.
(In reply to comment #23)
> Adding text in bug summary, for ease of tracking and search.
Sorry but I really don't think we need to know which specific versions in the summary, especially when branch statuses are governed by existing fields.
David: is this ready for 3.1 branch?
Comment on attachment 451184 [details] [diff] [review]
yes, I think this is ready for 3.1.1
Checked into comm-1.9.2 and relbranch for 3.1.1 build 2: