<a class="header-button" href="https://bugzilla.mozilla.org/home" title="Go to home page"> Bugzilla

Reporter

Comment 1

•

5 years ago

I'm experiencing this issue also on a brand new installation of TB 68.2.2 (64 bits).
I'm not experiencing this issue on Google account

Comment 2

•

5 years ago

I assume you're using IMAP.

Assignee

Comment 3

•

5 years ago

Reporter Manuel, assuming the account that doesn't work is imap, an IMAP:5 log would probably be helpful. So follow the instructions here, https://wiki.mozilla.org/MailNews:Logging, and attach the log. Please indicate exactly what you did while recording the log.
Thanks.

Reporter

Comment 4

•

5 years ago

Attached file IMAP debug — Details

IMAP debug collected while reproducing issue.
At 2019-11-15 08:44:00.964000 I pressed 2 key (tagging a message into INBOX folder), after that I changed folder (Draft folder) and I gone back to INBOX folder

Assignee

Comment 5

•

5 years ago

Thanks for the log. It looks like the PERMANENTFLAGS response from Communigate Pro imap server doesn't support saving client defined tags (keywords) on the server. However, tb is supposed to still set the tag for the message in its database so it should still be preserved. (The tag will not be visible if the account accessed by another email client since it is not saved on the server.)

I will need to test this locally and cause a server to not store tags.

Did this work for you on previous versions of tb using the same account/imap server?

The reason it works for google/gmail is because their server does support storage of client defined tags, i.e., PERMANENTFLAGS contains "\*".

Assignee

Comment 6

•

5 years ago

•

Edited

I now know the flags are being set by Communigate server. See comment 8 below.
I see that your PERMANENTFLAGS responses contain $Label1, $Label2 and $Label3. Do you know where these come from or who is setting them. I guess it is possible they are built into the server response but it kind of looks like they may be being set by another client that is allowed custom keyword permission. Is it possible that there is shared or public folders in you setup? I do see public and shared paths in the namespace response from your Communigate imap server. If so, the shared users may be allowed to set custom keyword (tags) which will add them to the PERMANENTFLAGS for all users.

I have tried to duplicate your problem by disabling custom keywords on my server. I see them stored into the tb database and they survive a a folder selection change and a tb restart. Don't know why your tag setting don't stick. But I'll keep looking.

Can you tell me exactly which tag you are trying to set? It appears you are trying to set $Label2 which is the "Work" tag (in English).

Assignee

Comment 7

•

5 years ago

•

Edited

No need to record a log as requested below!
The commuigate imap server is saying that it is ok to store keywords $Label1, $Label2 and $Label3 but tb wont attempt to send the keyword to the server unless "\*" is in PERMANENTFLAG. This is very similar to the problem identified in bug 1260059 comment 13 where for yahoo $Junk and $NotJunk were in PERMANENTFLAG but tb wouldn't set send them to the server also because of lack of "\*" in PERMANENTFLAGS. But in the yahoo bug, $Junk and $NotJunk are legitimate flags put in PERMANENTFLAGS by the server. I question why the $Label* flags are in the communigate PERMANENTFLAGS.

Manuel, if you can't explain how $Label*s got in PERMANENTFLAGS (e.g., a folder shared with another client caused it as suggested above), please do this for me: Record a new IMAP:5 log and this time create a new folder. Then copy a message to the new folder and then select (click on) the new folder and shutdown tb. In the generated log near the end, look for where the "imap select" of the folder occurs. The response to the imap select will contain the FLAGS and the PERMANENTFLAGS response similar to this (where the new folder is called NewFolder):

2019-11-15 08:44:06.901000 UTC - [(null) 7080: Unnamed thread 0000018B5B0E1000]: I/IMAP 0000018B59D59000:mail.mydomain.it:A:SendData: 4 select "NewFolder"
2019-11-15 08:44:06.901000 UTC - [(null) 7080: Unnamed thread 0000018B5B0E1000]: D/IMAP ReadNextLine [stream=0000018B58812180 nb=116 needmore=0]
2019-11-15 08:44:06.901000 UTC - [(null) 7080: Unnamed thread 0000018B5B0E1000]: I/IMAP 0000018B59D59000:mail.mydomain.it:A:CreateNewLineFromSocket: * FLAGS (\Answered \Flagged \Deleted \Seen \Draft $MDNSent $Hidden $Media $Forwarded Junk $Label1 $Label2 $Label3)
2019-11-15 08:44:06.901000 UTC - [(null) 7080: Unnamed thread 0000018B5B0E1000]: D/IMAP ReadNextLine [stream=0000018B58812180 nb=138 needmore=0]
2019-11-15 08:44:06.901000 UTC - [(null) 7080: Unnamed thread 0000018B5B0E1000]: I/IMAP 0000018B59D59000:mail.mydomain.it:A:CreateNewLineFromSocket: * OK [PERMANENTFLAGS (\Answered \Flagged \Deleted \Seen \Draft $MDNSent $Hidden $Media $Forwarded Junk $Label1 $Label2 $Label3)] limited

Paste the similar lines from the log in a comment or attach them as a small text file. I don't need the full log. I am curious as to the response for a newly create folder that has not possibly been changed yet.

Comment 8

•

5 years ago

I now see by looking at Communigate Pro release notes that starting with version 5.1 they now include Junk, $label1, $label2 and $label3 in the PERMANENTFLAGS. https://www.communigate.com/CommuniGatePro/History51.html
So no need to create a folder and record a log that I requested in comment 7. But still curious if you have shared or public mailboxes in use with your imap account on Communigate server.

(In reply to Manuel Calavera from comment #1)

I'm experiencing this issue also on a brand new installation of TB 68.2.2 (64 bits).

Are you saying that you saw this or didn't see this on earlier TB versions too? Please explain.

I still can't duplicate the problem when simulating the Communigate server imap characteristics. I might be able to tell more with a temporary test account using your server. Would that be possible? You can send credentials via my bugzilla profile email address. Thanks.

Assignee

Comment 9

•

5 years ago

Sorry one more request for a log, but only necessary if a test account is not possible. As described here, bug 583677 comment 85, please add IMAP_KW:5 to the MOZ_LOG env var and redo the log again like I requested in comment 3 above and you produced in comment 4.

MOZ_LOG=IMAP:5,IMAP_KW:5

Again please try to set the tag and cause it to be lost like you did before. Thanks again!

Assignee

Comment 10

•

5 years ago

Sorry again... I am now seeing new tags that I set not stick. So at this time I don't need the test account or a new log with IMAP_KW:5 defined in env var.
It seems a bit random, sometimes they stick and sometimes not. Anyhow, I will continue to debug this locally. But any additional info you might have on this is appreciated.

Reporter

Comment 11

•

5 years ago

Some clarifications:
I start experiencing this issue from TB 68.1.1 (I've upgraded from 60.9.1 to 68.1.1).
In 60.9.1 tags work perfectly.

I've not added any custom tag, under Options\Display\Tags I've these tags: Important, Work, Personal, To Do, Later.

Assignee

Comment 12

•

5 years ago

I think I found the problem. If you can set the "To Do" and "Later" tags and they stick, that would verify it. Only the first 3 should have the problem you describe. If you could test setting "To Do" and "Later" I would appreciate it.

Assignee

Comment 13

•

5 years ago

Mikhail,
The fix for the tag/keyword bug you identified in Bug 583677 was released with tb v68. It is probably OK for your purposes but the reporter of this bug, Manuel, has identified a bug in the patch that causes a problem. I am curious if you see the same problem that Manuel sees.
If you could check and see if when you set one of TB's built-in tags that it stays set (i.e., sticks). Manuel observes that when one of the tags Important, Work, Personal, To Do or Later are set and if he switches to another folder and comes back or restarts TB that the tag becomes not set. This is on an IMAP server similar to yours where "custom" tags cannot be set. I don't see this problem on most IMAP servers which do allow custom tags to be set. So if you could check this on your setup I would appreciate it.
Thanks,
-gene

Flags: needinfo?(miketm)

Regressed by: 583677

Reporter

Comment 14

•

5 years ago

(In reply to gene smith from comment #12)

I think I found the problem. If you can set the "To Do" and "Later" tags and they stick, that would verify it. Only the first 3 should have the problem you describe. If you could test setting "To Do" and "Later" I would appreciate it.

I confirm that "To Do" and "Later" tags remain sticked on.

Assignee

Comment 15

•

5 years ago

A quick fix is to just remove this line from the code:
https://searchfox.org/comm-central/rev/8098988058c0f059ab3164f4f459c9ae07b0b93b/mailnews/imap/src/nsImapMailFolder.cpp#4507
This should also still fix the problem identified my Mikhail: Bug 583677

However, reviewing the code further, I have concluded that the line above this is also not necessary. Therefore putting and getting "definedKeywords" (from the the content of the imap select FLAGS response that is intended to contain user defined keywords) isn't needed at all. So I removed the code that I added in Bug 583677 that gets and puts these "tags" into an array of strings.

I have produced a "try" build by patching the lastest ESR 68 release that can be obtained here:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=028736cec72d2c5fef71b96d035c4579cec663d5

Manuel, if you could test the "try" build and see if it fixes this bug it would be appreciated. Just click on the green "B" next to the appropriate architecture, e.g., "Windows 2012 x64 opt" for 64-bit window or "Windows 2012 opt" for 32-bit windows. When you click the "B" you should then see a selection for "Job Details" in a black bar about half way down the screen. Clicking on "Job Details" you should see a long list of items, but the item you probably want is called "artifact uploaded: target.installer.exe". So you can download the installer and test tb using this link.

Mikhail, since this also affects you, if you could test this too it would be most helpful. I don't have a way to exactly duplicate your many user defined tags and I want to make sure your shared mailboxes and tags all still work with the changes I am proposing here. Thanks in advance.

One more thing: If either of you see a problem, please produce a log with IMAP:5 and IMAP_KW:5 included in MOZ_LOG, e..g,
MOZ_LOG=IMAP:5,IMAP_KW:5

Updated

•

5 years ago

Keywords: regression

Reporter

Comment 16

•

5 years ago

Great Job Gene! Now tags work again for me.

Thanks

Assignee

Comment 17

•

5 years ago

Thanks reporter Manuel for testing the "try" build. Just to be clear, this is not an official release but you are welcome to keep using it. I still want to hear from Mikhail and hopefully find that his test of the "try" build is OK too. Also, I will do some more test myself and submit the patch for final approval and incorporation into the official release.

Assignee

Comment 18

•

5 years ago

I've done some more tests and there is still an issue.

The proposed fix in the "try" build is OK for setups like Manuel's (reporter of this bug) where the folder is not shared and new keywords can't be stored on the server. Tags will be stored in the local TB database and not on the server and can be set and remain in place or be removed. The problem is when the folder is shared and keywords can be stored for one user and not by others sharing the folder. This is Mikhail's setup (reporter of bug 583677).

Now I realized what this line does:
https://searchfox.org/comm-central/rev/8098988058c0f059ab3164f4f459c9ae07b0b93b/mailnews/imap/src/nsImapMailFolder.cpp#4507
When the the folder is shared and a user that can set tags removes a tag, it causes the tag to be removed for users sharing the folder that can't set tags. (I should have put a comment in the code explaining this.) This line causes the problem identified by reporter Manuel and removing the line causes the described issue.

So with the "try" code, when a privileged user (user-a) sets a tag on a message it will be seen in the shared folder by users who can't store tags on the server (e.g., user-b). If user-b removes the tag, it will come right back since user-a has not yet removed it from the message in the shared folder on the server. This seems OK. But if user-a does remove the tag from the message on the shared folder, user-b will still see it. For user-b to not see the tag, it will have to removed first by user-a and then later by user-b. Not sure if this is a big problem.

Mikhail, I was able to test a setup similar to yours using the setup described here: bug 583677 comment 122. So you don't need to test the "try" build, but you are welcome to if you want.

Flags: needinfo?(miketm)

Assignee

Comment 19

•

5 years ago

•

Edited

Attached patch 1596371-review-changes.patch(v0) (obsolete) — Details — Splinter Review

Magnus, bugzilla wouldn't let me set Jorg as the reviewer like I normally do due to PTO.

Here's a patch to fix the regression. I've tested it and it works well. It also fixes this issue described in the previous comment 18:

So with the "try" code, when a privileged user (user-a) sets a tag on a message it will be seen in the shared folder by users who can't store tags on the server (e.g., user-b). If user-b removes the tag, it will come right back since user-a has not yet removed it from the message in the shared folder on the server. This seems OK. But if user-a does remove the tag from the message on the shared folder, user-b will still see it. For user-b to not see the tag, it will have to be removed first by user-a and then later by user-b. Not sure if this is a big problem.

Now when user-a removes the tag, user-b will automatically no longer see the tag and not have to remove it manually.

Manuel and/or Mikhail: If you want a "try" build to test with this patch, let me know.
Edit: The feedback "minus" was unintentional.

Assignee: nobody → gds

Status: UNCONFIRMED → ASSIGNED

Ever confirmed: true

Attachment #9110765 - Flags: review?(mkmelin+mozilla)

Attachment #9110765 - Flags: feedback-

Updated

•

5 years ago

Attachment #9110765 - Flags: feedback-

Comment 20

•

5 years ago

Comment on attachment 9110765 [details] [diff] [review] 1596371-review-changes.patch(v0) Review of attachment 9110765 [details] [diff] [review]: ----------------------------------------------------------------- Looks good I think. Sure would be nice to have a test for this. Maybe build on test_imapFlagChange.js? ::: mailnews/imap/src/nsImapMailFolder.cpp @@ +4489,5 @@ > ParseString(localKeywords, ' ', localKeywordArray); > + ParseString(keywords, ' ', keywordArray); > + ParseString(prevKeywords, ' ', prevKeywordArray); > + > + uint32_t i; please declare this for each for loop instead like for(uint32_t i; i < .....) @@ +4491,5 @@ > + ParseString(prevKeywords, ' ', prevKeywordArray); > + > + uint32_t i; > + // If keyword not received now but was the last time, remove it from > + // the localKeywords. This mean the keyword was removed by another user means

Attachment #9110765 - Flags: review?(mkmelin+mozilla) → review+

Reporter

Comment 21

•

5 years ago

Is this fix present in TB 68.3.0?
I've installed it but this bug is still here :(

Comment 22

•

5 years ago

No it's not. The code change hasn't even been added to the code base. Maybe it will be included in 68.4 (if you're lucky).

Assignee

Comment 23

•

5 years ago

Attached patch 1596371-reviewed-changes(v1).patch (obsolete) — Details — Splinter Review

Updated with requested changes (except for the new test).

Attachment #9110765 - Attachment is obsolete: true

Attachment #9114413 - Flags: review?(mkmelin+mozilla)

Comment 24

•

5 years ago

Attached patch 1596371-reviewed-changes.patch (v1) - reformatted — Details — Splinter Review

Please keep in mind that you need to run clang-format after making changes to C++ files.

Attachment #9114413 - Attachment is obsolete: true

Attachment #9114413 - Flags: review?(mkmelin+mozilla)

Attachment #9114996 - Flags: review?(mkmelin+mozilla)

Comment 25

•

5 years ago

(In reply to gene smith from comment #19)

Created attachment 9110765 [details] [diff] [review]
1596371-review-changes.patch(v0)

Magnus, bugzilla wouldn't let me set Jorg as the reviewer like I normally do due to PTO.

Here's a patch to fix the regression. I've tested it and it works well. It also fixes this issue described in the previous comment 18:

So with the "try" code, when a privileged user (user-a) sets a tag on a message it will be seen in the shared folder by users who can't store tags on the server (e.g., user-b). If user-b removes the tag, it will come right back since user-a has not yet removed it from the message in the shared folder on the server. This seems OK. But if user-a does remove the tag from the message on the shared folder, user-b will still see it. For user-b to not see the tag, it will have to be removed first by user-a and then later by user-b. Not sure if this is a big problem.

Now when user-a removes the tag, user-b will automatically no longer see the tag and not have to remove it manually.

Manuel and/or Mikhail: If you want a "try" build to test with this patch, let me know.
Edit: The feedback "minus" was unintentional.

Hello, gene smith!
First of all, I would like to thank you for your work and I am really sorry for the late reply. My team continue using this "custom tags" setup. Unfortunately, we still have not switched to the TB 68, because of changes in Addon's (Webextensions instead XUL / XPCOM) and now we are redeveloping our Addons under "Webextensions".

So, we still have not tested TB 68 and I can not confirm if bug 583677 was resolved or not. I think it is a good idea to "try" the build. So, if you make a build for me I would appreciate it. Thank you!

Comment 26

•

5 years ago

Hello(In reply to gene smith from comment #19)

Manuel and/or Mikhail: If you want a "try" build to test with this patch, let me know.

Hello, gene smith!

Could you prepare for me "build" with this bugfix, we would like to try it. Thank You!

Assignee

Comment 27

•

5 years ago

Mikhail, thanks for reminding me of this. Sorry for the delay but I will make the try build later today.

Comment 28

•

5 years ago

(In reply to gene smith from comment #27)

Mikhail, thanks for reminding me of this. Sorry for the delay but I will make the try build later today.

That's great! I am looking forward to it. Thank you!

Assignee

Comment 29

•

5 years ago

Mikhail, Sorry again for the delay. The "try" versions (patch to tb 68) is building now. It should be ready soon. You can find it here:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&collapsedPushes=525525&revision=59c9fd9ef97331e440191a98b68a901ed8d1308d
It includes a win32 and win64 version.

Assignee

Comment 31

•

5 years ago

Made a linux try build for the reporter of (probably) duplicate bug 1600371:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&collapsedPushes=525525&revision=f400d8343b99cf7ef3be36d4a339e99271ff838b

Updated

•

5 years ago

Attachment #9114996 - Attachment is patch: true

Updated

•

5 years ago

Attachment #9114996 - Flags: review?(mkmelin+mozilla) → review+

Updated

•

5 years ago

Keywords: checkin-needed-tb

Updated

•

5 years ago

Target Milestone: --- → Thunderbird 73.0

Pulsebot

Comment 32

•

5 years ago

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/047a04a5198d
Fix regression introduced by bug 583677. Improve handling of tags on shared folders. r=mkmelin

Status: ASSIGNED → RESOLVED

Closed: 5 years ago

Keywords: checkin-needed-tb

Resolution: --- → FIXED

Comment 33

•

5 years ago

Comment on attachment 9114996 [details] [diff] [review] 1596371-reviewed-changes.patch (v1) - reformatted Too late for a beta here, but since it's a regressions, we need to have it in ESR at some stage.

Attachment #9114996 - Flags: approval-comm-esr68?

Geoff Lankow (:darktrojan)

Updated

•

5 years ago

Attachment #9114996 - Flags: approval-comm-esr68? → approval-comm-esr68+

Comment 34

•

5 years ago

68.4:
https://hg.mozilla.org/releases/comm-esr68/rev/b713c06a547123bc7b28d926839cf3ffcb9adba6

status-thunderbird_esr68: --- → fixed

Comment 35

•

5 years ago

Hello!
Thank you for the beta version. We have tested it and seems like we found incorrect behaviour, which is considered as a bug.

Initial conditions:

We have "user A" who does have permissions to set tags in the shared IMAP folder
We have "user B" who does NOT have permissions to set tags in the shared IMAP folder

Steps are:

"user A" set a custom tag on the mail in the shared IMAP folder, for example, the tag "OPENED"
At the "user B" 's ThunderBird:
2.1 This tagged mail successfully appears in the virtual folder (works as expected);
2.2 This tag successfully appears on the mail in the main window with mails (works as expected);
"user B" set its own tag in the SAME mail, for example, "CLOSED". Not this mail has 2 tags "OPENED", which was set in the shared IMAP folder and "CLOSED" which was set locally at the "user B" 's ThunderBird:
"user B" updates the shared IMAP folder (for example, by clicking at the different IMAP folder and then click back)
BUG:
EXPECTED BEHAVIOUR (as TB ver 60 works): custom tags at "user B" 's ThunderBird were restored and now there is only "OPENED" tag on this mail
ACTUAL BEHAVIOUR (as TB ver 68.4.1 works): custom tags at "user B" 's ThunderBird were NOT restored and there are 2 tags on the mail:
"OPENED" and "CLOSED"

Even If I try to FIX this IMAP folder at the user B's ThunderBird I still have 2 tags on this mail: "OPENED" and "CLOSED"

So, user B must not have the possibility to set his own tags on the mails in the shared IMAP folder. If he set his own tag, it must be restored (and removed) after the next IMAP folder update. It has been working like this for many years.

PS
It seems like it is not a critical issue, nevertheless, I would like this bug to be fixed in the future versions. Shall I register a separate ticket?
Thank you.

Comment 36

•

5 years ago

Not this mail has 2 tags "OPENED" - misspelt.

I meant:
"NoW this mail has 2 tags: "OPENED"...

Comment 37

•

5 years ago

Shall I register a separate ticket?

Yes, please, with the steps to reproduce the issue from comment #35.

Assignee

Comment 38

•

5 years ago

Hi Mikhail,
So it is a new "feature" I added that user B can now set his own personal and private tag and and it stays after folder update or tb restart? Not sure why this is bad other than it didn't work like this before?
Sorry, haven't looked at the code in over a month so don't remember exactly what and why I did this or how to fix it.

Assignee

Comment 39

•

5 years ago

(In reply to Mikhail from comment #35)

Even If I try to FIX this IMAP folder at the user B's ThunderBird I still have 2 tags on this mail: "OPENED" and "CLOSED"

Hi Mikhail, I'm looking at this again right now and realize I don't understand what you mean here. User B set the CLOSED tag, right? Are you unable to remove the CLOSED tag using tb now? Not sure what you mean by "FIX"?

Also I wrote this above:

So it is a new "feature" I added that user B can now set his own personal and private tag and and it stays after folder update or tb restart?

What I mean to say was:

So is it a new "feature" I added that user B can now set his own personal and private tag and and it stays after folder update or tb restart?

Anyhow, I didn't mean to add a new feature.

I will test this now with shared folders, but my dovecot setup is messed up since I did a hardware and OS upgrade :(. So it may take a while to get it back the way it was (lots of permission issues to sort out).