replied/forwarded icons disappear after folder repair, detach/delete ("Internally set message flag" icluding "READ flag by Mark Folder Read" is not written to X-Mozilla-Status: until "manually changeable message flag" is changed manually)

RESOLVED FIXED in Thunderbird 38.0

Status

defect
P1
major
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: sergei.ivn+bugzilla, Assigned: aceman)

Tracking

(Blocks 1 bug, {dataloss, regression, testcase})

Dependency tree / graph
Bug Flags:
in-testsuite +

Thunderbird Tracking Flags

(thunderbird_esr38 fixed)

Details

(Whiteboard: [regression:tb12][workaround: do comment 11 on all messages])

Attachments

(2 attachments, 3 obsolete attachments)

Reporter

Description

6 years ago
it's pop3 account
1. repair folder
2. result: no replied/forwarded icons in the mail list anymore

it seems like TB does not update x-mozilla-status (doesn't set MSG_FLAG_REPLIED) for replied emails since v12 (not sure about version) and replied/forwarded status stored only in msfs.

http://forums.mozillazine.org/viewtopic.php?f=39&t=2658059

related(?) info:
Mark Folder Read does not update x-mozilla-status as well (doesn't set MSG_FLAG_READ)
(In reply to Serg Iv from comment #0)
> it's pop3 account
> 1. repair folder
> 2. result: no replied/forwarded icons in the mail list anymore
> it seems like TB does not update x-mozilla-status (doesn't set
> MSG_FLAG_REPLIED) for replied emails since v12 (not sure about version) and
> replied/forwarded status stored only in msfs.

I also could observe "Replied flag is not written to X-Mozilla-Staus: header of local msFolder file after reply" in Tb 17.0.2.
After "Compact", Replied flag was physically written to X-Mozilla-Staus: header.
[ X-Mozilla-Status: header]
(1) Initial : 0001 (read)
(2) Reply   : 0001 (replied icon is normally shown at thread pane)
(3) Compact : 0007 (replied flag is written)

Confirming.

See following web page for other flags in X-Mozilla-Status: and X-Mozilla-Status-2:.
> http://www.eyrich-net.org/mozilla/X-Mozilla-Status.html?en

If since Tb 12, it may be a regression by big change for "pluggable message store" enhancement.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Component: Untriaged → Backend
Product: Thunderbird → MailNews Core
Reporter

Comment 2

6 years ago
Just tried compacting but X-Mozilla-Status still the same 0001. I deleted several emails beforehand to get "real" compacting.

I tested Tb11 vs Tb12. After reply in TB11 X-Mozilla-Status=0003, in Tb12 X-Mozilla-Status=0001

Can you please comment this: Mark Folder Read does not update x-mozilla-status as well (doesn't set MSG_FLAG_READ). Is it bug or by design?
Keywords: regression
(In reply to Serg Iv from comment #2)
> Just tried compacting but X-Mozilla-Status still the same 0001.
> I deleted several emails beforehand to get "real" compacting.

Which do you mean?
- You deleted some mails before Compact, but still 0001 even after Compact.
- Before delete some mails, still 0001 after Compact request,
  but after delete some mails, "Replied" flag was written to X-Mozilla-Status:.
Reporter

Comment 4

6 years ago
To be more specific

1. I sent two emails to my acc from another address (let's call them message A and B)
2. I replied to the message A.
3. I deleted the message B.
4. I compacted the Inbox folder.

X-Mozilla-Status for the message A is still 0001
(In reply to Serg Iv from comment #4)
> To be more specific
> 1. I sent two emails to my acc from another address (let's call them message
> A and B)
> 2. I replied to the message A.
> 3. I deleted the message B.
> 4. I compacted the Inbox folder.
> X-Mozilla-Status for the message A is still 0001

I also saw it, without marking "Starred" what I accidentally set before Compact in my previous test.
When I marked "Starred" or "Unread", all flags were written to X-Mozilla-Status: header.
In my new test, I replied to mail which has Re: in subject.
(A)  0<expungedbytes of Inbox  
     Initial : 0001 => Reply : 0001 => Compact : 0001
     => mark as Starred : 0017 (HAS_RE, MARKED, REPLIED, READ)
(B)  Initial : 0001 => Reply : 0001
     => mark as Unread  : 0012 (HAS_RE, REPLIED, not-READ==Unread)
So, REPLIED flag was set by "mark as Starred" in my previous test instead of by Compact, and HAS_RE is also an example which is not automatically written to X-Mozilla-Status:.
Sorry for my confusion.

Phenomenon looks:
  "Internally set flag" is not written to X-Mozilla-Status:
  until "Manually changeable flag" is changed,
  where "Internally set flag"      : REPLIED, HAS_RE, FORWARDED, ATTACHMENT etc.
        "Manually changeable flag" : READ, MARKED etc.

(In reply to Serg Iv from comment #0)
> related(?) info:
> Mark Folder Read does not update x-mozilla-status as well (doesn't set MSG_FLAG_READ)

I think it's following.
> If mail is already marked as READ, READ flag is not updated by "Mark Folder Read".
> And, Replied mail usually has READ flag already upon Reply.
Summary: replied/forwarded icons disappear after folder repair → replied/forwarded icons disappear after folder repair("Internally set message flag" is not written to X-Mozilla-Status: until "manually changeable message flag" is changed)
Reporter

Comment 6

6 years ago
> If mail is already marked as READ, READ flag is not updated by "Mark Folder Read".
> And, Replied mail usually has READ flag already upon Reply.

I meant if you instead of actual reading make "mark folder read" then X-Mozilla-Status will be 0000, not 0001. This issue is not related to replying, just another case of the same bug I think. X-Mozilla-Status is not updated when it should be.
To reproduce get some emails, instead of reading make "mark folder read", look in the Inbox file and you'll see that X-Mozilla-Status=0000. Side effect - after a folder repair procedure those marked email will be UNreaded. It's very confusing.
(In reply to Serg Iv from comment #6)
> To reproduce get some emails, instead of reading make "mark folder read",
> look in the Inbox file and you'll see that X-Mozilla-Status=0000.

I could see it too, by "chanhe all mails to Unread(xxx0) manually, then Mark Folder Read".
 X-Mozilla-Status: X-Mozilla-Status-2:   After Mark Folder Read
   0000              00000000              0000  00000000
   0000              00800000              0000  00800000
   0010              00800000              0010  00800000
"Mark Folder Read" seems "internal flag change like REPLIED" for current Tb.
Summary: replied/forwarded icons disappear after folder repair("Internally set message flag" is not written to X-Mozilla-Status: until "manually changeable message flag" is changed) → replied/forwarded icons disappear after folder repair("Internally set message flag" icluding "READ flag by Mark Folder Read" is not written to X-Mozilla-Status: until "manually changeable message flag" is changed manually)
Reporter

Comment 8

6 years ago
Today my TB did some automatic maintenance work and automatically rebuilt the msf file for my Inbox. No more replied & forwarded information for me.
WADA, can we change the bug importance to "major" because it seems like TB loses important information even without specific user actions?

To be honest I managed to create simple and stupid Python script that scans the Sent files, collects all 'In-Reply-To:' info and then changes 'X-Mozilla-Status' for the corresponding emails but it's just a workaround.
Assignee

Updated

6 years ago
Severity: normal → major
Keywords: dataloss
OS: Windows 7 → All
Hardware: x86 → All
Summary: replied/forwarded icons disappear after folder repair("Internally set message flag" icluding "READ flag by Mark Folder Read" is not written to X-Mozilla-Status: until "manually changeable message flag" is changed manually) → replied/forwarded icons disappear after folder repair, detach/delete ("Internally set message flag" icluding "READ flag by Mark Folder Read" is not written to X-Mozilla-Status: until "manually changeable message flag" is changed manually)
Duplicate of this bug: 913778

Comment 10

6 years ago
Hej everybody!

We have TB 25 now and still this bug persists! Does nobody exept for SergeIV see the major impact of dataloss in this Bug? - I didn't realize before today, that all my FORWARDED and REPLIED data since early 2013 has been lost! I do alot office-work with TB and it's crucial to know, which mails have been answered/forwarded, sometimes I need that knowledge half a year later, like this time.

So I really hope someone will start to get rid of this bug! IN THE MEANTIME I really need to get a workaround for this problem to stop losing these email connections, I think I've already lost quite enough of them... - so could anyone explain the workaround to me? Or at least the thing "about the manual changeable flag" - how could I make use of it as a workaround?

Many thanks to anyone
who'd like to help me
or who'd like to fix this bug!
Reporter

Comment 11

6 years ago
Here's workaround. To save the replied/forwarded status to the Inbox file you should "star"/"unstar" the incoming message after you replied to/forwarded it.

Comment 12

6 years ago
Thank you Serg Iv for your quick help!

Though it's uncomfortable and demands a lot of discipline not to forget about it, that's easier than I thought and quite effective. I doublechecked the effect by removing the msf-file and restarting TB. Works fine!

But to anyone else reading by: in my opinion this is still a major bug and the workaround is not a sufficient solution to it.
Assignee

Comment 13

6 years ago
I can also confirm this bug on TB17 (and also that adding+removing star makes the replied/forward flag to get propagated to mbox file). It is true that bug 402392 makes changes to writing out X-Mozilla-Status (it removes some functions).

David Bienvenu, are you able to help us here?
Flags: needinfo?(mozilla)

Comment 14

6 years ago
I just had the same problem in TB 24.1.1 after "repairing" my Inbox (Mac OS X 10.6.8).
I did some testing with older versions of TB and can confirm Serg Iv's observation that the regression must have happened between v11 and v12.
Duplicate of this bug: 983656
modified testcase of realraven 
1. set display setting to "as read" immediately on display; 
2. in a folder set all messages as unread
3. used [N] to step through a bunch of mails to set some as read, note the number of unread messages M 
4. Mark Folder Read => 0 unread
5. Repair Folder => M unread messages

Failure using these steps happens with TB12.0a1 20111225, but not with 20111124.  This squarely puts the bead on maildir bug 402392
2011-12-24 16:18:06 PST
checked in - http://hg.mozilla.org/comm-central/rev/097bc36f180d
Blocks: 402392
Flags: needinfo?(mozilla)
Keywords: testcase
other possible related/dup: bug 832759

pre-version 12 bugs which sound similar: bug 709790, bug 645650
Whiteboard: [regression:tb12]
Assignee

Comment 18

4 years ago
It bothers me that all my replied/forwarded messages have wrong status headers due to this bug. Let's fix it at last.
Assignee: nobody → acelists
Status: NEW → ASSIGNED
Priority: -- → P1
Assignee

Comment 19

4 years ago
Posted patch 840418.patch (obsolete) — Splinter Review
So this works for me. The bug was that the nsMsgLocalMailFolder was missing the necessary overriding methods and UI functions called e.g. folder->MarkAllMessagesRead() which thus called the nsIMsgFolder version (non-overriden) that just operated on the msg database, not on the msgStore.
IMAP had the functions implemented so it shouldn't be affected. So I modeled the nsMsgLocalMailFolder methods after IMAP. And made a nice test :) The code is probably useful for maildir too, but the test will probably not work. It relies on the fact that compaction adds the X-Mozilla-Status headers into the mbox file. And I don't know how that works in maildir.

Aryx, please push to try server.
Flags: needinfo?(archaeopteryx)
Attachment #8561044 - Flags: review?(rkent)
Attachment #8561044 - Flags: review?(mkmelin+mozilla)
Reporter

Comment 20

4 years ago
Will it work for feeds too? X-Mozilla-Status should be 0041 for them, i think.
Assignee

Comment 21

4 years ago
Thanks for watching the bug. Yes, I have now tested it and Feeds do work too (reply/forward/mark folder read). They should be also handled by the nsMsgLocalMailFolder code (like POP3/Local folders).
After reading them, they get status of 0041. I am not sure why they have 0000, instead of 0040 while unread (new), but that is probably a different bug.
Flags: in-testsuite+
Whiteboard: [regression:tb12] → [regression:tb12][workaround: do comment 11 on all messages]
Assignee

Comment 23

4 years ago
Posted patch 840418.patch v1.1 (obsolete) — Splinter Review
OK, this fixes the mozmill failures (like OS X menu bar at its breaking job again).
Attachment #8561044 - Attachment is obsolete: true
Attachment #8561044 - Flags: review?(rkent)
Attachment #8561044 - Flags: review?(mkmelin+mozilla)
Attachment #8561100 - Flags: review?(rkent)
Attachment #8561100 - Flags: review?(mkmelin+mozilla)
Comment on attachment 8561100 [details] [diff] [review]
840418.patch v1.1

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

Overall the backend stuff looks good. There are just a few minor issues that need cleaning up before this is done, hence the feedback+.

I don't do mozmill, so I did not review the test.

::: mailnews/base/util/nsMsgUtils.cpp
@@ +2055,5 @@
> +  NS_ENSURE_SUCCESS(rv, rv);
> +
> +  for (uint32_t kindex = 0; kindex < aNumKeys; kindex++) {
> +    nsCOMPtr<nsIMsgDBHdr> msgHdr;
> +    aDB->GetMsgHdrForKey(aMsgKeys[kindex], getter_AddRefs(msgHdr));

How do you handle missing keys? The existing method using nsTArray silently skips those. You are adding a nullptr to the array. It seems to me like silently skipping is safer.

::: mailnews/local/src/nsLocalMailFolder.cpp
@@ +1311,5 @@
> +
> +    // Setup a undo-state
> +    if (aMsgWindow)
> +      rv = AddMarkAllReadUndoAction(aMsgWindow, thoseMarked, numMarked);
> +    nsMemory::Free(thoseMarked);

You can't have any possible returns between the allocation of thoseMarked and the free. I would just use a do {} while(false) block myself with breaks to handle errors. Also, initialize thoseMarked == nullptr and check for non-null before calling Free.

@@ +1337,5 @@
> +    rv = msgStore->ChangeFlags(messages, nsMsgMessageFlags::Read, true);
> +    NS_ENSURE_SUCCESS(rv, rv);
> +
> +    mDatabase->Commit(nsMsgDBCommitType::kLargeCommit);
> +    nsMemory::Free(thoseMarked);

Same issues about no intermediate returns here of course.
Attachment #8561100 - Flags: review?(rkent) → feedback+
Assignee

Comment 25

4 years ago
Posted patch 840418.patch v1.2 (obsolete) — Splinter Review
OK, thanks.
I have intentionally done it via mozmill as I need to test the path from UI to the backend. That it calls the proper override method.
I hope Magnus can check the test.
Attachment #8561100 - Attachment is obsolete: true
Attachment #8561100 - Flags: review?(mkmelin+mozilla)
Attachment #8564539 - Flags: review?(rkent)
Attachment #8564539 - Flags: review?(mkmelin+mozilla)

Updated

4 years ago
Attachment #8564539 - Flags: review?(rkent) → review+
Comment on attachment 8564539 [details] [diff] [review]
840418.patch v1.2

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

LGTM! r=mkmelin

::: mail/test/mozmill/folder-display/test-message-commands-on-msgstore.js
@@ +14,5 @@
> +const MODULE_REQUIRES = ["folder-display-helpers",
> +                         "compose-helpers",
> +                         "window-helpers"];
> +
> +//Cu.import("resource:///modules/MailUtils.js");

remove

@@ +27,5 @@
> +var gOutbox;
> +var gAutoRead;
> +
> +function setupModule(module) {
> +  for (let lib of MODULE_REQUIRES)

please use braces for all loops

@@ +84,5 @@
> +
> +  let mboxstring = IOUtils.loadFileToString(folder.filePath);
> +
> +  let expectedStatusString = aStatus.toString(16);
> +  while (expectedStatusString.length < 4)

braces
Attachment #8564539 - Flags: review?(mkmelin+mozilla) → review+
Assignee

Comment 27

4 years ago
Thanks, fixed.
Attachment #8564539 - Attachment is obsolete: true
Attachment #8564733 - Flags: review+
Assignee

Updated

4 years ago
Keywords: checkin-needed
Pushed as https://hg.mozilla.org/comm-central/rev/bc4533660d85
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 38.0
Assignee

Comment 29

4 years ago
OS X must be different again. Meta-Shift-Enter only works there if the focus is in the body. So this should fix it.

Aryx, please push to try server, OS X mozmill only. Thanks
Flags: needinfo?(archaeopteryx)
Assignee

Updated

4 years ago
Blocks: 1134024
Assignee

Comment 31

4 years ago
Comment on attachment 8565664 [details] [diff] [review]
fix test on OS X [will be checked in in bug  	1133957]

Yes! The try run is green on OS X.
Attachment #8565664 - Flags: review?(mkmelin+mozilla)
Attachment #8565664 - Flags: review?(mkmelin+mozilla) → review+
But please move the test fix patch to bug 1133957
Assignee

Updated

4 years ago
Depends on: 1133957
Assignee

Updated

4 years ago
Attachment #8564733 - Attachment description: 840418.patch v1.3 → 840418.patch v1.3 [checked in]
Assignee

Updated

4 years ago
Attachment #8565664 - Attachment description: fix test on OS X → fix test on OS X [will be checked in in bug 1133957]

Updated

4 years ago
Duplicate of this bug: 1151697
You need to log in before you can comment on or make changes to this bug.