583677 - Sometimes can't see tags set by another user. SELECT to server doesn't tell TB it supports \* flags, and TB won't store the keywords in .msf file

Status: RESOLVED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[Mikhail]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; ru; rv:1.9.2.8) Gecko/20100722 Firefox/3.6.8
Build Identifier: 3.1.1

Hello. 

Please, help me with the following problem, I have face with. So, there are Approximately 10-15 people 
who use Mozilla ThunderBird. We use Mozilla Thunderbird 2.0 about 4 years. 
We actively use the tags. I mean, when we recieve a new letter, we mark it with some tag (or few tags).
We do NOT use the default tags, we create our own tags. For example (fragment of "pref.js" file):

user_pref("mailnews.labels.description.33", "OKHUS");
user_pref("mailnews.labels.description.34", "AIV");
user_pref("mailnews.labels.description.35", "MNUSH");
user_pref("mailnews.labels.33", "#000000");
user_pref("mailnews.labels.34", "#000000");
user_pref("mailnews.labels.35", "#000000");
user_pref("mailnews.tags.mnush.color", "#000000");
user_pref("mailnews.tags.mnush.tag", "MNUSH");
user_pref("mailnews.tags.okhus.color", "#000000");
user_pref("mailnews.tags.okhus.tag", "OKHUS");
user_pref("mailnews.tags.aiv.color", "#000000");
user_pref("mailnews.tags.aiv.tag", "AIV");

So, some users have the rights (in our mail server) to mark letters with some tag on mail server. 
Another users DO NOT have such rights (do't have rights to mark letters with some tag on mail server).

When we recieve the new letter, the user who have rights mark letter with some tag, so this change is 
send to our mail server, so after that all users see this change (the letter with tag).

So 4 years we did't know any problems. So Mozilla TB 3 is came and we face
with the following problem.

We have 4 imap folders in our imap server:

imap.company.ru
	|
	|	
	|-FOLDER1
	|
	|
	|-FOLDER2
	     |
	     |-FOLDER2.1
	     |
	     |-FOLDER2.2

	
Unexpectedly few of our users lost the possibility to see tags in one of imap
folders. It means:
1. If we have 10000 letters in imap folder and user lost possibility to see
tags on letters at 1 of July 2010 (for example), he can see OLD tags on letters before 1 of
July 2010, but can't see new tags on letter after 1 of July 2010, moreover If
we change tags on letter before 1 of July 2010, the user can't see this
chagne. 

2. If we try to "fix folder" (right mouse click->"fix folder") - it does not
help!

3. If we try to delete *.msf file, it does not help - and he lost the
possibility to see tags on letters before 1 of July 2010

4. While user faces this problem in one of imap folder, he does not face it in
another imap folder.

5. We tried to reinstall mozilla and some times it helps and sometimes - NOT.

6. The STRANGE thing is: if we create "smart" folder and setup tag filter on
it, the smart folder works CORRECTLY. It means that it is problem in
visualization at list of letters, but actually tags were recieved from mail
server and smart folders can see this tags, but user can't see tags at list of
letters.

Sorry for my english :)



Reproducible: Sometimes

Steps to Reproduce:
Please read the details
Actual Results:  
Please read the details

Expected Results:  
Please read the details

Please read the details

Comment 1

[Ludovic Hirlimann [:Usul]]

So your issue is that sometimes tags aren't shown on some machines, while the mail is tagged on the imap server.

This  could come from an extension issue - did you try to start thunderbird in -safe-mode (see http://support.mozillamessaging.com/en-US/kb/Safe+Mode)?

Comment 2

[Mikhail]

Hello!
Yes, I did. I have started it in -safe-mode but there are no any results.
User can't see tags also (but virtual folder with tags filter is working!)

Please inform, if you have another idea to catch the problem. I will send screenshots and/or logs if neccessary

Thank you!

Comment 3

[Ludovic Hirlimann [:Usul]]

We'll need imap logs as described at https://wiki.mozilla.org/MailNews:Logging.

Comment 4

[Mikhail]

Ok, Thank you for your advice. I will reproduce the problem and send the log to you

Comment 5

[Mikhail]

Hello, Ludovic Hirlimann and everyone!
I have prepared an imap log for you. I send it to your email [e-mail]. It is zip archive with password - 12345

Please, Inform me If you don't recieve my message with attached archive. So, here some comments about log.

This log was created by Mozilla TB Ver.3, where the problem is reproduced.

I began my experiment with letter:
532[435d540]: 4806000:mail.sicex.ru:S-quik.support.BROKERS:CreateNewLineFromSocket: * 24394 FETCH (FLAGS (\Seen $MDNSent 2open) UID 999240578)

So, as you can see, this letter is marked with tag "2open". This tag was set by in Mozilla TB Ver.3, where the problem is NOT reproduced.

1. So after tag was set:
1.1 I see this tag at letter in Mozilla TB Ver.3, where the problem is NOT reproduced.
1.2 I can't see this tag at letter in Mozilla TB Ver.3, where the problem is reproduced.
1.3 I see this tag in IMAP.log in Mozilla TB Ver.3, where the problem is reproduced.
1.4 The virtual folder also show me this tag (the rule is "tag=2open") in Mozilla TB Ver.3, where the problem is reproduced.

So, problem in visualisation.

So, I have continued my experiment. I have set another tag (tag name is "mshubin") at the same letter:

532[435d540]: 4806000:mail.sicex.ru:S-quik.support.BROKERS:CreateNewLineFromSocket: * 24394 FETCH (FLAGS (\Seen $MDNSent mshubin 2open))

So, here we have the same letter with two tags - "mshubin" and "2open".

2.BUT AGAIN: 
2.1 I see these tags at letter in Mozilla TB Ver.3, where the problem is NOT reproduced.
2.2 I can't see these tags at letter in Mozilla TB Ver.3, where the problem is reproduced.
2.3 I see this tag in IMAP.log in Mozilla TB Ver.3, where the problem is reproduced.
2.4 The virtual folder also show me this tag (the rule is "tag=2open and tag=mshubin") in Mozilla TB Ver.3, where the problem is reproduced.


I have continued my experiment, but result was the same:
532[435d540]: 4806000:mail.sicex.ru:S-quik.support.BROKERS:CreateNewLineFromSocket: * 24394 FETCH (FLAGS (\Seen $MDNSent 2open))
532[435d540]: 4806000:mail.sicex.ru:S-quik.support.BROKERS:CreateNewLineFromSocket: * 24394 FETCH (FLAGS (\Seen $MDNSent mshubin 2open))
532[435d540]: 4806000:mail.sicex.ru:S-quik.support.BROKERS:CreateNewLineFromSocket: * 24394 FETCH (FLAGS (\Seen $MDNSent mshubin 2open okhus))
532[435d540]: 4806000:mail.sicex.ru:S-quik.support.BROKERS:CreateNewLineFromSocket: * 24394 FETCH (FLAGS (\Seen $MDNSent mshubin pperf 2open okhus))


It will be great if you help me understand the reason of this incident.
Thank you

Comment 6

[Mikhail]

Hello, Ludovic Hirlimann and everyone!
I have prepared an imap log for you. I send it to your email [e-mail]. It is zip archive with password - 12345

Please, Inform me If you don't recieve my message with attached archive. So, here some comments about log.

This log was created by Mozilla TB Ver.3, where the problem is reproduced.

I began my experiment with letter:
532[435d540]: 4806000:mail.sicex.ru:S-quik.support.BROKERS:CreateNewLineFromSocket: * 24394 FETCH (FLAGS (\Seen $MDNSent 2open) UID 999240578)

So, as you can see, this letter is marked with tag "2open". This tag was set by in Mozilla TB Ver.3, where the problem is NOT reproduced.

1. So after tag was set:
1.1 I see this tag at letter in Mozilla TB Ver.3, where the problem is NOT reproduced.
1.2 I can't see this tag at letter in Mozilla TB Ver.3, where the problem is reproduced.
1.3 I see this tag in IMAP.log in Mozilla TB Ver.3, where the problem is reproduced.
1.4 The virtual folder also show me this tag (the rule is "tag=2open") in Mozilla TB Ver.3, where the problem is reproduced.

So, problem in visualisation.

So, I have continued my experiment. I have set another tag (tag name is "mshubin") at the same letter:

532[435d540]: 4806000:mail.sicex.ru:S-quik.support.BROKERS:CreateNewLineFromSocket: * 24394 FETCH (FLAGS (\Seen $MDNSent mshubin 2open))

So, here we have the same letter with two tags - "mshubin" and "2open".

2.BUT AGAIN: 
2.1 I see these tags at letter in Mozilla TB Ver.3, where the problem is NOT reproduced.
2.2 I can't see these tags at letter in Mozilla TB Ver.3, where the problem is reproduced.
2.3 I see this tag in IMAP.log in Mozilla TB Ver.3, where the problem is reproduced.
2.4 The virtual folder also show me this tag (the rule is "tag=2open and tag=mshubin") in Mozilla TB Ver.3, where the problem is reproduced.


I have continued my experiment, but result was the same:
532[435d540]: 4806000:mail.sicex.ru:S-quik.support.BROKERS:CreateNewLineFromSocket: * 24394 FETCH (FLAGS (\Seen $MDNSent 2open))
532[435d540]: 4806000:mail.sicex.ru:S-quik.support.BROKERS:CreateNewLineFromSocket: * 24394 FETCH (FLAGS (\Seen $MDNSent mshubin 2open))
532[435d540]: 4806000:mail.sicex.ru:S-quik.support.BROKERS:CreateNewLineFromSocket: * 24394 FETCH (FLAGS (\Seen $MDNSent mshubin 2open okhus))
532[435d540]: 4806000:mail.sicex.ru:S-quik.support.BROKERS:CreateNewLineFromSocket: * 24394 FETCH (FLAGS (\Seen $MDNSent mshubin pperf 2open okhus))


It will be great if you help me understand the reason of this incident.
Thank you

Comment 7

[Ludovic Hirlimann [:Usul]]

What's the differences between the two versions of thunderbird (one that works and one that doesn't) ?

Ps I have a complete log on my disk.

Comment 8

[Mikhail]

The versions are identical. The lists of Add-ons are the same.
I have allready wrote that, my first guess about this problem was about two types of rights at mail server:

Some users have the rights (in our mail server) to mark letters with some
tag on mail server. 
Another users DO NOT have such rights (do't have rights to mark letters with
some tag on mail server).

The problem reproduced at TB without rights on mail server at MOST CASES. But unfortunatly, I have broken my hypothesis after my another experiment. I gave rights on mail server for TB, where the problem reproduce. Unfortunatly, it was not solve the problem.

So, now I have no idea :( And once again: the problem DO NOT reproduce at 2.0............ so, this bug is the reason of impossibility to use TB ver.3 :(

Comment 9

[Mikhail]

Attachment: pass - 12345

This is imap log
pass - 12345

Comment 10

[Mikhail]

Please inform about additional information you need...

Comment 11

[David :Bienvenu]

What we need is a log from the user who can't see the tag, along with the tag they can't see - it's unclear to me exactly what the log you've attached is demonstrating. Is that a user who can or can't see the tags?

Each user does have to have the tags defined in prefs.js, in order for the tags to be shown, but I believe you knew that. You've double-checked that the tags are in the prefs.js file for the user who can't see the particular tag?

I notice that your imap server doesn't return "\*" for flags that can be set, in some cases, but just enumerates all the flags that are set on messages in the folder. I think that might make Thunderbird not try to store user-flags on the server. That doesn't fit in with your description of the issues you're seeing, but I thought I'd mention it. Your server also supports CONDSTORE, but I don't think that's an issue either, if repair folder doesn't fix the problem.

Comment 12

[Mikhail]

Answers:
1. Attached log is log who CAN'T see the tags. I said that, when other user set the tag, we can see this change on imap.log (user who CAN'T see the tags) and virtual folders also can see this change and DO filter with condition tag="sometag" (for ex. tag="mshubin") but... user stil can't see tags at mails. 

2. Yes, of course! Each user has the same list of tags in prefs.js, index and names are same. Here the full list of tags at pref.js:

user_pref("mailnews.labels.1", "#FF0000");
user_pref("mailnews.labels.2", "#3333FF");
user_pref("mailnews.labels.20", "#000000");
user_pref("mailnews.labels.21", "#000000");
user_pref("mailnews.labels.22", "#000000");
user_pref("mailnews.labels.23", "#000000");
user_pref("mailnews.labels.24", "#000000");
user_pref("mailnews.labels.25", "#000000");
user_pref("mailnews.labels.26", "#000000");
user_pref("mailnews.labels.27", "#000000");
user_pref("mailnews.labels.28", "#000000");
user_pref("mailnews.labels.29", "#000000");
user_pref("mailnews.labels.3", "#CCCCCC");
user_pref("mailnews.labels.30", "#000000");
user_pref("mailnews.labels.31", "#000000");
user_pref("mailnews.labels.32", "#000000");
user_pref("mailnews.labels.33", "#000000");
user_pref("mailnews.labels.34", "#000000");
user_pref("mailnews.labels.35", "#000000");
user_pref("mailnews.labels.36", "#000000");
user_pref("mailnews.labels.4", "#CC33CC");
user_pref("mailnews.labels.5", "#33CC00");
user_pref("mailnews.labels.6", "#FF0000");
user_pref("mailnews.labels.7", "#FF6600");
user_pref("mailnews.labels.8", "#CCCCCC");
user_pref("mailnews.labels.description.1", "1SROCHNO");
user_pref("mailnews.labels.description.2", "2OPEN");
user_pref("mailnews.labels.description.20", "ALPAR");
user_pref("mailnews.labels.description.21", "ASHUM");
user_pref("mailnews.labels.description.22", "DSVET");
user_pref("mailnews.labels.description.23", "ESHUBIN");
user_pref("mailnews.labels.description.24", "IVANOV");
user_pref("mailnews.labels.description.25", "MGRU");
user_pref("mailnews.labels.description.26", "MSHUBIN");
user_pref("mailnews.labels.description.27", "PPERF");
user_pref("mailnews.labels.description.28", "RUDN");
user_pref("mailnews.labels.description.29", "SPUS");
user_pref("mailnews.labels.description.3", "3CLOSE");
user_pref("mailnews.labels.description.30", "UBASH");
user_pref("mailnews.labels.description.31", "VSKOR");
user_pref("mailnews.labels.description.32", "SKUN");
user_pref("mailnews.labels.description.33", "OKHUS");
user_pref("mailnews.labels.description.34", "AIV");
user_pref("mailnews.labels.description.35", "MNUSH");
user_pref("mailnews.labels.description.36", "PDEN");
user_pref("mailnews.labels.description.4", "4DEVELOP");
user_pref("mailnews.labels.description.5", "5PROCESSED");
user_pref("mailnews.labels.description.6", "6MAXAT");
user_pref("mailnews.labels.description.7", "7HIGHAT");
user_pref("mailnews.labels.description.8", "8SPAM");
user_pref("mailnews.tags.1srochno.color", "#FF0000");
user_pref("mailnews.tags.1srochno.tag", "1SROCHNO");
user_pref("mailnews.tags.2open.color", "#3333FF");
user_pref("mailnews.tags.2open.tag", "2OPEN");
user_pref("mailnews.tags.3close.color", "#CCCCCC");
user_pref("mailnews.tags.3close.tag", "3CLOSE");
user_pref("mailnews.tags.3develop.color", "#CC33CC");
user_pref("mailnews.tags.3develop.tag", "4DEVELOP");
user_pref("mailnews.tags.5processed.color", "#33CC00");
user_pref("mailnews.tags.5processed.tag", "5PROCESSED");
user_pref("mailnews.tags.6maxat.color", "#FF0000");
user_pref("mailnews.tags.6maxat.tag", "6MAXAT");
user_pref("mailnews.tags.7highat.color", "#FF6600");
user_pref("mailnews.tags.7highat.tag", "7HIGHAT");
user_pref("mailnews.tags.8spam.color", "#CCCCCC");
user_pref("mailnews.tags.8spam.tag", "8SPAM");
user_pref("mailnews.tags.aiv.color", "#000000");
user_pref("mailnews.tags.aiv.tag", "AIV");
user_pref("mailnews.tags.alpar.color", "#000000");
user_pref("mailnews.tags.alpar.tag", "ALPAR");
user_pref("mailnews.tags.ashum.color", "#000000");
user_pref("mailnews.tags.ashum.tag", "ASHUM");
user_pref("mailnews.tags.dsvet.color", "#000000");
user_pref("mailnews.tags.dsvet.tag", "DSVET");
user_pref("mailnews.tags.ivanov.color", "#000000");
user_pref("mailnews.tags.ivanov.tag", "IVANOV");
user_pref("mailnews.tags.mgru.color", "#000000");
user_pref("mailnews.tags.mgru.tag", "MGRU");
user_pref("mailnews.tags.mshubin.color", "#000000");
user_pref("mailnews.tags.mshubin.tag", "MSHUBIN");
user_pref("mailnews.tags.okhus.color", "#000000");
user_pref("mailnews.tags.okhus.tag", "OKHUS");
user_pref("mailnews.tags.pden.color", "#000000");
user_pref("mailnews.tags.pden.tag", "PDEN");
user_pref("mailnews.tags.pperf.color", "#000000");
user_pref("mailnews.tags.pperf.tag", "PPERF");
user_pref("mailnews.tags.skun.color", "#000000");
user_pref("mailnews.tags.skun.tag", "SKUN");
user_pref("mailnews.tags.spus.color", "#000000");
user_pref("mailnews.tags.spus.tag", "SPUS");
user_pref("mailnews.tags.ubash.color", "#000000");
user_pref("mailnews.tags.ubash.tag", "UBASH");
user_pref("mailnews.tags.version", 2);
user_pref("mailnews.tags.vskor.color", "#000000");
user_pref("mailnews.tags.vskor.tag", "VSKOR");

User can't see any of tags and I repeat again, this problem do not reproduce at TB 2.0 (using the same configuration and the prefs.js). This problem reproduces some times at 3.0. Not ALL of 3.0 users have this problem, just some users.

Unfortunatly, I do not know steps to reproduce this problem, but I will try to explain how the user face with problem.
So steps:
1. User has TB 2.0 and list of about 30000 letters at his Mozilla (in different folders), each of letter has 1 or 2 tags
2. User decides to update TB to ver 3.1.4
3. He updates it and continue to use old configuration (I mean prefs.js,virtual_folders.dat,*.msf,etc.)
4. So there are no any problems after update
5. User continue to work and recieved letters. So 2-3 days later he has about 31000 letters at his Mozilla (in different folders), each of letter has 1 or 2 tags (other user set tags in 5-10 minutes after receiving a new letter)
6.So, new day comes and user face the problem:
letters from 0 till 31500 have tags
but new letters from 31501 till 32000 have NOT got any tags (BUT we can see these tags at imap.log, at virtual folders, at other user's Mozillas)
after that this user can't see any change of tags at his Mozilla :(
I Tried to repair folder, but it did't fix the problem....

Comment 13

[David :Bienvenu]

If the user who has the problem with tags opens the tags ui (tools, options, display, tags tab), do they see all the tags that you would expect?

Comment 14

[Mikhail]

If the user who has the problem with tags opens the tags ui (tools, options,
display, tags tab), do they see all the tags that you would expect?

Answer:
Yes, he does

Comment 15

[David :Bienvenu]

looking at the code, I think the issue is that if the server doesn't tell us it supports \* flags, we won't store the keywords in the .msf file, and thus won't know about the tags. This is a bug on our part, but it's odd that the server isn't returning \* for settable flags...

Comment 16

[Mikhail]

David, Thank you for your investigation. 

I want to say that I am just an one of user who has problem (I am not admin of our mail server and have not got any knowledge about setting up of it).

I will send your information to our admin. You said:

"server doesn't tell us it supports \* flags" and "I notice that your imap server doesn't return "\*"

Do you think,
is it enough volume of information for mail admin to understand the reason of problem (and finding option in mail server to change in order to solve the problem)? :)

Comment 17

[Mikhail]

David, I have sent the information to our mailserver admin and unfortunatly recieve the answer that he does't understand what do you mean about:
"server doesn't tell us it supports \* flags"

:(


Could you give some explanation of it or maybe you know internet-link where we can read the information about it (in order to set up our mail server correctly and solve the problem...)

Comment 18

[Mikhail]

Ок, I have found article in the Internet about it =) It says:

Any flags that are in the Flags  but not in the PermanentFlags, can not be set permanently (i.e. across sessions). If the client attempts to store a flag that is not in the PermanentFlags list, the server will either ignore the change or store the state change for the remainder of the current session only. The PermanentFlags  list can also include the special flag "\*" (CanCreate), which indicates that it is possible to create new flags by attempting to apply those flags to the messages in the folder.

I will send it to our admin

Comment 19

[David :Bienvenu]

(In reply to comment #18)

> Any flags that are in the Flags  but not in the PermanentFlags, can not be set
> permanently (i.e. across sessions). If the client attempts to store a flag that
> is not in the PermanentFlags list, the server will either ignore the change or
> store the state change for the remainder of the current session only. The
> PermanentFlags  list can also include the special flag "\*" (CanCreate), which
> indicates that it is possible to create new flags by attempting to apply those
> flags to the messages in the folder.
> 
> I will send it to our admin

Yes, it's part of the response to the SELECT command, which flags can be stored on the server. 

We have to support both servers that support keywords and servers that don't. If the server doesn't support keywords, we just store tags in the .msf file, which means we don't want to treat absence of keywords on the server as meaning we should erase the tags from the hdr in the .msf file. We essentially use \* as evidence that we can safely set the keywords in the .msf file to whatever the server says the keywords are. If there's no \* in the select response, we don't. 

Ideally, we'd pay attention to exactly the set of flags returned by the SELECT response, but we'd not encountered a server before that supported a bunch of user-defined keywords but didn't let the user add new ones.

Comment 20

[Mikhail]

Ok, thank you. 

1. We will try to find right settings of our mail server...

2. You said that "This is a bug on our part". Are you sure that it is bug? If yes, are there any plans to fix it? May be month?half of year?1 year?

Comment 21

[David :Bienvenu]

(In reply to comment #20)
> Ok, thank you. 
> 2. You said that "This is a bug on our part". Are you sure that it is bug? If
> yes, are there any plans to fix it? May be month?half of year?1 year?

Yes, I'm sure it's a bug on our part. I would say it's more likely to be six months before a fix would get released. I'll be interested to hear if it's easy to fix on the server side.

Comment 22

[Mikhail]

Unfortunatly, we have tried but, we still not understand, if our mail server supports flag \* and how to know about it...

Our mail admin sent to me the following answer of out mail server:

TLS connection established: TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)
S: * OK [CAPABILITY IMAP4 IMAP4rev1 LITERAL+ ID AUTH=PLAIN AUTH=DIGEST-MD5 AUTH=CRAM-MD5 SASL-IR COM
PRESS=DEFLATE] maik.arqa.ru Cyrus IMAP v2.3.16 server ready
Please enter your password:
C: A01 AUTHENTICATE PLAIN XXXXXXXXXXXXXXXXXXX=
S: A01 OK [CAPABILITY IMAP4 IMAP4rev1 LITERAL+ ID LOGINDISABLED COMPRESS=DEFLATE ACL RIGHTS=kxte QUO
TA MAILBOX-REFERRALS NAMESPACE UIDPLUS NO_ATOMIC_RENAME UNSELECT CHILDREN MULTIAPPEND BINARY SORT SO
RT=MODSEQ THREAD=ORDEREDSUBJECT THREAD=REFERENCES ANNOTATEMORE CATENATE CONDSTORE SCAN IDLE LISTEXT
LIST-SUBSCRIBED X-NETSCAPE URLAUTH] Success (tls protection)
Authenticated.
Security strength factor: 256
A142 SELECT INBOX
* FLAGS (\Answered \Flagged \Draft \Deleted \Seen $MDNSent NonJunk Junk $Forwarded $Label1 $Label2 $
Label3 $Label4 $Label5 &bbseoarhbd0epgq1- &bbyepgq,bda- &bceepwqwbdw-)
* OK [PERMANENTFLAGS (\Answered \Flagged \Draft \Deleted \Seen $MDNSent NonJunk Junk $Forwarded $Lab
el1 $Label2 $Label3 $Label4 $Label5 &bbseoarhbd0epgq1- &bbyepgq,bda- &bceepwqwbdw- \*)]
* 5337 EXISTS
* 0 RECENT
* OK [UIDVALIDITY 1119157949]
* OK [UIDNEXT 1401705263]
* OK [NOMODSEQ] Sorry, modsequences have not been enabled on this mailbox
* OK [URLMECH INTERNAL]
A142 OK [READ-WRITE] Completed 




Is it possible to understand from this answer if our mail server supports flag \*? Here are the following string:

* OK [PERMANENTFLAGS (\Answered \Flagged \Draft \Deleted \Seen $MDNSent NonJunk Junk $Forwarded $Lab
el1 $Label2 $Label3 $Label4 $Label5 &bbseoarhbd0epgq1- &bbyepgq,bda- &bceepwqwbdw- \*)]

It contains "\*". But does it mean that flag "\*" is supported?

Comment 23

[David :Bienvenu]

\* means that the server supports adding user-defined keywords, yes. But I think the issue had to do with your shared folders, not the Inbox. Each folder can return a different set of permanent flags, and it could depend on which user is selecting the folder.

Comment 24

[Mikhail]

David, how did you understand from log (imap.log) I sent you before, that permanent flag \* does not support.

I look at this log and see the following...

first entrance of "PERMANENTFLAGS" was:

3212[435fd40]: ReadNextLine [stream=6892dc0 nb=33 needmore=0]
3212[435fd40]: 44e9800:mail.sicex.ru:A:CreateNewLineFromSocket: * OK [PERMANENTFLAGS (\Seen)]  

but the second entrance of "PERMANENTFLAGS" was:

3016[43f7c80]: ReadNextLine [stream=6892f40 nb=262 needmore=0]
3016[43f7c80]: 68e5800:mail.sicex.ru:A:CreateNewLineFromSocket: * OK [PERMANENTFLAGS (\Answered \Flagged \Draft \Deleted \Seen Junk NonJunk $Forwarded $MDNSent 8spam 7highat 3develop 5processed 6maxat 3close 1srochno 2open alpar ashum dsvet eshubin ivanov mgru mshubin okhus pperf rudn skun spus ubash vskor aiv mnush \*)]  


This is log of user who CAN'T see tags. So as I can see here, "\*" is exist in PERMANENTFLAGS list. 

So,finally, how did you understand from this log, that PERMANENTFLAG "\*" does not support?

Thank you for answer

Comment 25

[David :Bienvenu]

Neither flags nor permanent flags have \* in them, when selecting the Brokers shared folder:

3212[435fd40]: 44e9800:mail.sicex.ru:A:CreateNewLineFromSocket: 7 OK Completed (0.000 secs 10 calls) 3212[435fd40]: 44e9800:mail.sicex.ru:A:SendData: 8 select "quik.support.BROKERS" (CONDSTORE) 3212[435fd40]: ReadNextLine [stream=6892dc0 nb=728 needmore=0]
3212[435fd40]: 44e9800:mail.sicex.ru:A:CreateNewLineFromSocket: * FLAGS (\Answered \Flagged \Draft \Deleted \Seen $MDNSent Junk NonJunk $label3 $label1 $label4 $label2 &bbyepgq,bda- $label5 &bbseoarhbd0epgq1- &bb4emqrabdaemqqwbeieswqybdaenqrcbeeetw- $Forwarded mbul &bb4eqgq7bd4engq1bd0epg- &bceepwqwbdw- &bceepwqwbdw-a &bb4eowq+bdyenqq9bd4- &bbseoarhbd0epgq1-a vskor mshubin ae_autoextract pperf eshubin ivanov a_! a_&bbieswrb- visok maxim alpar ashum dsvet mgru 
3212[435fd40]: rudn spus ubash z_max z_vis _vis palpa pashu pdsve peshu pivan pmgru pzmax pvsko prudn pspus alparh ashumil zzvisok zzmaxim ubasha spusto rudnev mgrudc efhim fkofk kfokd kfokf kfokg kfokl kfokt nmkhi noprs uvrso mgrud rudne spust zzmax zzvis 1srochno 2open 3close 3develop 5processed 6maxat 7highat 8spam skun okhus mnush aiv) 3212[435fd40]: ReadNextLine [stream=6892dc0 nb=33 needmore=0]

but, when selecting the inbox,

3016[43f7c80]: 68e5800:mail.sicex.ru:A:CreateNewLineFromSocket: * FLAGS (\Answered \Flagged \Draft \Deleted \Seen Junk NonJunk $Forwarded $MDNSent 8spam 7highat 3develop 5processed 6maxat 3close 1srochno 2open alpar ashum dsvet eshubin ivanov mgru mshubin okhus pperf rudn skun spus ubash vskor aiv mnush) 3016[43f7c80]: ReadNextLine [stream=6892f40 nb=262 needmore=0]
3016[43f7c80]: 68e5800:mail.sicex.ru:A:CreateNewLineFromSocket: * OK [PERMANENTFLAGS (\Answered \Flagged \Draft \Deleted \Seen Junk NonJunk $Forwarded $MDNSent 8spam 7highat 3develop 5processed 6maxat 3close 1srochno 2open alpar ashum dsvet eshubin ivanov mgru mshubin okhus pperf rudn skun spus ubash vskor aiv mnush \*)]   3016[43f7c80]: ReadNextLine [stream=6892f40 nb=15 needmore=0]

The flags are different for each folder, so you need to look at each permanent flags response when each folder is selected.

Comment 26

[Mikhail]

I see now =)

Thank you!!

Comment 27

[Mikhail]

Hello!

Are there any news about this issue?

Comment 28

[Mikhail]

Hi!

Mozilla TB ver 5.0 was announced. Was this bug fixed in ver 5.0?
Unfortunatly most of my colleagues still works with Mozilla ThunderBird ver 2.0 because they still have a problem with 3.0 :(

Comment 29

[Mikhail]

Hi!

Today's version of Mozilla TB is 7.0 but bug still not resolved :(
We still works with Mozilla ThunderBird ver 2.0. Are there any ideas about solving problem?

Comment 30

[Mikhail]

Hi again!

Today's version of Mozilla TB is 11.0 but bug still not resolved :(
We still works with Mozilla ThunderBird ver 2.0. Are there any ideas about solving problem?

Comment 31

[Ludovic Hirlimann [:Usul]]

Mikhail can you answer David's question about 
The flags are different for each folder, so you need to look at each permanent flags response when each folder is selected.. 

And sorry for being silent for so long.

Comment 32

[Mikhail]

Hello! :)

1. Could you tell me, what question do you mean? 

2. As we found out from logs, mail-server I work with does not support flag \*

3. Unfortunatly, my server-administrators do not know how to fix or how to switch it on (flag ). 

4. FACT is that problem is exist at TB 3.0 and it DOES NOT exist at 2.0

If you need any more information I will be glad to help you (if can)

Comment 33

[Mikhail]

Hello!

Any news about this issue? We are still using 2.0 :(

Comment 34

[Mikhail]

2 years and 3 month from the day this bug was submitted...the problem still exist.
We have to use 2.0 to solve this problem...

Comment 35

[Mikhail]

3 years and 1 month from the day this bug was submitted...the problem still exist.

Can anybody help me?

Comment 36

[Mikhail]

any news?

Comment 38

[Mikhail]

6.5 years from the day this bug was submitted...the problem still exist.

Can anybody help me?

Comment 39

[Mikhail]

By the way, the problem still does not occur at Thunderbird ver 2.0 and occurs at Thunderbird ver >= 3.0

Comment 40

[Wayne Mery (:wsmwk)]

Gene didnt we have some dicussion about something like this in the last few months?

Comment 41

[gene smith]

(In reply to Wayne Mery (:wsmwk) from comment #40)
> Gene didnt we have some dicussion about something like this in the last few
> months?

I think there was something about flags on a message (like, starred) and how it was set in one app and not always seen in tb. However, I don't think there was a definite problem in tb. Maybe if imap server supports IDLE or if the force select thing we put in for charter/spectrum might help this user. 
I have read through this once and don't really understand what the problem is. I will try to parse this again. I've never heard of or worked with these "user defined tags" that this refers to. Wayne, if you understand and can explain this some to me it might help, but I will try going through this again...
-gene

Comment 42

[gene smith]

Not sure if this is what is described here but I can define a new tag called gds0 and assign it to a message on account0 on machine0. Then on machine1 I can define the new tag gds0 and I see the tag (message summary changes to the assigned color) at the same message in account0. 

If IDLE imap capability is supported, all I have to do is roll the mouse over the message on machine1 and its color changes. Otherwise, I may need to do "get messages" or click on the message for the color the change. I did this with several accounts and they all worked and at least 1, charter, didn't support IDLE.

I looked at the SELECT responses and I do see my gds0 tag/flag the and \* in the flags list. "\*" is what "David :B" was telling the reporter he should see, but apparently he was not seeing it on some selected mailboxes.

Not sure if the reporter is still interested or even still using tb after all this time but about the only way I could possibly duplicate and debug this would be to obtain a test account into his server and have him set some tags on messages and then see if I can see them, and, if not, why not. I will set "needinfo" to the reporter.

By the way, I didn't manually put the tags into the pref.js file. I just used the normal user interface. I can see info about my tag "gds0" in the file.

Comment 43

[Mikhail]

As Wayne asked me, I copy here my Reply to his questions "Am I still using ThunderBird?"

The Answer is:

===============================
Hello!

Our group of 15 people still using thunderbird for our needs. We still using technology of custom tags. We have separate tag for each person in group and some more additional tags for our needs.
  1/3 of group still using TB ver 3 or higher (ver 38, 45 etc). These group of people has permissions to set tags on mails in the imap folder.

2/3 of our group still use thunderbird ver 2, because these people do not have permissions to set tags. If they update their TB they will face the same problem. It was proven by hundreds of tests. So, the problem mentioned 8 years ago still exists and this is the reason they can not update their TB. We have developed some addons for TB: our developers have to release 2 versions of these addons (one for tb ver 2 and another for tb ver 3 or higher for each addon...it is not very convenient...).

If I can help you with this issue (by providing imap logs or whatever), please, tell me what shall I do...

Thank you.

Best regards
Mikhail
================================

Comment 44

[Mikhail]

Hello, everybody!

Any updates? I still hoping to get update in some nearest future...

Comment 45

[Mikhail]

Dear developers, Hello!

I still need help! If you need any logs in order to catch the bug, please tell me!

Comment 46

[gene smith]

Mikhail, As I mentioned before, I was unable to duplicate the problem you observed. It may be server or server version specific. My suggestion is for you to provide me with a temporary guest email account on your server so I can more easily see and debug the problem you are seeing. I will need the server url or ip address, and any relevant login information (e.g., password, connection type etc). This would be the ideal path to move forward. You can send the info to my bugzilla profile email address if you prefer, instead of posting it here.
If this is not possible, then a log for a user that can't see the tags is 2nd best. Google for "thunderbird logging" and be sure to use the env vars NSPR_LOG_MODULES and NSPR_LOG_FILE and *not* the MOZ_* vars since they are currently only good for daily and beta releases. Just attach it above; no need to zip it.

Comment 47

[Mikhail]

Thank you for your response. Access to the server is not possible, I am afraid, due to security reasons. I will try do the log (but 8 years ago I have done it allready, see my first comments)/

Comment 48

[Mikhail]

Attachment: tb2018.zip

Comment 49

[Mikhail]

Hello! I attached imap.log and 1.docx with description and screenshots.
Password for the archive: 12345

Please, help me to solve this problem...
If you need more information, I will do my best to provide you with it.

Comment 50

[Mikhail]

Comment on attachment 9012128 [details]
tb2018.zip

Password for the archive is: 12345

Comment 51

[gene smith]

Thanks for the log and description attachment. I am reviewing the previous comments for this bug from the beginning since I don't remember all the details since we last corresponded. Then I will look at details of your latest attachment.

Comment 52

[gene smith]

I assume the BR_2 folder is shared between the two accounts?  On mshubin account you write the test email and put it in BR_2 and set the two flags (a.k.a, tags or keywords). Then on quikutils account you see it in shared BR_2 but don't see the flags set (but it does appear in the virtual search folder so tb had to see the flags it seems).

At this point in time, to me, it looks like a visualization problem and not an imap or server configuration problem.

I can see in the log that the two flags (mshubin and 2open) are fetched OK for BR_2:
>2018-09-26 10:22:45.188000 UTC - 8412[154a0040]: 16670000:mail.arqa.ru:S-quik.support.BR_2:CreateNewLineFromSocket: * 2 FETCH (FLAGS (\Seen mshubin 2open) UID 1000222468)
So imap has supplied the correct information (also confirmed by the message's appearance in the virtual search folder).

The FLAGS response to SELECT for folder BR_2 contains all the standard flags plus your numerous custom flags, including mshubin and 2open. The PERMANENTFLAGS response to SELECT only reports \Seen (and no \*) which is OK since quikutils account has limited rights and you are not trying to set or create new flags on this account, just see and display existing flags.

Does this problem only occur on accounts with limited rights like quikutils? Asked another way, if accounts with full rights like mshubin set one of your custom flags on a message in a shared folder, can other accounts with equal or higher rights see the flags set in the same message in the shared folder? (Of course, I am asking this about a newer tb version >= 52.x.)

Comment 53

[gene smith]

Not sure there is really a problem in tb. It may be due to old and possibly somewhat incompatible profile data. Earlier you talked about editing pref.js to define new tags. It may be OK but I would recommend using the GUI instead. 

I tried to duplicate the problem by setting a new tag for a message on machine0. I did this by selecting the message and doing Message|Tag|New Tag... menu item. I added the tag "test-new-tag" and selected a color and the message appeared with the selected color and the new tag with the same color showed on the header display.

I then started tb on machine1 with the same account. The message that I tagged on machine0 did not show a tag at all, like you see on account quikutils. I also noticed in imap log for machine1 that the flag test-new-tag *was* fetched, so the imap server now has it stored. 

Next I went to the Preferences/Options tb screen on machine1 and created a new tag with the same name and color: test-new-tag. This doesn't store anything to the imap server but just lets tb know how to display the tag if it is fetched for a message. A restart of tb on machine1 causes the message marked with tag test-new-tag on machine0 to appear also marked properly on machine1.

A problem with my theory is that the search folder you have sees the tags, so they must be already defined on quikutils it seems. Therefore, on account quikutils I would recommend removing all the custom tag definitions from prefs.js and put them back in using the preference/options screens. If that doesn't help, I recommend creating a brand new profile for quikutils and test again with a completely fresh and compatible profile.

Here's the tag entries for my pref.js:

user_pref("mailnews.tags.$label1.color", "#FF0000");
user_pref("mailnews.tags.$label1.tag", "Important");
user_pref("mailnews.tags.$label2.color", "#FF9900");
user_pref("mailnews.tags.$label2.tag", "Work");
user_pref("mailnews.tags.$label3.color", "#009900");
user_pref("mailnews.tags.$label3.tag", "Personal");
user_pref("mailnews.tags.$label4.color", "#3333FF");
user_pref("mailnews.tags.$label4.tag", "To Do");
user_pref("mailnews.tags.$label5.color", "#993399");
user_pref("mailnews.tags.$label5.tag", "Later");
user_pref("mailnews.tags.test-new-tag.color", "#CC33CC");
user_pref("mailnews.tags.test-new-tag.tag", "test-new-tag");
user_pref("mailnews.tags.version", 2);

I'm not sure, but if you still want to edit this file, shutdown tb and I would recommend removing at least these, if present, like shown in comment 0 since I don't think these "labels.description" items are used anymore:

user_pref("mailnews.labels.description.33", "OKHUS");
user_pref("mailnews.labels.description.34", "AIV");
user_pref("mailnews.labels.description.35", "MNUSH");

Also, this may be important, make sure the correct version line is present, i.e.:
user_pref("mailnews.tags.version", 2);

...but I would really recommend making a new profile from scratch and avoid low level file edits...

Comment 54

[gene smith]

On page 4 of your docx file it show the messages in VIRTUAL_FOLDER. They were put there only because both tag were seen, right? However, this display doesn't show the messages with the expected color and it doesn't show the tags listed on the header pane, right? 
Not sure what this means. Do you only see this when testing accounts that don't have permission (rights) to set and create new tags on the imap server?

Comment 55

[gene smith]

README first!
Ok, I rebuilt tb (for machine1 described above) and caused it to ignore the PERMANENTFLAGS item \* so now tb on machine1 thinks the server doesn't support setting user defined flags. Now when I set the flag for a message on machine0 (released 52.x tb build), it doesn't appear on machine1. This seem very close to what reporter Mikhail is saying. 

However, I don't see the message getting put into the virtual search folder. That seems like the only inconsistency.

So probably not a profile or pref.js problem. Looking deeper to see what is going on. Machine1 tb should not ignore user defined flags set by other client (machine0) just because server says they can't be set by this (machine1) client.

Comment 56

[Mikhail]

(In reply to gene smith from comment #54)
> On page 4 of your docx file it show the messages in VIRTUAL_FOLDER. They
> were put there only because both tag were seen, right? However, this display
> doesn't show the messages with the expected color and it doesn't show the
> tags listed on the header pane, right? 

Correct! You have just recapped the main idea of this bug! :) 

The user without permission does not see at the imap folder expected colors and the names of the tags on the mail. 
In the real life we have hundreds of mails in the imap folder. About 99% of them have tag "3CLOSE" and grey color (this color does not strike the eye), but 1% of mails (usually the most recent ones) have a combination of "2OPEN" tag and personal tag, for example "MSHUBIN" (as in my example) or ALEXANDER, MARIA or any another name. "2OPEN" is set up usually like a bright color, for example blue. 

So, the user goes to imap folder and see only black colored mails (the defualt color). The only workaroud is to create a virtual folder: it helps, I mean TB understands that it is neccessary to put mails with a particular combination of TAGs at this folder but still does not show colors and tags in the imap folder itself.

Comment 57

[Mikhail]

(In reply to gene smith from comment #55)
> So probably not a profile or pref.js problem. Looking deeper to see what is
> going on. Machine1 tb should not ignore user defined flags set by other
> client (machine0) just because server says they can't be set by this
> (machine1) client.

Before woking with TAGs we deleted all the default tags. After that we closed TB and checked pref.js to be sure that all infromation about defualut tags had been deleted from pref.js. If not, we deleted it manually. Only after that we added our own tags to the pref.js and started to work with them.

Comment 58

[gene smith]

(In reply to David :Bienvenu from comment #15)
> looking at the code, I think the issue is that if the server doesn't tell us
> it supports \* flags, we won't store the keywords in the .msf file, and thus
> won't know about the tags. This is a bug on our part, but it's odd that the
> server isn't returning \* for settable flags...

From what I can see too this is probably the problem. I think the tag code was "improved" somewhere before or at the v3 release and caused the regression. See http://kb.mozillazine.org/Tags for evidence. 

> but it's odd that the server isn't returning \* for settable flags...

Apparently for the cyrus server that you are using it is not "odd" that setting custom tag is not allowed since the folder has limited privilege:

2018-09-26 10:22:45.188000 UTC - 8412[154a0040]: 16670000:mail.arqa.ru:S-quik.support.BR_2:CreateNewLineFromSocket: * MYRIGHTS quik.support.BR_2 lrsp

l = ok to list and subscribe
r = can do select
s = can set and store \SEEN flag
p = ok to post to folder
but w not present so can't set flags other than \SEEN and \DELETED which includes user defined custom tags!

Do user defined custom tag work for Inbox for account quikutils? It seems to have full permissions, including w:

2018-09-26 10:22:45.157000 UTC - 9796[154a0820]: 1667b800:mail.arqa.ru:S-INBOX:CreateNewLineFromSocket: * MYRIGHTS INBOX lrswipkxtecdan

Anyhow, trying to find a fix. Sorry it has been 8 years! Hopefully it won't be much longer.

Comment 59

[Mikhail]

It is OK for users such as quikutils to have full permission for Inbox. Seems like quikutils does not have any problem with custom tags in the Inbox. 

The problem is only with shared imap folders (such as BR_2). quikutils has limited permissions to this folder, that's why he has a problem mentioned above. mshubin user has extended permissions to imap folder BR_2, so he can set custom tags and does not have any problem with it.

Comment 60

[Mikhail]

If you need more information, logs, screenshots or whatever, please, inform me.

Comment 61

[gene smith]

Good to know Inbox works OK. 

I think a possible solution is to go ahead and let tb ignore the lack of the \* in the PERMANENTFLAGS response and treat it like it is present anyhow. The user at low permission folders like quikutils at folder BR_2 can still attempt to set a custom tag but it should be rejected by the server. It will still be stored in the .msf database for the current session and look like it was applied but will go away on the next tb start up. I will need to try and test this, of course.

Right now, for you what happens when quikutils tries to set a custom tag on a message in BR_2? Does it seem to accept it? Does nothing happen? Is there an error reported?

Comment 62

[gene smith]

This bug is a regression of the fix in Bug 370440. It was put into v3 less than a year before this bug was originated by Mikhail. 
As a test, I set the return statement seen in the patch associated with Bug 370440 to its pre-3.0 state as seen at the end of this patch fragment and it fixes Mikhail's problem:

-nsresult nsImapMailFolder::HandleCustomFlags(nsMsgKey uidOfMessage, nsIMsgDBHdr *dbHdr, nsCString &keywords)
+nsresult nsImapMailFolder::HandleCustomFlags(nsMsgKey uidOfMessage,
+                                             nsIMsgDBHdr *dbHdr,
+                                             PRUint16 userFlags,
+                                             nsCString &keywords)
 {
   ToLowerCase(keywords);
   PRBool messageClassified = PR_TRUE;
   // Mac Mail uses "NotJunk"
   if (keywords.Find("NonJunk", PR_TRUE /* ignore case */) != -1 || keywords.Find("NotJunk", PR_TRUE /* ignore case */) != -1)
   {
     nsCAutoString msgJunkScore;
     msgJunkScore.AppendInt(nsIJunkMailPlugin::IS_HAM_SCORE);
@@ -4248,33 +4251,37 @@ nsresult nsImapMailFolder::HandleCustomF
   if (messageClassified)
   {
     // only set the junkscore origin if it wasn't set before. 
     nsCString existingProperty;
     dbHdr->GetStringProperty("junkscoreorigin", getter_Copies(existingProperty));
     if (existingProperty.IsEmpty())
       dbHdr->SetStringProperty("junkscoreorigin", "imapflag");
   }
-  return dbHdr->SetStringProperty("keywords", keywords.get()); <---calling this unconditionally fixes this bug 583677
+  return (userFlags & kImapMsgSupportUserFlag) ?
+          dbHdr->SetStringProperty("keywords", keywords.get()) : NS_OK;
 }

The reason for the use of "userFlags" is discussed in Bug 370440 Comment 22. I need to understand more exactly what this does before backing this change out. Or there may be a completely better way to solve the problem.

Comment 63

[Mikhail]

> Right now, for you what happens when quikutils tries to set a custom tag on
> a message in BR_2? Does it seem to accept it? Does nothing happen? Is there
> an error reported?

So, this is not quikutils's duty to set different TAGs: the user should only see them correctly. That's why he does not have appropriate permissions on the mail server. Nevertheless, if he tries to set a tag in IMAP folder it is set correctly (No errors occur). But, of course, TAGs are stored somewhere locally at the computer of quikutils and do not go to the mail server and of course, nobody else sees them. Even if quikutils tries to "Fix folder" or restart ThunderBird these tags (which were set by quikutils) still at their place), BUT if quikutils unsubscribes/subscribes of/to IMAP folder, all TAGs he set previously disappear (I guess it is because Thunderbird got actual tags status from the mail server).

Comment 64

[gene smith]

I now have my local Dovecot server providing a new and limit rights account, like your quikutils, with a shared folder like your BR_2, called Inbox.555. With unchanged tb I can set a tag, test-new-tag, on a message in Inbox.555 with new account and it stays there even on tb restart. However, I see that it is *not* sent to the server so if folder is repaired, or message copied or unsubscribed/subscribed (all cause re-download from server) the flag goes away. The fix with Bug 370440 ensures that the tag stays in place as long as the message is not re-downloaded.

When I apply my fix shown in comment 62 and rebuild tb, it fixes the problem you see but now the "private" tag set by the minimal rights account does not survive a tb restart. Since as you say "is not quickutils's duty to set different tags" this would not be a problem for you. However, other users, like the reporter of Bug 370440, may see this as a problem and another regression.

So still trying to determine what to do.

Comment 65

[gene smith]

Attachment: 583677-allow-all-configured-tags-in-imap-response-to-be-displayed.patch

So far the only reliable (and fairly simple) way to fix this problem that I have found is to add a new preference. Since the reporter's organization is the only user I know of that has reported this very valid and long ago reported problem and a general solution is not trivial, it seems reasonable to provide them with a special pref that will fix the problem for them but not affect other users since it defaults to false.

But to make sure this actually fixes the problem for the reporter, I would like to provide him with a "try" build he can run. But, Jorg, before that I want to make sure that the reporter is willing to test the "try" build. Therefore I am setting the NI and requesting confirmation from reporter Mikhail before you go to that trouble.

Mikhail, if you are willing to test the "try" build, please respond in the next comment so Jorg can do the build. You can download it from the link that Jorg will provide. When you run it you need to go to Options/Advanced/Config Editor and find the new preference 

mail.imap.unconditionally_store_keywords

and set it to true, then restart thunderbird for it to take effect. This should fix this issue.

Also, not sure but maybe Jorg needs to know what platform you are running: Windows, Linux or OSX. We may need to know this to direct you to the proper download link. ...Thanks!

Comment 66

[Magnus Melin [:mkmelin]]

gene: we should really get you level 1 access so you can push to try and such. Please file a bug and cc me. I'll vouch for you. Instructions here: https://www.mozilla.org/en-US/about/governance/policies/commit/

Comment 67

[Mikhail]

Yes, please, I am ready to test a new build. We have prepared a computer for such testing already. 
We use Windows 10 x64. There is a very low probability that somebody will use Linux or OSX in the nearest future. So, please, prepare a build for Windows 10 x64.

Comment 68

[Mikhail]

(In reply to Mikhail from comment #67)
> Yes, please, I am ready to test a new build. We have prepared a computer for
> such testing already. 
> We use Windows 10 x64. There is a very low probability that somebody will
> use Linux or OSX in the nearest future. So, please, prepare a build for
> Windows 10 x64.

One more detail. We need ThunderBird x32 (not X64) it is because we use some add-ons which are not available for ThunderBird x64 yet. Thank you.

Comment 69

[Jorg K (CEST = GMT+2)]

You can also pull the comm-esr60 repository and do try pushes with it. That gives the user something close to TB 60 ESR to work with. As we know, only few add-ons work on TB 64 Daily.

Comment 70

[Jorg K (CEST = GMT+2)]

OK, this needed bug 1428097 applied first and then it still didn't quite apply to comm-esr60. I made it apply, so here's the try run for Win32 and TB 60 ESR:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=bcf6453b18a4063aaf00519d76b3aac419678b6c

Next time, as Magnus suggested, you can do it yourself ;-)

Reporter, you need to click onto the "B", then "Job Details" below, and then download target.zip or target.installer.exe.

Comment 71

[Jorg K (CEST = GMT+2)]

Comment on attachment 9013514 [details] [diff] [review]
583677-allow-all-configured-tags-in-imap-response-to-be-displayed.patch

Let's clear the flags for now.

Comment 72

[Mikhail]

(In reply to Jorg K (GMT+2) from comment #70)
> OK, this needed bug 1428097 applied first and then it still didn't quite
> apply to comm-esr60. I made it apply, so here's the try run for Win32 and TB
> 60 ESR:
> https://treeherder.mozilla.org/#/jobs?repo=try-comm-
> central&revision=bcf6453b18a4063aaf00519d76b3aac419678b6c
> 
> Next time, as Magnus suggested, you can do it yourself ;-)
> 
> Reporter, you need to click onto the "B", then "Job Details" below, and then
> download target.zip or target.installer.exe.

Is it ready for being downloaded? I have tried to go the link you have mentioned, then I clicked the "B", then "Job Details" below but after that nothing happened. I mean The window below the "Job Details" was empty and I have not found target.zip or target.installer.exe. thank you.

Comment 73

[Jorg K (CEST = GMT+2)]

The B needs to go green first. Takes about an hour to build.

Comment 74

[Mikhail]

Ok. I have tested! Looks like it works! :) See the proof in the attachment in the next message.

1. I have installed the TB you have prepared for me.
2. Switched mail.imap.unconditionally_store_keywords to the TRUE
3. Restart TB
4. All TAGs became visible.
5. I sent a few more emails and set new TAGs. All TAGs have appeared successfully!

Comment 75

[Mikhail]

Attachment: testing.jpg

Comment 76

[gene smith]

(In reply to Magnus Melin [:mkmelin] from comment #66)
> gene: we should really get you level 1 access so you can push to try and
> such. Please file a bug and cc me. I'll vouch for you. Instructions here:
> https://www.mozilla.org/en-US/about/governance/policies/commit/

Thanks Magnus, I have submitted the required "bug".

Jorg, Sorry for the merge/build problems. I made the changes on top the the other patch. Also, been a while since I pulled in the latest code.

Mikhail, Glad it's working again!

Comment 77

[Mikhail]

> Mikhail, Glad it's working again!

I am glad as well, thank you :) What are the further steps? Shall I just wait for the release of ThunderBird with this fix?

Comment 78

[Jorg K (CEST = GMT+2)]

A release containing this will not be for a few months. You can keep using the version we provided, it's TB 60.2.1 with some IMAP improvements although it says "Daily".

Comment 79

[gene smith]

(In reply to Jorg K (GMT+2) from comment #69)
> You can also pull the comm-esr60 repository and do try pushes with it. That
> gives the user something close to TB 60 ESR to work with. As we know, only
> few add-ons work on TB 64 Daily.

I cloned comm-esr60 for tb and mozilla and it built OK it seems. (Couldn't get a pull of esr60 to work with my existing local comm-central tree.) Anyhow when I try to run it, it fails and all I see is 

Couldn't load XPCOM.

I also updated by existing comm-central tree to latest trunk and it builds and runs OK.
 
Probably a shared lib problem. It appears all the libs are in place including libxul.so. Can't find much on bugzilla or google other than "reinstall".

edited 2019-2-28: Tried this again and still got "Couldn't load XPCOM" after clone and build of comm-esr60. The problem is that when I run mozilla-esr60/obj-x86_64-pc-linux-gnu/dist/bin/thunderbird it is a symlink pointing to mozilla-esr60/obj-x86_64-pc-linux-gnu/comm/mail/app/thunderbird binary and XPCOM isn't found there. But if I run mozilla-esr60/obj-x86_64-pc-linux-gnu/dist/bin/thunderbird-bin binary (not a symlink and same as .../app/thunderbird), tb runs since the libs are in the same path. I don't see this problem when I clone and build comm-central since symlinks aren't used (both .../bin/thunderbird and .../bin/thunderbird-bin are real files and identical.

Comment 80

[Jorg K (CEST = GMT+2)]

Looks like you misunderstood. You just have comm-esr60 locally, apply the patch and push to try. No local building of TB 60, although that should be possible as well. But it needs older versions of the compiler, or gcc instead of clang, and an older version of rust. Too much hassle.

Comment 81

[Magnus Melin [:mkmelin]]

Would it be be possible to instead of a pref, make it so that user defined keywords are never *removed*, when the mailbox permissions do not allow setting them?

Perhaps another option worth considering is that for tags that don't get set server side, we make them named with leading underscore, like _example (to note "private"). Then there would be no conflict with the remote keywords.

Comment 82

[gene smith]

(In reply to Magnus Melin [:mkmelin] from comment #81)
> Would it be be possible to instead of a pref, make it so that user defined
> keywords are never *removed*, when the mailbox permissions do not allow
> setting them?

Hard to tell user defined and local only from those set by other client, but ...

> 
> Perhaps another option worth considering is that for tags that don't get set
> server side, we make them named with leading underscore, like _example (to
> note "private"). Then there would be no conflict with the remote keywords.

This idea about the leading underscore might be a solution to avoid the pref. I will check.
I assume you mean that tb prepends the '_' to whatever tag name the user chooses and the user doesn't enter the leading '_' himself, so the user never sees the '_'.

Comment 83

[Magnus Melin [:mkmelin]]

Yes, for the case when tags can't be stored server side.

Comment 84

[gene smith]

Mikhail,
What is the highest number of user defined tags that you have defined in tb? Looking at your imap log it appears that there are about 75 user defined tags listed in the SELECT--FLAGS response returned by your imap server for BR_2 mailbox:

>2018-09-26 10:22:45.188000 UTC - 8412[154a0040]: 16670000:mail.arqa.ru:A:CreateNewLineFromSocket: * FLAGS (\Answered \Flagged \Draft \Deleted \Seen $MDNSent Junk NonJunk $label3 $label1 $label4 $label2 &bbyepgq,bda- $label5 &bbseoarhbd0epgq1- &bb4emqrabdaemqqwbeieswqybdaenqrcbeeetw- $Forwarded mbul &bb4eqgq7bd4engq1bd0epg- &bceepwqwbdw- &bceepwqwbdw-a &bb4eowq+bdyenqq9bd4- &bbseoarhbd0epgq1-a vskor mshubin ae_autoextract pperf eshubin ivanov a_! a_&bbieswrb- visok maxim alpar ashum dsvet mgru 
>2018-09-26 10:22:45.188000 UTC - 8412[154a0040]: rudn spus ubash z_max z_vis _vis palpa pashu pdsve peshu pivan pmgru pzmax pvsko prudn pspus alparh ashumil zzvisok zzmaxim ubasha spusto rudnev mgrudc efhim fkofk kfokd kfokf kfokg kfokl kfokt nmkhi noprs uvrso mgrud rudne spust zzmax zzvis 1srochno 2open 3close 3develop 5processed 6maxat 7highat 8spam skun okhus mnush aiv pden 5wish sgor ilis zovd mrom ezaytsev &bbcepqqwbd0eoarp- stvor qout mter
>2018-09-26 10:22:45.188000 UTC - 8412[154a0040]:  5qout agor)

Is this typical?

Also, do you actually use all of these tags?  Just curious.

Thanks.

Comment 85

[gene smith]

>Perhaps another option worth considering is that for tags that don't get set server side, we make them named with leading underscore, like _example (to note "private"). Then there would be no conflict with the remote keywords.

This sounds easy but where the tag is first stored in the database there doesn't seem to be a way to know at that point whether the mailbox allows new user tags or not. 

>Would it be be possible to instead of a pref, make it so that user defined keywords are never *removed*, when the mailbox permissions do not allow setting them?

This is what I have done but it took some brute force. I had to store the user defined tags that are returned from the SELECT/FLAGS response for the mailbox. There can be a lot of them as I show in comment 84 above. Then when the actual FLAGS that are set in the server and returned for each message, they are compared to the defined flags and the currently stored flags to determine what is then stored back into the database. So each message requires iteration through arrays of keywords (strings). But this only occurs if the mailbox doesn't support keyword setting so that local-only keywords are not reset. (This is a very simple explanation of this proposed changed and I can provide more details if needed.)

I have now done a win32/optimized try build, on top of esr60 like Jorg did, that can be accessed at 

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=2413b34b6ab63a0de66a3ae1dd69b31a7e54606d

This does not require the pref (no longer available) and it also allows private/local tags to be set and retained for mailboxes that don't allow storing user tags in the server. Of course, this is not a requirement of reporter Mikhail. However, it is probably more CPU intensive since the string iterations and compares occur (but only for the read-only keyword mailboxes).

It seems to be working with dovecot server I have set up to emulate Mikhail's cyrus imap server's keyword read-only mailbox.

For debugging purposes I have added a new logging item, called IMAP_KW (imap keyword). It can be used by itself or with the IMAP logging, e.g., by itself:
MOZ_LOG=IMAP_KW:5
or with IMAP:
MOZ_LOG=IMAP:5,IMAP_KW:5

IMAP_KW:5 (all log items are debug level) will produce a lot of output into the log file but it may be helpful to enable it, at least for a while.

Comment 86

[Magnus Melin [:mkmelin]]

(In reply to gene smith from comment #85)
> >Perhaps another option worth considering is that for tags that don't get set server side, we make them named with leading underscore, like _example (to note "private"). Then there would be no conflict with the remote keywords.
> 
> This sounds easy but where the tag is first stored in the database there
> doesn't seem to be a way to know at that point whether the mailbox allows
> new user tags or not. 

Not at that point yeah, the info would need to be passed from a higher level. I mean, at the moment "tag" is pressed for a message we do know the folder, and presumably therefore what that folder allows (if we don't keep track of that currently, at least we could). 

Regarding possible increased CPU I wouldn't be too worried. Looping through 100s of strings is hardly even measurable cup time when you're doing it for a single message. If you're doing it for 100000 messages then it may add up to something.

Comment 87

[Mikhail]

(In reply to gene smith from comment #84)
> Mikhail,
> What is the highest number of user defined tags that you have defined in tb?
> Looking at your imap log it appears that there are about 75 user defined
> tags listed in the SELECT--FLAGS response returned by your imap server for
> BR_2 mailbox:
> 
> >2018-09-26 10:22:45.188000 UTC - 8412[154a0040]: 16670000:mail.arqa.ru:A:CreateNewLineFromSocket: * FLAGS (\Answered \Flagged \Draft \Deleted \Seen $MDNSent Junk NonJunk $label3 $label1 $label4 $label2 &bbyepgq,bda- $label5 &bbseoarhbd0epgq1- &bb4emqrabdaemqqwbeieswqybdaenqrcbeeetw- $Forwarded mbul &bb4eqgq7bd4engq1bd0epg- &bceepwqwbdw- &bceepwqwbdw-a &bb4eowq+bdyenqq9bd4- &bbseoarhbd0epgq1-a vskor mshubin ae_autoextract pperf eshubin ivanov a_! a_&bbieswrb- visok maxim alpar ashum dsvet mgru 
> >2018-09-26 10:22:45.188000 UTC - 8412[154a0040]: rudn spus ubash z_max z_vis _vis palpa pashu pdsve peshu pivan pmgru pzmax pvsko prudn pspus alparh ashumil zzvisok zzmaxim ubasha spusto rudnev mgrudc efhim fkofk kfokd kfokf kfokg kfokl kfokt nmkhi noprs uvrso mgrud rudne spust zzmax zzvis 1srochno 2open 3close 3develop 5processed 6maxat 7highat 8spam skun okhus mnush aiv pden 5wish sgor ilis zovd mrom ezaytsev &bbcepqqwbd0eoarp- stvor qout mter
> >2018-09-26 10:22:45.188000 UTC - 8412[154a0040]:  5qout agor)
> 
> Is this typical?
> 
> Also, do you actually use all of these tags?  Just curious.
> 
> Thanks.

The short answer - "yes".

The more detailed answer is:

I looked thru these list of tags.  I may say that:
- my team (about 15 people) are using 25 of the mentioned user-defined tags actively every day (but each particular email usually contains not more than 3-4 tags at a single moment in time);
- about 15 of the mentioned tags we do not use anymore (but it is possible that these tags still exist on old emails, I think, that's why the mail server returns these tags);
- about 35 of mentioned tags seem to be created by my colleagues from another team who use this mail-server as well (not sure, but I may assume that at least 50% of these 35 tags they actively use).

Comment 88

[gene smith]

Ok, thanks for the answer. When I tested with dovecot server and I created a tag, there didn't seem to be a way to deleted them from the server once they are created but no longer needed. So that can accumulate.

Hopefully you can try the "try" build above and let me know if it works OK.

Comment 89

[Mikhail]

> Hopefully you can try the "try" build above and let me know if it works OK.
I downloaded and tested this:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=2413b34b6ab63a0de66a3ae1dd69b31a7e54606d

Seems like tags at TB 60.2.2 (32-bit) work OK.

Comment 90

[gene smith]

Attachment: tag-fix-from-try-server.diff

Not sure but thought I read that try server output and diff goes away after a period of time. Here's the diff saved from the treeherder/try site. 
Anyhow, guess I need to research some more Magnus's other idea about prepending tags with _?.

Comment 91

[Mikhail]

Hello, gene smith and Magnus Melin!

I and my team would like to start working with the version you provided me earlier. Could you please build for me x64-version of ThunderBird Daily which includes this fix (I asked you to build x32 before, but now we are seemed to be ready to use x64)?  If you build Russian-locale it would be great! If Russian is not possible, English-locale would be ok for now.
Thank you!

Comment 92

[gene smith]

I am sure I can do a win64 try build but not sure about the localization.

At this time we have 3 choices. 1: Add my new pref. 2. My change that parses are all the flags. 3. Magnus's leading underscore.

I personally prefer the change that just adds the new pref since it directly fixes your problem and is low risk of breaking anything or affecting performance. 

But I know that Magnus doesn't like adding new "hidden prefs".

Doing the leading underscore method that Magnus wants is probably a bit outside my .js programing experience. I haven't done anything more on this yet, other than my initial rejection of this approach, described above, after looking at the code.

I assume you have been testing with this choice (2 above) that parses all the flags:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=2413b34b6ab63a0de66a3ae1dd69b31a7e54606d
?

Comment 93

[Mikhail]

> 
> I assume you have been testing with this choice (2 above) that parses all
> the flags:
> https://treeherder.mozilla.org/#/jobs?repo=try-comm-
> central&revision=2413b34b6ab63a0de66a3ae1dd69b31a7e54606d
> ?

Thank you for your reply. I have downloaded a build using your link and found that it still x86 (win32).
Maybe I just misunderstood how to download x64...could you help me with that. Thank you.

Comment 94

[gene smith]

No, there is only win32 at that link. I will push the patch and build as win64. Unfortunately, I don't know a way to make a "try" build with Russian localization. I will provide a new link ASAP in the following comment.

Comment 95

[gene smith]

Mikhail,
Here's the 64bit link. I just started the build and it might take up to an hour to complete.

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=ade3a745a46361219677e6318c859e9916edbe37

You should soon see 100% and "complete" and the "B" will say successful or not. Then, if successful, click the B to see the link below, like you did before, for obtaining the installation file.

Comment 96

[Jorg K (CEST = GMT+2)]

https://queue.taskcluster.net/v1/task/fPakpQUCQQiV5ZmBIx1VaA/runs/0/artifacts/public/build/install/sea/target.installer.exe

Comment 97

[Mikhail]

Hello!

I would like to inform that all my team members had successfully updated ThunderBird to the version you provided above. Seems like there are no problems with tags. So, the issue is finally solved, thank you!

I am looking forward to seeing these changes in the release version of ThunderBird.

PS
TB at the computer of one of my team members has a crash 1 or 2 times a day (the version you provided is used). I do not why. Other team members do not have such problems.

Does Thunderbird create any dumps during the crash?

Comment 98

[Mikhail]

Attachment: crash_report.zip

5 minutes ago it crashed again. I attached the crash report.

Comment 99

[Jorg K (CEST = GMT+2)]

Gene, can we move this forward, please.

Comment 100

[gene smith]

(In reply to Mikhail from comment #98)
> Created attachment 9029266 [details]
> crash_report.zip
> 
> 5 minutes ago it crashed again. I attached the crash report.

I don't know how to interpret the crash report. Is it caused by the patch for this bug or something else (don't see the string "imap" in the text)? Maybe Wayne would look at it and tell (setting NI)?
 
(In reply to Jorg K (GMT+1) from comment #99)
> Gene, can we move this forward, please.

OK, I will clean up the patch and submit it.

Comment 101

[Mikhail]

I guess this crash does not neccessary connected with tag functionality. It might be a bug in general functionality of TB >=60 ver. (Users says that there were no crashes before update, he used ver 52 before). This problem occured only in 1 computer, other TB seem to work fine. So, I guess current bug report is not a correct place to discuss this crash. I wrote it just for you to be informed.

Comment 102

[gene smith]

Attachment: 583677-ensure-custom-tags-are-visible.patch(rev0)

Here's the patch that reporter Mikhail has been running from a try build that solves the regression problem with custom keywords (tags) that he has been living with for years. This is an alternative to the simpler patch (attachment 9013514 [details] [diff] [review]) that fixes the problem using a new pref.

Comment 103

[Jorg K (CEST = GMT+2)]

Comment on attachment 9030062 [details] [diff] [review]
583677-ensure-custom-tags-are-visible.patch(rev0)

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

Looks OK. A question and a few nits.

::: mailnews/imap/src/nsImapMailFolder.cpp
@@ +4835,5 @@
> +    uint32_t i = 0;
> +    nsAutoCString mozLogDefinedKWs;
> +    if (MOZ_LOG_TEST(IMAP_KW, mozilla::LogLevel::Debug))
> +      mozLogDefinedKWs.AppendLiteral("Defined keywords = |");
> +    do {

This can be done is a for loop.
for (i = 0; i < 100; i++).

What's the story with 100?

@@ +4866,5 @@
> +    // Combine local and rcvd keyword arrays into a single string
> +    // so it can be passed to SetStringProperty(). If element of
> +    // local already in rcvd, avoid duplicates in combined string.
> +    nsAutoCString combinedKeywords;
> +    for (i=0; i<localKeywordArray.Length(); i++)

Nit: Spaces around = and <

@@ +4874,5 @@
> +        combinedKeywords.Append(localKeywordArray[i]);
> +        combinedKeywords.Append(' ');
> +      }
> +    }
> +    for (i=0; i<rcvdKeywordArray.Length(); i++)

and here.

@@ +4881,5 @@
> +      combinedKeywords.Append(' ');
> +    }
> +    return dbHdr->SetStringProperty("keywords", combinedKeywords.get());
> +  }
> +  else

No else after return.

::: mailnews/imap/src/nsImapServerResponseParser.cpp
@@ +1882,5 @@
> +      // But only do this for FLAGS response, not PERMANENTFLAGS response
> +      // and if '\*' has not appeared in a PERMANENTFLAGS response.
> +      if (storeUserFlags && *fNextToken != '\r')
> +      {
> +        if ( *(fNextToken + strlen(fNextToken) - 1) != ')')

Nit: No space after (

Comment 104

[gene smith]

(In reply to Jorg K (GMT+1) from comment #103)
> Comment on attachment 9030062 [details] [diff] [review]
> 583677-ensure-custom-tags-are-visible.patch(rev0)
> 
> Review of attachment 9030062 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks OK. A question and a few nits.
> 
> ::: mailnews/imap/src/nsImapMailFolder.cpp
> @@ +4835,5 @@
> > +    uint32_t i = 0;
> > +    nsAutoCString mozLogDefinedKWs;
> > +    if (MOZ_LOG_TEST(IMAP_KW, mozilla::LogLevel::Debug))
> > +      mozLogDefinedKWs.AppendLiteral("Defined keywords = |");
> > +    do {
> 
> This can be done is a for loop.
> for (i = 0; i < 100; i++).
> 
> What's the story with 100?

Yes, changed to for loop. The 100 is just to limit the number keywords processed in the response to a reasonable number in case the server has stored up a huge number of them. I have changed this to a "const uint32_t" instead of a literal constant. I talk about my concern about the number of tags in comment 85 and comment 84.

Is this OK:
    :
    const uint32_t MaxKeywords = 100; // Limit the number of keywords processed.
    for (i = 0; i < MaxKeywords; i++)
    {
      flagState->GetOtherKeywords(i, definedKeyword);
      if (definedKeyword.IsEmpty())
      {
    :

I guess alternatives are to process an unlimited number or make a pref for the limit.

> 
> @@ +4866,5 @@
> > +    // Combine local and rcvd keyword arrays into a single string
> > +    // so it can be passed to SetStringProperty(). If element of
> > +    // local already in rcvd, avoid duplicates in combined string.
> > +    nsAutoCString combinedKeywords;
> > +    for (i=0; i<localKeywordArray.Length(); i++)
> 
> Nit: Spaces around = and <
> 
> @@ +4874,5 @@
> > +        combinedKeywords.Append(localKeywordArray[i]);
> > +        combinedKeywords.Append(' ');
> > +      }
> > +    }
> > +    for (i=0; i<rcvdKeywordArray.Length(); i++)
> 
> and here.

Done

> 
> @@ +4881,5 @@
> > +      combinedKeywords.Append(' ');
> > +    }
> > +    return dbHdr->SetStringProperty("keywords", combinedKeywords.get());
> > +  }
> > +  else
> 
> No else after return.

I don't know what you mean by this. Does it mean "you need an else after the return" or "don't put an else after the return".
Regardless, the structure currently is like this:

nsresult fff()
{
  :
  bool yy = ...;
  if (something)
  {
    do this;
    do that;
    return fn(x);
  }
  else
    return yy ? fn(y) : NS_OK;
}

I will update the patch to this:

nsresult fff()
{
  :
  bool yy = ...;
  if (something)
  {
    do this;
    do that;
    rv = fn(x);
  }
  else
  {
     rv = yy ? fn(y) : NS_OK;
  }
  return rv;
}

> 
> ::: mailnews/imap/src/nsImapServerResponseParser.cpp
> @@ +1882,5 @@
> > +      // But only do this for FLAGS response, not PERMANENTFLAGS response
> > +      // and if '\*' has not appeared in a PERMANENTFLAGS response.
> > +      if (storeUserFlags && *fNextToken != '\r')
> > +      {
> > +        if ( *(fNextToken + strlen(fNextToken) - 1) != ')')
> 
> Nit: No space after (

Done

I will next attach an updated patch if you approve of these answers.

Comment 105

[Magnus Melin [:mkmelin]]

(In reply to gene smith from comment #104)
> Yes, changed to for loop. The 100 is just to limit the number keywords

Probably doesn't need to limit this.
> > > +    return dbHdr->SetStringProperty("keywords", combinedKeywords.get());
> > > +  }
> > > +  else
> > 
> > No else after return.
> 
> I don't know what you mean by this. Does it mean "you need an else after the

It means you should not have an "if" when the if-clause returns. You just go on with the code

if (foo)
  return;
bar(); // good

instead of

if (foo)
  return;
else
  bar(); // bad

---

Having to deal with the else adds mental overhead, and also adds unnecessary indention.

Comment 106

[gene smith]

(In reply to Magnus Melin [:mkmelin] from comment #105)
> (In reply to gene smith from comment #104)
> > Yes, changed to for loop. The 100 is just to limit the number keywords
> 
> Probably doesn't need to limit this.

OK

> It means you should not have an "if" when the if-clause returns. You just go
> on with the code
> 
> if (foo)
>   return;
> bar(); // good
> 
> instead of
> 
> if (foo)
>   return;
> else
>   bar(); // bad
> 
> ---
> 
> Having to deal with the else adds mental overhead, and also adds unnecessary
> indention.

OK I'll change it like you and Jorg prefer. But I kind of like the else. I also prefer just one return point in a function which is specified in functional safety project coding guidelines that I used before. But that ship has sailed with the frequent use of NS_ENSURE_SUCCESS(rv,rv) macro throughout the tb code. So I will use this form:

nsresult fff()
{
  nsresult rv = mozFn();
  NS_ENSURE_SUCCESS(rv,rv);
  :
  bool yy = ...;
  if (something)
  {
    do this;
    do that;
    return fn(x);
  }
  return yy ? fn(y) : NS_OK;
}

I didn't know this is actually a part of the mozilla coding standard. However, not everyone agrees with it. Re: https://blog.mozilla.org/nnethercote/2009/08/31/no-else-after-return-considered-harmful/

Comment 107

[gene smith]

Attachment: 583677-ensure-custom-tags-are-visible.patch(rev1)

Updated based Jorg's and Magnus's comments.

Comment 108

[Magnus Melin [:mkmelin]]

Comment on attachment 9030614 [details] [diff] [review]
583677-ensure-custom-tags-are-visible.patch(rev1)

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

Looks ok to me, with some small adjustments

::: mailnews/imap/public/nsIImapFlagAndUidState.idl
@@ +69,5 @@
>     * @param aCustomAttributeValue   Value of the attribute,
>     */
>    ACString getCustomAttribute(in unsigned long aUid,
>                                in ACString aCustomAttributeName);
> +  void setOtherKeywords(in unsigned short index, in string otherKeyword);

please add documentation, and make this.
would be nicer with AUTF8String

::: mailnews/imap/src/nsImapFlagAndUidState.cpp
@@ +110,5 @@
>  
> +NS_IMETHODIMP
> +nsImapFlagAndUidState::SetOtherKeywords(uint16_t index, const char* otherKeyword)
> +{
> +  if (!index)

I would do index == 0
for clarity

::: mailnews/imap/src/nsImapMailFolder.cpp
@@ +4831,5 @@
> +    ParseString(localKeywords, ' ', localKeywordArray);
> +    ParseString(keywords, ' ', rcvdKeywordArray);
> +
> +    nsAutoCString definedKeyword;
> +    uint32_t i;

please declare the i's where used

for (uint32_t i; i = 0; i++)

@@ +4855,5 @@
> +      }
> +      bool inLocal = false;
> +      bool inRcvd = false;
> +      if (localKeywordArray.Contains(definedKeyword))
> +        inLocal = true;

why not directly

bool inLocal = localKeywordArray.Contains(definedKeyword);

The same for inRcvd

Comment 109

[Jorg K (CEST = GMT+2)]

Comment on attachment 9030614 [details] [diff] [review]
583677-ensure-custom-tags-are-visible.patch(rev1)

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

r+ with Magnus' and my comments considered. I think we only need one declaration of 'i' as I indicated.

::: mailnews/imap/public/nsIImapFlagAndUidState.idl
@@ +70,5 @@
>     */
>    ACString getCustomAttribute(in unsigned long aUid,
>                                in ACString aCustomAttributeName);
> +  void setOtherKeywords(in unsigned short index, in string otherKeyword);
> +  ACString getOtherKeywords(in unsigned short index);

Yes, both should be AUTF8String.

::: mailnews/imap/src/nsImapMailFolder.cpp
@@ +4831,5 @@
> +    ParseString(localKeywords, ' ', localKeywordArray);
> +    ParseString(keywords, ' ', rcvdKeywordArray);
> +
> +    nsAutoCString definedKeyword;
> +    uint32_t i;

My preference you be to do
uint32_t i = 0;
while (true)
  flagState->GetOtherKeywords(i, definedKeyword);
  i++;
}

i is also used as a look variable below, so no need to declare it three times.

::: mailnews/imap/src/nsImapServerResponseParser.cpp
@@ +1884,5 @@
> +      if (storeUserFlags && *fNextToken != '\r')
> +      {
> +        if (*(fNextToken + strlen(fNextToken) - 1) != ')')
> +        {
> +          // Token doesn't ends in ')' so save it as is.

typo: doesn't end.

@@ +1885,5 @@
> +      {
> +        if (*(fNextToken + strlen(fNextToken) - 1) != ')')
> +        {
> +          // Token doesn't ends in ')' so save it as is.
> +          fFlagState->SetOtherKeywords(numOtherKeywords++, fNextToken);

You can use nsDependentCString().

@@ +1892,5 @@
> +        {
> +          // Token ends in ')' so end of list. Change ')' to null and save.
> +          char* nonConstStr = PL_strdup(fNextToken);
> +          *(nonConstStr + strlen(fNextToken) - 1) = 0;
> +          fFlagState->SetOtherKeywords(numOtherKeywords++, nonConstStr);

You can use:
nsDependentCSubstring(fNextToken, strlen(fNextToken) - 1).

Comment 110

[Jorg K (CEST = GMT+2)]

(In reply to Jorg K (GMT+1) from comment #109)
> i is also used as a look variable below, so no need to declare it three
> times.
 ... loop variable ...

Comment 111

[gene smith]

> Yes, both should be AUTF8String.

Just in the .idl file or everywhere? Sorry, not familiar with this type. If I just change the idl I get lots of compile errors.

Will this work for the nsDependentCSSubstring?


#if 0     // my Original code
          // Token ends in ')' so end of list. Change ')' to null and save.
          char* nonConstStr = PL_strdup(fNextToken);
          *(nonConstStr + strlen(fNextToken) - 1) = 0;
          fFlagState->SetOtherKeywords(numOtherKeywords++, nonConstStr);
          PR_Free(nonConstStr);
#else   
          // This instead? Does this just extract the substring (I want to leave off the ')').
          nsDependentCSubstring tokenWithoutParen(fNextToken, strlen(fNextToken) - 1);
          fFlagState->SetOtherKeywords(numOtherKeywords++, tokenWithoutParen.get());
#endif

Also, don't know how/where to use the nsDependentCString() either that you said I should use.  Sorry, again, I've never seen this before and documentation is pretty bad.

Rest of the comments I understood.

Comment 112

[Magnus Melin [:mkmelin]]

(In reply to gene smith from comment #111)
> > Yes, both should be AUTF8String.
> 
> Just in the .idl file or everywhere? Sorry, not familiar with this type. If
> I just change the idl I get lots of compile errors.

In the idl (for the methods you touch), and then the cpp needs to adapt the type to take in an nsACString instead of "const char*".
ACString is ASCII only, and AUTF8String UTF-8 so general purpose. See https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Guide/Internal_strings

Comment 113

[Jorg K (CEST = GMT+2)]

Attachment: 583677-ensure-custom-tags-are-visible.patch (v2)

Like this. I've also made the other changes and rebased the patch since it didn't apply, looks like it pre-dated the CONDSTORE stuff.

Please refresh locally and check that all is still working. And let's not make the mistake of not running it through a try before landing:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=4e5fadd5b6cf0af7fa3a31dc4c3e762da78930ab

Comment 114

[Jorg K (CEST = GMT+2)]

FYI, interdiff spits the dummy on this one and doesn't show all the changes I made :-(
https://bugzilla.mozilla.org/attachment.cgi?oldid=9030614&action=interdiff&newid=9031344&headers=1

Comment 115

[Magnus Melin [:mkmelin]]

(In reply to Jorg K (GMT+1) from comment #109)
> i is also used as a loop variable below, so no need to declare it three
> times.

Well, only, isn't it? That's why it's better declared where used. I always assume it's going to be used/needed somewhere outside the loop if it's declared outside.

Comment 116

[Jorg K (CEST = GMT+2)]

That's really a matter of taste, but my while construct needs it to be declared before the loop. If you really need the value outside the loop, you should use less vanilla names, like:
  indexFound = i;
  break;

Comment 117

[Jorg K (CEST = GMT+2)]

TEST-UNEXPECTED-FAIL | comm/mailnews/imap/test/unit/test_imapFilterActions.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/imap/test/unit/test_imapFilterActions.js | MarkReadBody - [MarkReadBody : 590] 0 == 1
TEST-UNEXPECTED-FAIL | comm/mailnews/imap/test/unit/test_imapFilterActionsPostplugin.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/imap/test/unit/test_imapFilterActionsPostplugin.js | KillThread - [KillThread : 418] 1 == 2

:-(

Comment 118

[gene smith]

Sorry for the problems. The patch was based on recent esr60 update where I did the trys and not latest comm-central. I suspected it might conflict but figured I would fix it after all the changes were approved.

> TEST-UNEXPECTED-FAIL | comm/mailnews/imap/test/unit/test_imapFilterActions.js | xpcshell return code: 0

I guess you want me to try to debug this? 

> Like this....

Thanks for fixing it.

Comment 119

[Jorg K (CEST = GMT+2)]

(In reply to gene smith from comment #118)
> > TEST-UNEXPECTED-FAIL | comm/mailnews/imap/test/unit/test_imapFilterActions.js | xpcshell return code: 0
> I guess you want me to try to debug this? 
I'm afraid so.

Comment 120

[gene smith]

Fixed the test failure fairly easily due to a simple coding error on my part. However, with attachment 9031344 [details] [diff] [review] and my coding fix it fails for another reason. However, with my original attachment 9030614 [details] [diff] [review] with my coding fix it passes the the problem tests. Both test fail like this:

Unexpected Results
------------------
comm/mailnews/imap/test/unit/test_imapFilterActionsPostplugin.js
  FAIL comm/mailnews/imap/test/unit/test_imapFilterActionsPostplugin.js - xpcshell return code: 0
  FAIL AddTag - [AddTag : 146] "TheTag" == "thetag"
/home/gene/mozilla2/obj-x86_64-pc-linux-gnu/_tests/xpcshell/comm/mailnews/imap/test/unit/test_imapFilterActionsPostplugin.js:AddTag:146
  FAIL comm/mailnews/imap/test/unit/test_imapFilterActionsPostplugin.js - xpcshell return code: 0
  FAIL AddTag - [AddTag : 146] "TheTag" == "thetag"
/home/gene/mozilla2/obj-x86_64-pc-linux-gnu/_tests/xpcshell/comm/mailnews/imap/test/unit/test_imapFilterActionsPostplugin.js:AddTag:146

Seems to be caused by the string handling changes. Will look closer tomorrow.

Comment 121

[Jorg K (CEST = GMT+2)]

Well, we force stuff to lower case now, no?
https://hg.mozilla.org/try-comm-central/rev/b8850e2e4330b31d953f2a0833f9a701798aae3f#l2.18
Maybe just do the comparison/search in lower case but add the tag in its original case.

Comment 122

[gene smith]

Attachment: 583677-ensure-custom-tags-are-visible.patch(v3)

Sorry, false alarm. The failure was caused by a seemingly innocuous "improvement" I made. Patch v3 now just one line different that previous patch v2 to fix the original test errors and the tests pass.

I also retested the patch and it works well. For the record I have two dovecot imap accounts for two users (gene and marianne). Account gene has full access to all folder including a shared folder, called 555, that marianne account only has read access to. Marianne has full access to all other (not shared) folders.

555 is shared using symlinks in dovecot's Maildir directory, not using imap shared/public folders namespaces. I think this is similar to how the reporter Mikhail is setup for sharing.

I then run two tb instances with separate profiles. One accesses gene's account and the other marianne's account. On gene account I can set custom tags or built-in labels (e.g., Important) for an email in folder 555 and see them on gene account and I can see them on the same email in folder 555 of marianne account provided the tags strings are defined there. If not defined at marianne, you still don't see them. Custom tags or labels set on emails of folder 555 at marianne are not seen at gene even if defined at gene (however, they are preserved at marianne on restart).

Just to be clear, the problem this fixes is that custom tags (a.k.a, keywords) set on emails at gene account were not seen at marianne account for shared folder 555 since marianne only has read access to 555.

Note: I did notice one problem. When I defined a new tag, regardless of the color chosen or the folder, the tag appears only in black on white in the displayed email. Possibly a known bug?

Comment 123

[Jorg K (CEST = GMT+2)]

Comment on attachment 9032263 [details] [diff] [review]
583677-ensure-custom-tags-are-visible.patch(v3)

Thanks, one line changed.

Comment 124

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/2d9043361668
Fix so custom tags (keywords) are visble to all users. r=jorgk

Comment 125

[gene smith]

(In reply to gene smith from comment #122)
> 
> Note: I did notice one problem. When I defined a new tag, regardless of the
> color chosen or the folder, the tag appears only in black on white in the
> displayed email. Possibly a known bug?

Yes, a well know recent bug with a couple of dups: Bug 1497041