Closed Bug 1739789 Opened 3 years ago Closed 3 years ago

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

Categories

(Thunderbird :: Search, defect)

Unspecified
All
defect

Tracking

(thunderbird_esr91+ fixed, thunderbird95 affected)

RESOLVED FIXED
96 Branch
Tracking Status
thunderbird_esr91 + fixed
thunderbird95 --- affected

People

(Reporter: standard8, Assigned: rachel)

References

(Regression)

Details

(Keywords: intl, regression, Whiteboard: [datalossy])

Attachments

(1 file, 2 obsolete files)

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.

Blocks: 1739814

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.

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).

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

...

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

(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.

In reply to comment #7
All my accounts are IMAP

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.

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

...

  1. 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.

Blocks: tb91found
Severity: -- → S2
Keywords: intl
OS: Unspecified → All
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
Whiteboard: [datalossy]

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).

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?

(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.

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

Mark, please retest and close if resolved.

Flags: needinfo?(standard8)

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.

So far it's only fixed on Daily builds.

(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.

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 :-(

Flags: needinfo?(standard8)

(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)?

Attached patch 1739789-string-APIs-AUTF8String.patch (obsolete) (deleted) — Splinter Review

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]

Attachment #9252567 - Flags: review?(mkmelin+mozilla)
Attached patch 1739789-searchFolderUri-utf-8.patch (obsolete) (deleted) — Splinter Review

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.
Attachment #9252682 - Flags: review?(mkmelin+mozilla)

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

Attachment #9252567 - Attachment is obsolete: true
Attachment #9252682 - Attachment is obsolete: true
Attachment #9252567 - Flags: review?(mkmelin+mozilla)
Attachment #9252682 - Flags: review?(mkmelin+mozilla)
Attachment #9252700 - Flags: review?(mkmelin+mozilla)

(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.

(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.

@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.

(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.

(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 on attachment 9252700 [details] [diff] [review] 1739789-string-APIs-AUTF8String.patch (version 2) Review of attachment 9252700 [details] [diff] [review]: ----------------------------------------------------------------- Looks alright, r=mkmelin
Attachment #9252700 - Flags: review?(mkmelin+mozilla) → review+
Assignee: nobody → rachel
Status: NEW → ASSIGNED
Target Milestone: --- → 96 Branch

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/126e11fc097f
Change search-related URI APIs and all other remaining ones to AUTF8String. r=mkmelin

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

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.

Attachment #9252700 - Flags: approval-comm-esr91?

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

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

[Triage Comment]
Approved for esr91

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

Attachment

General

Creator:
Created:
Updated:
Size: