798648 - Gmail Label parsing needs to handle case for "\\Important" and "\\Starred" special labels

Status: NEW

(MailNews Core :: Networking: IMAP, defect)

Description

[David Lechner (:dlech)]

follow up to item 5 in https://bugzilla.mozilla.org/show_bug.cgi?id=721316#c60

Need to add case for handling "\\Important" just like "\\Drafts", "\\Inbox", etc. here:
https://mxr.mozilla.org/comm-central/ident?i=labelNames&tree=comm-central&filter=nsImapMailFolder

This also means that we need to add special folder flag for 'Important' here:
https://mxr.mozilla.org/comm-central/ident?i=kImapXListTrash&tree=comm-central&filter=nsImapCore
and here:
https://mxr.mozilla.org/comm-central/ident?i=kImapXListTrash&tree=comm-central&filter=nsImapServerResponseParser
and here:
https://mxr.mozilla.org/comm-central/ident?i=Unused1&tree=comm-central&filter=nsMsgFolderFlags

Comment 1

[David Lechner (:dlech)]

Also need to fix imap fakeserver here:
https://mxr.mozilla.org/comm-central/ident?i=specialFolderFlagsLookupTable+&tree=comm-central&filter=

Comment 2

[Atul Jangra [:atuljangra]]

Confirming it on the basis of Bug 721316.

Comment 3

[Atul Jangra [:atuljangra]]

(In reply to David Lechner (:dlech) from comment #1)
> Also need to fix imap fakeserver here:
> https://mxr.mozilla.org/comm-central/
> ident?i=specialFolderFlagsLookupTable+&tree=comm-central&filter=

This link is giving nothing :-/

Comment 4

[David Lechner (:dlech)]

(In reply to Atul Jangra [:atuljangra] from comment #3)
> (In reply to David Lechner (:dlech) from comment #1)
> > Also need to fix imap fakeserver here:
> > https://mxr.mozilla.org/comm-central/
> > ident?i=specialFolderFlagsLookupTable+&tree=comm-central&filter=
> 
> This link is giving nothing :-/

I already fixed it in Bug 495318, so 'specialFolderFlagsLookupTable' is gone from imapd.js. Instead, when setting up a test you manually assign SPECIAL-USE/XLIST flags using something like: 

gIMAPMailbox.specialUseFlag = "\\Inbox";

Also, if you are thinking about working on this bug, bug 558659 needs to be fixed first. Here are my thoughts on how I would approach this:

XLIST is basically non-standard SPECIAL-USE (RFC 6154). We should make sure that anything we do for XLIST also works for SPEICAL-USE and vice versa. Here is a table of the XLIST and SPECIAL-USE equivalents.

RFC 6154    | XLIST
------------+----------------
\All        | \AllMail
\Archive

Comment 5

[David Lechner (:dlech)]

Accidentally hit save. Starting the table over:

RFC 6154    | XLIST      | nsImapCore.h
------------+------------+-------------
(N/A)       | \Inbox     | kImapInbox
\All        | \AllMail   | kImapAllMail
\Archive    | \AllMail*  | (needed)
\Drafts     | \Drafts    | kImapDrafts
\Flagged    | \Important | (needed)
(N/A)       | \Starred   | (needed)
\Junk       | \Spam      | kImapSpam
\Sent       | \Sent      | kImapSent
\Trash      | \Trash     | kImapXListTrash

* Thunderbird currently forces users to use \AllMail as archive, but that is another discussion.

I think that the XLIST \Important flag should be the equivelant of the SPECIAL-USE \Flagged flag. I am not sure that any clients\servers are using \Flagged yet (http://www.imapwiki.org/SpecialUse), but I am guessing that this is the intention of those that authored the RFC (which includes someone from google). We also need a flag for \Starred so that we can lookup the folder.

Comment 6

[David Lechner (:dlech)]

Also, in the spirit of Bug 558659, I think we should differentiate between server specified special folders and user specified special folders. Currently Thunderbird forces the use of the special folders sent by XLIST and users cannot override them. 

https://mxr.mozilla.org/comm-central/source/mailnews/base/public/nsMsgFolderFlags.idl currently has a flag for special-use folders, however, this IS NOT the folders indicated by the SPECIAL-USE RFC. This should be renamed to something like 'ClientSpecialUse'. Additional (potentially redundant) flags should be added for 'ServerSpecialUse' so that we can tell whether the server or the user specified the use. 

For example, the current flags are used at https://mxr.mozilla.org/comm-central/source/mailnews/imap/src/nsImapMailFolder.cpp#9644 - however, if the trash model is selected as 'mark as deleted', 

rootFolder->GetFolderWithFlags(nsMsgFolderFlags::Trash, getter_AddRefs(subMsgFolder)); 

does not return anything even if we are subscribed to the Trash folder. If we had a separate ServerTrash flag, then we could use it instead.

Comment 7

[WADA:World Anti-bad-Duping Agency]

(In reply to David Lechner (:dlech) from comment #5)
> Accidentally hit save. Starting the table over:

Following change is applied to table.
(i)   Status in nsMsgFolderFlags in nsMsgFolderFlags is added.
(ii)  Gmail/Gmail IMAP uses [Gmail]/Starred for "mail which has \Flagged flag, for which Tb shows Star mark at thread pane.
Gmail/Gmail IMAP doesn't use "\Flagged flag" for mail in [Gmail]/Starred. Only \Seen was observded in IMAP log.
(iii) nsMsgFolderFlags only "Queue" row is added. 

> +------------+------------+------------------+---------------------------------------------
> |RFC 6154    | XLIST      | nsImapCore.h     | nsMsgFolderFlags (SpecialUse flags)
> +------------+------------+------------------+---------------------------------------------
> |(N/A)       | \Inbox     | kImapInbox       | Inbox
> |\All        | \AllMail   | kImapAllMail     | (needed?)
> |\Archive    | \AllMail   | (needed)         | Archive
> |\Drafts     | \Drafts    | kImapDrafts      | Drafts
> |\Flagged?   | \Important | (needed)         | (needed?)
> |\Flagged?   | \Starred   | (needed)         | (needed?)  Note: Gmail IMAP uses \Flagged flag of IMAP for this folder
> |\Junk       | \Spam      | kImapSpam        | Junk
> |\Sent       | \Sent      | kImapSent        | SentMail
> |\Trash      | \Trash     | kImapXListTrash  | Trash
> |(N/A)       | (N/A)      | ???              | Queue (for Outbox in Tb, aka Unsent Messages)
> +------------+------------+------------------+---------------------------------------------

Comment 8

[WADA:World Anti-bad-Duping Agency]

(In reply to David Lechner (:dlech) from comment #6)
> Currently Thunderbird forces the use of the special folders sent by XLIST
> and users cannot override them. 

It's slightly inaccurate, if narrow deinition of FORCE.

At Copies&Folders panel and Junk Settings panel, user can select any folder, and Tb normally uses the selected folder by user according to user's request as Sent, Drafts, Archives, Templates, and Junk.
Problem is;
  Because default folder(top level Sent, top level Drafts, top level Archives,
  top level Junk) is usually doesn't exist at server if Gmail account,
  folders in XLIST is used after account definition.
  Similar "fall back to folder in XLIST" occurs upon restart of Tb, if user
  deletes folder used as Tb's special folder while Tb is not runnning.
I can't call it "forced always by Tb".

"Actual force", "narrow FORCE", is currently following only if Gmail IMAP, as I wrote in bug 800035.
- If Gmail IMAP, unable to use other than [Gmail]/Trash as trash folder in Tb 
  for IMAP delete model of "Move to trash".
- Unable to use Archive option if Gmail IMAP, even when ordinal Gmail Label is
  selected by user as Archive folder. (mail.identity.idX.archive_granularity)
  Selection of Local mail folder or IMAP folder of ordinal IMAP server is
  needed to use Archive option.

Needless to say, reardless of "wide force" or "narrow force", as Ben Bucksch says in bug 800035 and bug 558659 comment #40, and as David Lechner (:dlech) says, 'ClientSpecialUse' / 'ServerSpecialUse' like one will be needed to improve.

> rootFolder->GetFolderWithFlags(nsMsgFolderFlags::Trash, getter_AddRefs(subMsgFolder));
> does not return anything even if we are subscribed to the Trash folder.

As seen in bug 533140, current root cause of "trash problem" is;
  After folder re-discovery(by account collapse/expand etc.),
  if account is Gmail IMAP(recently, isGmailServer=true/is_gmail=true which is
  set from X-GM-X-GM-EXT-1 capability response),
  Tb always removes SpecialUse=Trash flag of any folder under Gmail IMAP account
  even if user selected other than [Gmail]/Trash as trash at Server Settings,
  (by it, Tb normally saves name in trash_folder_name and adds Trash flag)
  and Tb always sets SpecialUse=Trash flag of trash folder in XLIST
  ( [Gmail]/Trash if Gmail IMAP ).
i.e. Tb is currently intentionally forcing [Gmail]/Trash always, regardless of user's wants.
Trash problem is slightly different from Copies&Folders problem and Junk problem, although 'ClientSpecialUse' / 'ServerSpecialUse' like one will be very helpfull to resolve Trash problem too.

Comment 9

[WADA:World Anti-bad-Duping Agency]

Sorry type. Read as follows, please.
  Gmail/Gmail IMAP doesn't use "\Flagged flag" for mail in [Gmail]/Important.

Comment 10

[WADA:World Anti-bad-Duping Agency]

Attachment: Gmail Label and Mbox Name in Gmail IMAP

Gmail uses [Gmail]/xxx in imap mbox name for Gmail's special folder,
and uses [Imap]/xxx in Gmail Label for top level imap folder of special name.
Reserved Label Name is [Gmail]/xxx and [Imap]/xxx where xxx is pre-defined set only.
Gmail Label is merely a string, although "/" is used as IMAP Mbox hierarchy delimiter in Gmail IMAP..
So, user can create user deified Gmail Label named [Gmail], [Imap], [Gmail]/<ReservedName>/AnyName, [Imap]/<ReservedName>/AnyName,

Comment 11

[WADA:World Anti-bad-Duping Agency]

Gmail IMAP actually returns \Important and \Starred to XLIST and FETCH X-GM-LABELS, and mapping of "Gmail's LABEL to actual Mbox name in Gmail IMAP" is needed to support Gmail's localized folder name of special folders in Gmail, and the mapping is returned as XLIST response.
"adding at lease next items is neede" is obvious, isn't it?
> +------------+-------------+-----------------------+----------------------------------------+--------------------------------+
> |RFC 6154    | XLIST           | nsImapCore.h            | nsMsgFolderFlags (SpecialUse flags)  | msgFolder.boxFlags                |
> +------------+-------------+-----------------------+----------------------------------------+-------------------------------+
> |N/A             | \Important  | kGmailmportant         | N/A                                                      | Should be stored in boxFlags |
> |N/A             | \Starred      | kGmailStarred            | N/A                                                      | Should be stored in boxFlags |
> +------------+-------------+-----------------------+----------------------------------------+--------------------------------+

By tthe way, mapping of "folderURI, folder name -> GMLABEL" is possible because GMLABEL information is stored in msFolder.boxFlags.
However mapping of "GMLABEL -> actual folder name for the GMLABEL" is slightly expensive, because "scan of msgFolder.boxFlags of all [Gmail]/xxx folders" is needed.
"Data of GMLABEL -> actual mbox name in XLIST response" is better saved somewhere.
Or msgFolder.getFolderForGMLABEL() or server.getFolderForGMLABEL()  is sufficient because "number of folders should be scanned" is far less than getFolderForFlags()?

FYI.
Gmail IMAP currently permits creation of top level folder named \??? where \??? iis same string as GMLABEL which is returned to XLIST.
So, when a MSGID in [Gmail]/All Mail haas GMLABEL of \???, imap client can't know [Gmail]/???  or top level \??? from GMLABEL string of a MSGID.
I checked it for top level \Important,  \Starred, \Drafts, and \Sent.
It can be called "Gmail's bug"(Gmail should reserve such name), but we are perhaps better prohibit creation of top level folder of GMLABEL string.