1959545 - Dragging a folder to a new parent doesn't insert at the expected position for IMAP folders

Status: RESOLVED FIXED

(Thunderbird :: Folder and Message Lists, defect, P2)

Description

[betterbird.project+18]

Attachment: re-order-video.mp4

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/135.0.0.0 Safari/537.36 Edg/135.0.0.0

Steps to reproduce:

Dragging a folder to a new parent doesn't insert at the expected position, see video.
Maybe this patch for the previous version of the re-order functionality fixes this.

Comment 1

[itagagaki]

Attachment: 139.a1.2025-04-04-10-03-46-comm-central-l10n.mp4

139.0a1 nightly 2025-04-04-10-03-46-comm-central-l10n

Comment 2

[itagagaki]

139.0a1 nightly 2025-04-04-10-03-46-comm-central-l10n is works fine as attached.
No need the patch for the previous version. See what ReorderFolderListener does.
https://hg-edge.mozilla.org/comm-central/rev/61cd87a2f6d0121a6784303b7c6b94e0e11939fc

Comment 3

[Magnus Melin [:mkmelin]]

So WORKSFORME?

Comment 4

[betterbird.project+18]

Doesn't work in 139.0a1 (2025-04-09) (64-bit), we didn't make up the video artificially.

Sorry, forgot to say that the failure is for IMAP folders only. There seems to be some sort of timing issue since the folder doesn't exist until it's communicated by the IMAP server (or similar). Likely the same root cause as bug 82262.

Comment 5

[itagagaki]

Reproduced.
IMAP in the folder pane is generally weird.
Various unexplained phenomena.
Not sure if this is related to bug 82262.
Will try to analyze the behavior reported in this bug.

Comment 6

[itagagaki]

Comment on attachment 9478172 [details]
re-order-video.mp4

These folders are under the IMAP server.

Comment 7

[itagagaki]

Comment on attachment 9478192 [details]
139.a1.2025-04-04-10-03-46-comm-central-l10n.mp4

These folders are under the Local Folders.

Comment 8

[itagagaki]

Okay, I've figured out why that happened.

I modified nsMsgCopyService::NotifyCompletion() to get the folder as a result of the move/copy operation.

For example, if the user drags and drops mailbox://nobody@Local%20Folders/test2/A to mailbox://nobody@Local%20Folders/test, the argument nsIMsgFolder* dstFolder will point to the folder
mailbox://nobody@Local%20Folders/test/A. (However, if A exists before the move/copy, the object will be named A(2), not A, after the dialog).
I wanted to keep this dstFolder to get it later with nsMsgCopyService::GetArrivedFolder().

The problem is that IMAP passes test instead of test/A as an argument dstFolder to this NotifyCompletion.

https://searchfox.org/comm-central/rev/9300037a76850ea56175b9ccc40142619a4f0b3c/mailnews/imap/src/nsImapMailFolder.cpp#5199-5202

if (srcFolder) {
  copyService->NotifyCompletion(m_copyState->m_srcSupport, this,
                                aExitCode);
}

Where this is the folder that received the drop.

This means that the NotifyCompletion interface is ambiguously defined, and one of them is misusing it.

What do you know about this?

Comment 9

[itagagaki]

The existing code for IMAP that I referenced in my previous comment needs to be modified to pass an object pointing to imap://user@host/test/A (in the example) instead of this (pointing to imap://user@host/test in the example) to NotifyCompletion().

However, I am not sure how to do this.

--
As a side note, if imap://user@host/test/A existed before the move, TB did not issue any dialog or error and did nothing with the move.
I think this should be improved.

Comment 10

[Corey Bryant]

Is this a regression?

Comment 11

[itagagaki]

(In reply to Corey Bryant from comment #10)

Is this a regression?

It is hard to call this a regression. It did not break anything conventional. The problem is with the conventional code.
But until it is fixed, it can be called a regression.

Comment 12

[itagagaki]

Okay, I have a countermeasure. I'll submit the patch tomorrow.

Comment 13

[itagagaki]

Attachment: Bug 1959545 - Dragging a folder to a new parent doesn't insert at the expected position for IMAP folders. r=#thunderbird-back-end-reviewers

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/0364f4954ff1
Dragging a folder to a new parent doesn't insert at the expected position for IMAP folders. r=mkmelin