Closed Bug 1156669 Opened 9 years ago Closed 9 years ago

Trash folder duplication while using IMAP with localized TB

Categories

(Thunderbird :: Folder and Message Lists, defect)

38 Branch
defect
Not set
major

Tracking

(thunderbird38+ fixed, thunderbird39 fixed, thunderbird40 fixed, thunderbird_esr31 unaffected, thunderbird_esr38 affected)

RESOLVED FIXED
Thunderbird 40.0
Tracking Status
thunderbird38 + fixed
thunderbird39 --- fixed
thunderbird40 --- fixed
thunderbird_esr31 --- unaffected
thunderbird_esr38 --- affected

People

(Reporter: m.ostrowski, Assigned: rkent)

References

Details

(Keywords: regression)

Attachments

(8 files)

Attached image after.png
User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:37.0) Gecko/20100101 Firefox/37.0
Build ID: 20150407092223

Steps to reproduce:

* Run polish version of TB 31.6.0. Configure imap account with defaults.
* Delete one e-mail from inbox - would be moved to folder "/Trash" on server, displayed as "Kosz" in UI.
* Close TB 31.6.

* Run polish version of TB 38b1.
* Delete one e-mail from inbox - would be moved to folder "/Trash" on server, displayed as "Kosz" in UI.
* Close Tb 38b1.

* Run polish version of TB 38b1.
* TB would now create "/Kosz" folder on server, would show two "Kosz" folders in UI.
* Delete one e-mail from inbox - would be moved to folder "/Trash" on server, displayed as "Kosz" in UI.
* Close Tb 38b1.

* Run polish version of TB 38b1.
* TB would now show both "Kosz" folders in UI.
* Delete one e-mail from inbox - would be moved to folder "/Kosz" on server, displayed as "Kosz" in UI.



Actual results:

* Two "Kosz" folders in UI. One "/Trash", one "/Kosz" on server (attachment after.png).
* "/Kosz" on server used as trash folder instead of "/Trash"



Expected results:

* Only "/Trash" folder on server, displayed as "Kosz" in UI.
* Compatibility of trash folder between different language versions of TB.

This problem is not a duplicate of bug #480393 because Tb 31.6.0 works fine. It is serious regression which would affect any non-english IMAP users.
Severity: normal → major
Keywords: regression
OS: Unspecified → All
Hardware: Unspecified → All
TB 38.0b2 is also affected.
I asked for help on tb-planning to confirm this bug.
Attached image TRASH.png
I have 2 IMAP mail accounts on 2 different servers. One is having this bug, other isn't.
Basicly, TB created trash folder with localized name "Smeće" even when "Trash" folder already exists.

I'm not sure which mail servers are used, is there a way to check it as user?
Only by checking the logs - https://wiki.mozilla.org/MailNews:Logging#Generating_a_Protocol_Log

Please check Advanced under Account Settings | Server Settings to see if namespace or server directory differs between the two accounts.
I tried to reproduce this with a Spanish version using an IMAP account which is hosted in a Spanish server.

The first problem I have, is that I can't configure TB to actually move anything to the trash folder. In Account Settings, Server Setting, "When I delete a message: Move to this folder (Choose Folder)" I can select a folder, but when I revisit the setting later, this folder was not stored. It might stick for a while, but when deleting the first message, the setting gets lost and the deleted messages is not moved to the trash folder.

Using the Spanish version I managed to create another inbox (Bandeja de entrada) containing another trash folder and switching back to English, I got a third inbox and trash folder.

I'd say this is pretty much broken.
Hmm, I'm having a bad time cleaning up the mess I created.
Very hard to get rid of the extra inboxes and their trash cans.
Hmm, the various trash folders all seem to be the same folder on the server, yet they have varying local storage:
imap://info%40hostalpancheta.es@mail.hostalpancheta.es/INBOX/Bandeja%20de%20entrada/Trash
imap://info%40hostalpancheta.es@mail.hostalpancheta.es/INBOX/INBOX/Trash
imap://info%40hostalpancheta.es@mail.hostalpancheta.es/INBOX/Trash

I haven't managed to get rid of them. I unsubscribed, deleted the folders on the disk. Refreshed the subscription list. They don't go away from the subscription list. How do I fix that?
Deleted the account in TB and deleted folders on the server (via Roundcube web interface) to clean up the mess.
I've been using TB38b4 for my tests.

After deleting the account from TB and cleaning up the server, I recreated the account.

Now everything works like it should. I don't get the "Trash" folder any more that can be seen on the attached screenshots. Instead I get "Papelera", the Spanish for "(waste) paper basket".
So it's all good again, or so it seems.

Going back to the English version however, I get an additional inbox "Bandeja de entrada" with a trash "Papelera". Back to Spanish, I end up with two inboxes and two trashes (see picture).

However, I never saw what is to be confirmed here, that there are two trash cans at the same level in the hierarchy.
Oops, I finally managed to create "Trash" and "Papelera" at the same time, however, only one has the trash icon. The one without the trash icon is the one that contains the previously deleted messages.

So there is certainly something wrong here.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Just checked my everyday profile. I only have one non-gmail imap account and sure enough I also have duplicated trash folder. Both display trash icon. When I currently delete an email it goes to /Trash on server. There is one email under /Kustutatud on server too, received January 2015 and deleted sometime after. I've been running aurora builds. Under settings server directory is empty, namespace is "INBOX."
I can also confirm this problem. At first I thought it was caused by some mis-configuration of K-9 Mail(android app https://code.google.com/p/k9mail/) accessing the same IMAP server. But now (due to this bug) it seems a Thunderbird issue.
I can see this, so I'll work on it. Thanks for the confirmations.
Assignee: nobody → rkent
I'm reasonably sure this is a regression from the second patch landed with bug 558659.
Blocks: RFC6154
I don't believe that bug 558659 properly accounted for the fact that special-case folders sometimes have a URL name (Trash) that is different from the displayed folder name (which is localized).
Neil, because this is a blocker for Thunderbird 38, the review is urgent.
Attachment #8601954 - Flags: review?(neil)
Kent: You took less time for the fix than it took me to mess around. Excellent!!
Comment on attachment 8601954 [details] [diff] [review]
Use url leaf to compare to trash folder name

Review of attachment 8601954 [details] [diff] [review]:
-----------------------------------------------------------------

::: mailnews/imap/src/nsImapIncomingServer.cpp
@@ +1559,5 @@
> +              // different from the folder GetName if the trash name is
> +              // localized.
> +              nsAutoCString trashURL;
> +              trashFolder->GetFolderURL(trashURL);
> +              int32_t leafPos = trashURL.RFindChar('/');

probably wont work for for servers using . as delimiter instead of /
Attached image NAME_SPACE.png
Sorry for not sending this before, but I was sold out all day yesterday and today.
Anyway, both servers are run on Dovecot, first on Ubuntu, second is unknown (not showing up in logs).

Attached is picture of name space settings for both accounts.
These are AUTO settings, I didn't mess with those.
Comment on attachment 8601954 [details] [diff] [review]
Use url leaf to compare to trash folder name

>+        // XXX GetFolder only returns folders one level below root.
>+        //     trashFolderName is a leaf name. So this will not find INBOX.Trash
I haven't actually compiled this, but I spent hours in the debugger so that I could be quite sure that I understood what was going on, including making matters worse by trying to use the account manager UI, which really threw me, since the server that I was using uses INBOX. as the personal namespace, so it's completely broken.

>+              // trashName is the leaf name on the folder URI, which will be
>+              // different from the folder GetName if the trash name is
>+              // localized.
Ugh, this was me thinking that this was what SetPrettyName was for.

>+              int32_t leafPos = trashURL.RFindChar('/');
>+              if (leafPos > 0)
>+              {
>+                trashURL.Cut(0, leafPos + 1); // get rid of the parent name
>+              }
>+              nsAutoCString unescapedName;
>+              MsgUnescapeString(trashURL, nsINetUtil::ESCAPE_URL_PATH,
>+                                unescapedName);
Did you mean > 0? leafPos >= -1, so leafPos + 1 is always valid, although my preference would be to pass Substring(trashURL, leafPos + 1) to MsgUnescapeString.
Attachment #8601954 - Flags: review?(neil) → review+
I know absolutely nothing about the matter, but just out of interest:
What is this namespace business about? The default "Personal namespace" in TB seems to be "INBOX.".
Magnus commented (comment #19) on the "/" vs. ".". Is this a valid concern?
How do I find out what the server uses? Well, in SquirrelMail (web interface) and other web interfaces I can see links ending in INBOX.Trash, etc., so these servers use "INBOX." I assume.
(In reply to Magnus Melin from comment #19)
...
> 
> probably wont work for for servers using . as delimiter instead of /

The existing code is quite confusing about which variables use he hierarchyDelimiter, and which use slashes; which are fully qualified, and which are leaf name; which are translated, and which are not. Hence the bug.

But my understanding is that the variables that I have chosen are all following URL conventions, that is the delimiter is '/'.  Actually the server I tested on had INBOX. as the namespace, so I would be more concerned that it failed with '/' (or with some localized name) than with '.'
(In reply to Jorg K from comment #22)
> I know absolutely nothing about the matter, but just out of interest:
> What is this namespace business about? The default "Personal namespace" in
> TB seems to be "INBOX.".
Not true, it's server-specific. For Microsoft Exchange server, the personal namespace is "", and folders can be created as children of the server. For Gmail, the personal namespace is "", but you can only create subfolders of of the "[Google Mail]" folders. For other servers, the personal namespace is "INBOX." and you can only create subfolders of Inbox.

> Magnus commented (comment #19) on the "/" vs. ".". Is this a valid concern?
I'm pretty sure RDF URIs always use "/", and the "." gets substituted at a lower level.

> How do I find out what the server uses? Well, in SquirrelMail (web
> interface) and other web interfaces I can see links ending in INBOX.Trash,
> etc., so these servers use "INBOX." I assume.
Select an IMAP folder in the mail window, open the Error Console and evaluate this:
top.opener.msgWindow.openFolder.nsIMsgImapMailFolder.hierarchyDelimiter
(In reply to neil@parkwaycc.co.uk from comment #24)
> > The default "Personal namespace" in TB seems to be "INBOX.".
> Not true, it's server-specific.

Oh, I see, the advanced setting is picked up from the server then.

> Select an IMAP folder in the mail window, open the Error Console and
> evaluate this:
> top.opener.msgWindow.openFolder.nsIMsgImapMailFolder.hierarchyDelimiter

Thanks. Shows a |.| in my case (as expected)
https://hg.mozilla.org/comm-central/rev/358db6b87dc9
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Attachment #8601954 - Flags: approval-comm-beta?
Attachment #8601954 - Flags: approval-comm-aurora?
Comment on attachment 8601954 [details] [diff] [review]
Use url leaf to compare to trash folder name

https://hg.mozilla.org/releases/comm-aurora/rev/62b5abd46440
https://hg.mozilla.org/releases/comm-beta/rev/2ed97196ca01
Attachment #8601954 - Flags: approval-comm-beta?
Attachment #8601954 - Flags: approval-comm-beta+
Attachment #8601954 - Flags: approval-comm-aurora?
Attachment #8601954 - Flags: approval-comm-aurora+
This looks a whole lot better now. I tested with a Spanish version of TB38b5.

There is one case left, where the Trash is doubled up. It goes like this:

Start the Spanish version with an IMAP account.
1) In the account configuration, select "Papelera" (paper basket) as the folder to move deleted messages to.
2) Go to the inbox "Bandeja de entrada" (in-tray) and delete a messages which is moved to the "Papelera".
3) Quit the Spanish version and start an English version.

Result:
The deleted message from step 3) is in the Trash.
However, there now is a "Bandeja de entrada" folder which contains a "Papelera". Not so good.

What do you think about this, Kent?
Flags: needinfo?(rkent)
Blocks: TB38found
Depends on: 1175446
I'm trying to clear out old NI requests.

Concerning comment 28, if there is a remaining issue after this bug is landed, please file a new bug. I don't have time at the moment to investigate the remaining issue myself.
Flags: needinfo?(rkent)
You need to log in before you can comment on or make changes to this bug.