1939403 - getTagFolder fails with nsIMsgLocalMailFolder.createLocalSubfolder

Status: RESOLVED FIXED

(Thunderbird :: Add-Ons: Extensions API, defect)

Description

[Philipp Kewisch [:Fallen][📱📆]]

The messenger.folders.getTagFolder function fails for some users when the virtual tag folder does not yet exist.

They are running into the other branch here: https://searchfox.org/comm-esr128/rev/7ba4471e047afbc95ea809a368a8390e1c3af273/mail/modules/SmartMailboxUtils.sys.mjs#173

With an error like this:

Component returned failure code: 0x80550013 [nsIMsgLocalMailFolder.createLocalSubfolder]
getTagFolder resource:///modules/SmartMailboxUtils.sys.mjs:224
verify resource:///modules/SmartMailboxUtils.sys.mjs:178
getSmartMailbox resource:///modules/SmartMailboxUtils.sys.mjs:267
getTagFolder chrome://messenger/content/parent/ext-folders.js:1300
result resource://gre/modules/ExtensionParent.sys.mjs:1221
withCallContextData resource://gre/modules/ExtensionParent.sys.mjs:664
result resource://gre/modules/ExtensionParent.sys.mjs:1220
withPendingBrowser resource://gre/modules/ExtensionParent.sys.mjs:674
result resource://gre/modules/ExtensionParent.sys.mjs:1219
callAndLog resource://gre/modules/ExtensionParent.sys.mjs:1170
recvAPICall resource://gre/modules/ExtensionParent.sys.mjs:1218
SmartMailboxUtils.sys.mjs:180:17

The code to trigger this was simply:

let tags = await messenger.messages.tags.list();
let tagFolders = await Promise.all(tags.map(async tag => {
  let folder = await messenger.folders.getTagFolder(tag.key);
  folder.color = tag.color;
  folder.subFolders = [];
  return folder;
}));

It appears to be specific tags, I'm asking for more info at https://github.com/kewisch/quickmove-extension/issues/194

Comment 1

[Philipp Kewisch [:Fallen][📱📆]]

This also occurs for getUnifiedFolder

Comment 2

[John Bieling (:TbSync)]

This is a twisted bug, and I do not think it is caused by my recent changes for the API. However, the API exposed an existing bug in core code.

The URI of tag folders with special chars could look like the following:

prettyName: "6: special"
URI: "mailbox://nobody@smart%20mailboxes/tags/67a2857d5"

What happens is that this code tries to guess the URI of the tag folder based on its key (but the guessed URI is wrong), and then checks for the existence of the folder.

    let folder = this.#tagsFolder.getChildWithURI(
      this.getTagFolderUriForKey(tag.key),
      false,
      false
    );
    if (folder) {
      return folder;
    }
    folder = this.#tagsFolder.createLocalSubfolder(tag.key);

Since the folder was not found, it is created with the key. However, the folder with that name DOES exist and the create operation fails. Similar code is already used in TB115.

I think the conflict is due to different functions being used to create folders. We have
* folder.createFolder()
* folder.createLocalSubfolder()

The first one is sanitizing the name via a hash function:
https://searchfox.org/comm-central/rev/08aea30323820d7d612f849c932fd29d4d9ed972/mailnews/local/src/nsMsgMaildirStore.cpp#334

How to fix this?

• We could use folder.getChildNamed() to find the tag folder via its name
• We could try to call the used sanitize function from JS and reconstruct the hashed URI, but we might then not find folders which where created using createLocalSubfolder()

We probably should no longer use folder.createLocalSubfolder() in SmartMailboxUtils.sys.mjs, but folder.createFolder()?

Edit: It seems both functions use the hash method in the end. That would mean special chars in tag folders are just broken?

Comment 3

[John Bieling (:TbSync)]

Attachment: Bug 1939403 - Fix smart folder lookup for folders with hashed URI components. r=#thunderbird-reviewers

Some special chars in folder names cause them to be created with a
hashed URI component, instead of an escaped URI component, and tag
handling is currently broken for such tags.

Steps to reproduce:

• enable tags mode in the folder pane
• context click on a tag and select "manage tags"
• create a tag "6: Test"
• verify the new tag is listed in the folder pane
• return to the tag managment tab and edit the newly created tag
• rename it to "6: Test2"
• observe the tag not being renamed in the folder pane
• disable and re-enable tags mode in the folder pane
• observe the new tag no longer being listed at all

The failure is do to 1 failing to return the correct URI. There is
also a problematic behavior of folder.createLocalSubfolder(name): The
URI of the new folder will be derived from the provided name, it will be
sanitized and hashed. However, it is currently not possible to known in
JavaScript, what the final URI will be before the call. It is therefore
not possible to check if the folder exists already (its name can be
anything, unrelated to the used URI and not helpful).

This issue was revealed through our WebExtension APIs, which call
SmartMailBox.getSmartFolder() and SmartMailBox.getTagFolder() and
started throwing errors when tags with special chars were used.

This patch tries to solve these issues as follows:

• catch all errors in SmartMailBox.getSmartFolder() and
  SmartMailBox.getTagFolder(), make them return null if the folder
  could not be retrieved.
• make these functions also handle the "folder is missing case"
• change callers to properly handle the case of no folder being returned
• keep track of URIs of created tag folders in an internal map
• replace 1 by looking up the known URI from the internal map
• remove obsolete tag folders to reduce the possibility of trying to
  create a folder which exists already

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/bce689132a38
Fix smart folder lookup for folders with hashed URI components. r=mkmelin