Closed Bug 2087812 Opened 13 days ago Closed 9 days ago

IMAP: Thunderbird sends CREATE for an existing Trash folder on every folder discovery when the personal namespace has a prefix (e.g. "INBOX.")

Categories

(MailNews Core :: Networking: IMAP, defect)

defect

Tracking

(thunderbird_esr153 wontfix, thunderbird158 wontfix, thunderbird159 wontfix, thunderbird160 wontfix, thunderbird161 affected)

RESOLVED FIXED
161 Branch
Tracking Status
thunderbird_esr153 --- wontfix
thunderbird158 --- wontfix
thunderbird159 --- wontfix
thunderbird160 --- wontfix
thunderbird161 --- affected

People

(Reporter: olav, Assigned: welpy-cw)

Details

Attachments

(1 external file)

Steps to reproduce

  1. Use an IMAP server with personal namespace (("INBOX." ".")) whose Trash folder INBOX.Trash exists, is subscribed, and is not flagged \Trash (no SPECIAL-USE), e.g. Courier-style servers.
  2. Keep the default mail.server.serverN.trash_folder_name = Trash and "Move it to the Trash folder" on delete.
  3. Start Thunderbird (or expand the account) and look at the IMAP log.

Actual results

Discovery lists INBOX.Trash, but Thunderbird still sends create "INBOX.Trash" on every start. The server answers NO mailbox already exists, and the discovery URL fails with 0x80550021:

list "" "INBOX.Trash"
* LIST () "." "INBOX.Trash"
create "INBOX.Trash"
NO mailbox already exists
URL failed with code 0x80550021 (imap://…/discoverallboxes)

Seen with the official 159.0 build (Linux) against a local test server, on three consecutive starts, and with a comm-central build of 2026-10-19 in an xpcshell test.

Analysis

In nsImapProtocol::DiscoverMailboxSpec(), the Trash folder is only recognized if the canonical mailbox name occurs in m_trashFolderPath:

CaseInsensitiveFindInReadable(m_trashFolderPath, adoptedBoxSpec->mAllocatedPathName)

m_trashFolderPath is the pref value Trash, but the canonical name keeps the namespace prefix: INBOX/Trash. INBOX/Trash never occurs in Trash, so the online Trash folder is not marked as existing, and MailboxDiscoveryFinished() tries to create it. Only the failed CREATE then sets SetOnlineTrashFolderExistsForHost(true), but the URL is already marked as failed. The \Trash flag check just before this comparison would cover servers with SPECIAL-USE.

Bug 1779543 (TB 102) shows exactly this sequence in its log (comment 5 and 6). It was closed WORKSFORME because the reporter's UI symptom disappeared, but the CREATE was never addressed. Bug 1335982, bug 1428666 and bug 491424 fixed related Trash detection problems with an INBOX. namespace, but not this comparison.

Bug 519797 was a similar case for Gmail.

No data loss observed. It costs one round trip per discovery and reports a failure for a discovery that actually succeeded.

Expected results

No CREATE when the Trash folder was just discovered. Discovery should succeed.

Severity: -- → S3
Status: UNCONFIRMED → NEW
Ever confirmed: true

Resolve relative Trash paths using the server and canonical path conversions
before comparing them with discovered mailboxes. Refresh the Trash existence
state after the fallback LIST so an existing folder is not created again.

Add regression coverage for namespace prefixes, custom paths, subscriptions,
server directories, non-ASCII names, SPECIAL-USE flags, and missing Trash.

Assignee: nobody → h.w.forms
Status: NEW → ASSIGNED

Review for D338704 (Phabricator refuses comments and accepts from my account; edit policy is a restricted project, presumably editbugs):

Thanks, this works. With only the test applied, discovery fails with 0x80550021; with the fix, the test passes and the whole IMAP xpcshell suite passes (206/206). A few nits:

  1. The first hunk drops the if (m_hostSessionList) guard that the same block still uses a few lines above. Either keep it or drop it consistently.
  2. Without the CaseInsensitiveFindInReadable pre-check, AllocateServerPath() and AllocateCanonicalPath() (each with a do_GetService) now run for every listed mailbox until Trash is found. That covers all of them on servers without Trash. Could the canonical Trash path be computed once per discovery?
  3. In the test, the SELECT check in the second round always passes: playTransaction() returns the whole transaction of the single connection, so the first round's SELECT already matches. Counting the SELECTs, or calling resetTest() between rounds, would make the second check meaningful.

From my side it is r+ with these nits.

(In reply to Olav Seyfarth from comment #2)

Review for D338704 (Phabricator refuses comments and accepts from my account; edit policy is a restricted project, presumably editbugs):

Thanks! Please request editbugs permission for yourself, see https://bugzilla.mozilla.org/page.cgi?id=get_permissions.html.

Thanks, the updated diff (1424642) addresses all three points: the guards are dropped consistently, the canonical Trash path is cached per URL and discovery, and the test now counts the SELECTs with forceSelect. Rebuilt and ran the IMAP xpcshell suite locally: 206/206 pass. r+ from me.

Pushed by daniel@thunderbird.net:
https://hg.mozilla.org/comm-central/rev/a95e750a6301
Recognize existing IMAP Trash folders with namespace prefixes. r=nursoda,mkmelin

Status: ASSIGNED → RESOLVED
Closed: 9 days ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: