Closed Bug 1773605 Opened 2 years ago Closed 2 years ago

Moving a single message between IMAP folders is very slow / requires reloading the origin folder

Categories

(MailNews Core :: Networking: IMAP, defect)

Thunderbird 102
defect

Tracking

(thunderbird102+ fixed)

RESOLVED FIXED
103 Branch
Tracking Status
thunderbird102 + fixed

People

(Reporter: sancus, Assigned: gds)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(2 files)

In Thunderbird 102, when moving a single message from one IMAP folder to another, it appears to take 10-15 seconds or more for the message to appear in the destination folder.

Additionally, it seems as though the message never appears unless you manually refresh the folder.

In Thunderbird 91, these moves are consistently done in 1 second or less, and the folder consistently refreshes itself and shows the message even if you view the folder slightly before the move is complete.

This is a major regression in feel and makes message moving feel unreliable and very janky.

Tested on 102.0b4 and 91.10 on Windows 10.

Here is a video, the first section is 91, the second section is 102, as labeled by chapters.

Summary: Moving a single message between IMAP folders is very slow → Moving a single message between IMAP folders is very slow / requires reloading the origin folder

After further testing, it seems that clicking on the ORIGIN folder and then clicking back to the TARGET again makes the move complete properly in a shorter time period.

I noticed this too while testing another bug today. I was wondering if it might be something I messed up like this: Bug 1747173. Preliminary test with this backed out still seeing what you see. I'll look closer at it tomorrow.

Component: Database → Networking: IMAP
Blocks: tb102found

I do wonder, perhaps this is indeed the bug that would cause bug 1742975 to show. That could itself be some old bug, that's now being much more visible due to this slowness... changed timings.

See Also: → 1742975

I think this bug and Bug 1772550 are duplicates. I narrowed the cause down to starting at beta 101.0b1. I was intending to look closer at this bug but got sidetracked on another bug. I'll work on this again today.

This move bug is caused by this: Bug 1767005 and appeared on daily build for 4-30-2022 and at beta 101.0b1. The timer used for copying/moving messages in nsImapMailFolder.cpp wasn't always occurring. Reverting the timer handling in nsImapMailFolder.cpp to the old/original way fixes the problem but I will attach a patch to fix the problem using the new timer method.
I have doubts that this affects bug 1742975 since that bug occurred well before 4-30-2022.

Regressed by: 1767005
Assignee: nobody → gds
Status: NEW → ASSIGNED

I tested the patch on Windows 10 and it definitely resolves the problem for me.

Severity: -- → S2
Target Milestone: --- → 103 Branch
OS: Windows 10 → All
Hardware: x86_64 → All

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/035c3d3a0d97
[Regression] Fix the folder copy playback timer handling r=benc

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED

There are some debug test failures from this:

TEST-UNEXPECTED-FAIL | xpcshell.ini:comm/mailnews/imap/test/unit/test_imapHighWater.js | xpcshell return code: -11
Hit MOZ_CRASH([Parent 8132, Main Thread] ###!!! ASSERTION: expected null: '!srcImapFolder->m_playbackTimer', file /builds/worker/checkouts/gecko/comm/mailnews/imap/src/nsImapMailFolder.cpp:6904) at /builds/worker/checkouts/gecko/xpcom/base/nsDebugImpl.cpp:517

TEST-UNEXPECTED-FAIL | xpcshell.ini:comm/mailnews/imap/test/unit/test_offlineCopy.js | xpcshell return code: -11
Hit MOZ_CRASH([Parent 8629, Main Thread] ###!!! ASSERTION: expected null: '!srcImapFolder->m_playbackTimer', file /builds/worker/checkouts/gecko/comm/mailnews/imap/src/nsImapMailFolder.cpp:6904) at /builds/worker/checkouts/gecko/xpcom/base/nsDebugImpl.cpp:517

Flags: needinfo?(gds)

Re: comment 11
I Replaced NS_ASSERTION with timer Cancel() if it has not yet timed out. Not sure if this is really needed but similar thing done for timer for purge and nntp:
https://searchfox.org/comm-central/rev/1e23367b672ba053968ea2741cf98df8a53f9d7c/mailnews/base/src/nsMsgPurgeService.cpp#101
https://searchfox.org/comm-central/rev/1e23367b672ba053968ea2741cf98df8a53f9d7c/mailnews/news/src/nsNntpIncomingServer.cpp#245

Also, now see that the callback function PlaybackTimerCallback() is static so removed the srcImapFolder-> in front so now it is like it originally was.

Flags: needinfo?(gds)

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/797fe99ec4a4
Remove assert to fix test failures. r=mkmelin

Comment on attachment 9280819 [details]
Bug 1773605 - [Regression] Fix the folder copy playback timer handling r=benc

[Approval Request Comment]
Regression caused by (bug #):
Bug 1767005

User impact if declined:
Manual file moves feel almost broken without this patch.

Testing completed (on c-c, etc.):
Risk to taking this patch (and alternatives if risky):
Similar risk profile to msf corruption fix, seems like.

Attachment #9280819 - Flags: approval-comm-beta?
Attachment #9281709 - Flags: approval-comm-beta?

Comment on attachment 9280819 [details]
Bug 1773605 - [Regression] Fix the folder copy playback timer handling r=benc

[Triage Comment]
Approved for beta

Attachment #9280819 - Flags: approval-comm-beta? → approval-comm-beta+

Comment on attachment 9281709 [details]
Bug 1773605 - Remove assert to fix test failures. r=mkmelin

[Triage Comment]
Approved for beta

Attachment #9281709 - Flags: approval-comm-beta? → approval-comm-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: