1739789 - Smart/unified/virtual folders do not keep folder selection properly when a folder contains a non-ascii character

Status: RESOLVED FIXED

(Thunderbird :: Search, defect)

Description

[Mark Banner (:standard8)]

STR

1. Set up a folder with a non-ascii character in the name, e.g. å
2. Set up a smart search folder to search for something that is in that folder, and manually select the folders searched to include that folder.
3. Check the smart search returns a result.
4. Restart

Expected Results

=> The smart search folder still returns a result, and has the non-ascii folder selected in its list.

Actual Results

=> The smart folder on longer returns a result, the non-ascii folder is not selected.

I suspect this is a regression from bug 1571672, and may relate to nsIMsgFolder.getChildWithURI taking a ACString argument for the URI rather than an AUTF8String.

Comment 5

[Jean-Pierre]

This bug was indicated as duplicate of Bug 1739414. I am not sure it is the case.

I am working in French and I have issue with the "Messages envoyés" (sent messages) from Gmail. Please refer to the precise description of bug 1739414

1. I have no issue with "Envoyés" used for sent message by @me.com s, or by @laposte.net, or by orange.fr. I have issue only with a single virtual folder and a single type of mail provider (Gmail) using "Messages envoyés", as described for bug 1739414.

2. I have been able to create a Test smart search folder that mimics the expected behavior of the "Message envoyés" virtual folder, searching the various folders "Messages Envoyés" in all my accounts. And this search delivers the expected result. Note : I did not do this test before as I did not know about smart search folders.

Happy to answer any question if useful.

Comment 6

[Piotr Biesiada]

I suspect some bug when translating "Wysłane" this way or another for Gmail IMAP account. So for Gmail maybe it should be "Sent", not "Wysłane". Depends on the language. Additionally but not a must it can be diacritic character involved, so the name "Wysłane" encoding itself.

Seems like this bug may be related to "virtualFolders.dat" file or its support/service (surroundings).

Comment 7

[Piotr Biesiada]

(In reply to Jean-Pierre from comment #5)

...

Aren't Gmail the only IMAP account in your TB? (the rest is POP3?)

Comment 8

[Mark Banner (:standard8)]

(In reply to Jean-Pierre from comment #5)

This bug was indicated as duplicate of Bug 1739414. I am not sure it is the case.

I am working in French and I have issue with the "Messages envoyés" (sent messages) from Gmail. Please refer to the precise description of bug 1739414

Whilst they are not exactly the same, I would be confident enough to say that it isn't worth keeping them separate at the moment. The root cause for both is most likely what I referenced i comment 0. Once this is fixed, we can always re-assess/re-open.

Comment 9

[Jean-Pierre]

In reply to comment #7
All my accounts are IMAP

Comment 10

[EarlgreyTea]

This issue is also occurring in non-French Gmail.
When Thunderbird was updated from 78 to 91, there was a problem with Japanese name folders in Gmail.

case-1: Gmail message list is not displayed in the following unified folders.

• 下書き (Drafts)
• 送信済みトレイ (Sent)
• 迷惑メール (Junk)
• ごみ箱 (Trash)

case-2: Gmail message list is not displayed in the search folder that searches for folders with any Japanese name other than the following.

• 受信トレイ (Inbox)
• アーカイブ (Archives)

In Thunderbird 78 IMAP account, the Japanese name folders are named using modified UTF-7. Due to bug 1571672, Thunderbird 91 supports "UTF8 = ACCEPT" IMAP Capability and Gmail returns "UTF8 = ACCEPT", so the Japanese name folder will be named using UTF-8.
However, I think that smart folders do not support UTF-8 URIs and are no longer working for non-ASCII folders such as Japanese and French.

Comment 11

[Jean-Pierre]

(In reply to Jean-Pierre from comment #5)

...

2. I have been able to create a Test smart search folder that mimics the expected behavior of the "Message envoyés" virtual folder, searching the various folders "Messages Envoyés" in all my accounts. And this search delivers the expected result. Note : I did not do this test before as I did not know about smart search folders.

Erratum: Search works first time...but when I restart Thunderbird, my Gmails accouts are again ignored by this bespoke search. Sorry.

Comment 13

[Jean-Pierre]

I made further tests. Created the same folder named "Tést" (with e accute) in :

• Local Folders
• @me.com account
• @gmail.com account

Worked well initially. But the content of the Tést account for Gmail was missing after relaunch. For the other two accounts, content was OK.

So apparently the statement

"Summary: Smart folders do not keep folder selection properly when a folder contains a non-ascii character → Smart/unified/virtual folders do not keep folder selection properly when a folder contains a non-ascii character"

is true only for some mail providers (in particular Gmail).

Comment 14

[EarlgreyTea]

bug 1739903 was closed as a duplicate of this bug.
Indeed, both 1739789 and bug 1739903 were triggered by bug 1571672, which caused the non-ASCII version of Gmail's IMAP folder to be saved in UTF-8, but the two bugs are not the same bug.
Please reconsider.

bug 1739789

• This bug targets smart folder (unified folder and search folder) and focuses on the API not supporting the scope specified in UTF-8 in virtualFolders.dat.
• virtualFolders.dat is updated when folder pane is switched to unified folder view or when search folder is created.
• Thunderbird tries to write the UTF-8 folder name to virtualFolders.dat, but it's already corrupted.

bug 1739903

• This bug targets message filters. If the folder is renamed without the user's permission, it will be inconsistent with the folder specified in MUTF-7 in msgFilterRules.dat and the filter will not work. I argue that is the problem.
• Message filter is different from the smart folder and doesn't use virtualFolders.dat.
• If the user edits the message filter rule and respecifies the folder, it works.
• However, not a few users set and use many message filters. What do users think if they suddenly stop working and need to be manually fixed one by one?

Comment 15

[Rachel Martin]

(In reply to Mark Banner (:standard8) from comment #0)

I suspect this is a regression from bug 1571672, and may relate to nsIMsgFolder.getChildWithURI taking a ACString argument for the URI rather than an AUTF8String.

getChildWithURI() is fixed by our patch in bug 1739814. May be worth testing after that landed. It's really hard to tell which API affects which function without debugging/tracing through the code. For example, to fix the "save draft" issue, we changed all likely candidates and then there was still more.

Comment 17

[Rachel Martin]

Tested with the patch from bug 1739814 applied: Search folder based on IMAP folder testé retains result after restart.

Comment 18

[Rachel Martin]

Mark, please retest and close if resolved.

Comment 19

[Piotr Biesiada]

Problem still exists in Thunderbird 91.3.2 x64.

Selected folders messages are not visible, selected folder is not checked when closing folders list (Properties for folder - Choose) and opening once again.

Comment 20

[Rachel Martin]

So far it's only fixed on Daily builds.

Comment 21

[EarlgreyTea]

(In reply to Rachel Martin from comment #20)

So far it's only fixed on Daily builds.

Will the fix be in the next build?
I checked it with Daily 96.0a1 (2021-11-25), but unified folder and search folder still do not work.

Comment 22

[Rachel Martin]

It should have been fixed in Daily three days ago, see bug 1739814 comment #11. We've just retested it, it works on one IMAP server which doesn't support UTF-8 in folder names, and we believe it didn't work there before, but it still doesn't work on Gmail which does support UTF-8 folder names. Looking at API that still need changing, there are still these ones:

mailnews/base/public/nsIMessengerWindowService.idl
void openMessengerWindowWithUri(in string aWindowType, in string aFolderURI, in nsMsgKey aMsgKey);

mailnews/base/public/nsIMsgMessageService.idl
void Search(in nsIMsgSearchSession aSearchSession, in nsIMsgWindow aMsgWindow, in nsIMsgFolder aMsgFolder, in string aSearchUri);

mailnews/base/public/nsISubscribableServer.idl
void startPopulatingWithUri(in nsIMsgWindow aMsgWindow, in boolean forceToServer, in string uri);

mailnews/db/msgdb/public/nsIMsgDatabase.idl
nsIMsgEnumerator getCachedHits(in string aSearchFolderUri);
Array<nsMsgKey> refreshCache(in string aSearchFolderUri, in Array<nsMsgKey> aNewHits);
void updateHdrInCache(in string aSearchFolderUri, in nsIMsgDBHdr aHdr, in boolean aAdd);
boolean hdrIsInCache(in string aSearchFolderUri, in nsIMsgDBHdr aHdr);

mailnews/db/msgdb/public/nsIMsgOfflineImapOperation.idl
attribute string destinationFolderURI; // for move or copy
attribute string sourceFolderURI;

mailnews/compose/public/nsISmtpServer.idl
readonly attribute ACString serverURI;

mailnews/imap/public/nsIImapIncomingServer.idl
ACString getUriWithNamespacePrefixIfNecessary(in long namespaceType, in ACString originalUri);

mailnews/base/public/nsIMsgAccountManager.idl
ACString folderUriForPath(in nsIFile aLocalPath);

mailnews/base/public/nsIMsgDBView.idl
readonly attribute ACString URIForFirstSelectedMessage;

mailnews/addrbook/public/nsIAbOutlookInterface.idl
Array<ACString> getFolderURIs(in ACString aURI);

Anything with "search" in the name likely contributes to the failure. We'll submit a patch soon ... and test it on Gmail before declaring it fixed :-(

Comment 23

[EarlgreyTea]

(In reply to Rachel Martin from comment #22)

It should have been fixed in Daily three days ago, see bug 1739814 comment #11.

I thought there might be a time lag between the fix and the build

Anything with "search" in the name likely contributes to the failure. We'll submit a patch soon ... and test it on Gmail before declaring it fixed :-(

When retesting, could you please check comment #10 in Gmail (display language: Japanese)?

Comment 24

[Rachel Martin]

Attachment: 1739789-string-APIs-AUTF8String.patch

This is the mechanical part, but there's still something wrong. After the first restart the search folder is empty and after the second one, it's gone. Also this happens:
JavaScript error: resource:///modules/DBViewWrapper.jsm, line 1113: NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIMsgDBView.open]

Comment 25

[Rachel Martin]

Attachment: 1739789-searchFolderUri-utf-8.patch

This is the second part. The searchFolderURIs property is now stored as UTF-8 in the database, hence, the JS access needs to be changed accordingly.

A few comments:

• Given the 13 regressions bug 1571672 caused, it is really questionable whether it was a good idea to make folder names non-ASCII internally instead of MUTF-7 or percent encoding them.
• Here and in bug 1739903, this approach invalidates stored data: Filters storing non-ASCII folder names become dysfunctional and the same goes for virtual folders. It will cause users quite some drama to get rid of the old data and recreate everything again. Particularly if one day an IMAP-Server switches to UTF-8 support, filters and virtual folders related to that server will become invalid.
• While testing the patch, we ran into bug 1344724, for example the search folder testésearch of testé doubles up after the first restart if testé was open at shutdown. On the next restart it becomes good again. This is for the original bug to fix.

Comment 26

[Rachel Martin]

Attachment: 1739789-string-APIs-AUTF8String.patch (version 2)

All that was really needed was to convert {get|set}CharProperty() to AUTF8String, no JS conversion required. The previous comment still applies.

Comment 27

[EarlgreyTea]

(In reply to Rachel Martin from comment #25)

• Given the 13 regressions bug 1571672 caused, it is really questionable whether it was a good idea to make folder names non-ASCII internally instead of MUTF-7 or percent encoding them.
• Here and in bug 1739903, this approach invalidates stored data: Filters storing non-ASCII folder names become dysfunctional and the same goes for virtual folders. It will cause users quite some drama to get rid of the old data and recreate everything again. Particularly if one day an IMAP-Server switches to UTF-8 support, filters and virtual folders related to that server will become invalid.

I think that's exactly what it is.
Currently, bug 1571672 is causing many disadvantages to users. The only benefit to the user of this is that it makes the contents of the local directory where the message is stored easier to understand.

I think supporting UTF8 = ACCEPT should be canceled once.
In the next Thunderbird 91 release, we would like to request the following:

• Set the default value of mail.server.default.allow_utf8_accept to false
• Prepare a UI for the setting items of mail.server.default.allow_utf8_accept so that it can be changed at the user's will.
• Officially announce that problems will occur if you are using non-ASCII names in Gmail and that you can work around it by disabling mail.server.default.allow_utf8_accept.
  Please consider.

Comment 28

[Jean-Pierre]

(In reply to EarlgreyTea from comment #27)

• Officially announce that problems will occur if you are using non-ASCII names in Gmail and that you can work around it by disabling mail.server.default.allow_utf8_accept.
  Please consider.

In France, sent mail folder name is « message envoyés ». With a non ascii name. There is no way to change this.

Comment 29

[Rachel Martin]

@EarlgreyTea: reverting mail.server.default.allow_utf8_accept would need to be requested in another bug. We're not sure how much confusion that would cause.
@Jean-Pierre: The French name has been like this for decades, but there is no need to encode it in UTF-8. Everything worked without it before.

Comment 30

[EarlgreyTea]

(In reply to Rachel Martin from comment #29)

@EarlgreyTea: reverting mail.server.default.allow_utf8_accept would need to be requested in another bug. We're not sure how much confusion that would cause.

I reported Bug 1743253.

Comment 31

[Jean-Pierre]

(In reply to Rachel Martin from comment #29)

@EarlgreyTea: reverting mail.server.default.allow_utf8_accept would need to be requested in another bug. We're not sure how much confusion that would cause.
@Jean-Pierre: The French name has been like this for decades, but there is no need to encode it in UTF-8. Everything worked without it before.

Thanks for the hint. Indeed, I set mail.server.default.allow_utf8_accept to false.
I had a duplicate folder. So indeed, it creates confusion. I had to dig into the imap folders in order to remove the duplicate folder. This is certainly something that should not be advised in general.
But, now, with this temporary fix, Thunderbird behaves like in the previous version.

Comment 32

[Magnus Melin [:mkmelin]]

Comment on attachment 9252700 [details] [diff] [review]
1739789-string-APIs-AUTF8String.patch (version 2)

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

Looks alright, r=mkmelin

Comment 33

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/126e11fc097f
Change search-related URI APIs and all other remaining ones to AUTF8String. r=mkmelin

Comment 34

[Rachel Martin]

Comment on attachment 9252700 [details] [diff] [review]
1739789-string-APIs-AUTF8String.patch (version 2)

[Approval Request Comment]
Regression caused by (bug #): bug 1571672
User impact if declined: Virtual folders with non-ASCII names don't work.
Testing completed (on c-c, etc.): Yes.
Risk to taking this patch (and alternatives if risky):
Not risky, mostly mechanical change of raw string API to "smart" string API allowing for non-ASCII strings to be correctly passed around.
Part of a pack of six bugs which need to be uplifted in this order:
Bug 1739784, bug 1741559, bug 1741619, bug 1739814, bug 1739903, bug 1739789 (this one here). The first one is already on ESR, the next four are on beta 95, this last one here missed beta 95.

Comment 35

[EarlgreyTea]

In Daily 96.0a1 (20211130102722), I confirmed that the unified/search folder works with the Japanese Gmail folders.

Comment 37

[Wayne Mery (:wsmwk)]

Comment on attachment 9252700 [details] [diff] [review]
1739789-string-APIs-AUTF8String.patch (version 2)

[Triage Comment]
Approved for esr91

Comment 38

[Rob Lemley [:rjl]]

Thunderbird 91.4.1:
https://hg.mozilla.org/releases/comm-esr91/rev/892edd477387