Last Comment Bug 805626 - when going back to online mode, drafts saved while in offline mode are deleted, critical data loss
: when going back to online mode, drafts saved while in offline mode are delete...
Status: RESOLVED FIXED
[gs]
: dataloss, regression, reproducible
Product: MailNews Core
Classification: Components
Component: Networking: IMAP (show other bugs)
: 16
: All All
: -- critical with 5 votes (vote)
: Thunderbird 20.0
Assigned To: David Lechner (:dlech)
:
:
Mentors:
https://getsatisfaction.com/mozilla_m...
Depends on: 402392
Blocks: 782490
  Show dependency treegraph
 
Reported: 2012-10-25 14:35 PDT by Jason Paul Joines
Modified: 2015-12-20 17:32 PST (History)
21 users (show)
standard8: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
+
fixed
fixed
18+
fixed


Attachments
patch v1 (11.37 KB, patch)
2012-12-06 15:03 PST, David Lechner (:dlech)
standard8: review+
standard8: approval‑comm‑aurora+
standard8: approval‑comm‑beta+
standard8: approval‑comm‑esr17+
Details | Diff | Splinter Review
First error displayed on draft save failure (14.58 KB, image/png)
2012-12-11 18:37 PST, David Lechner (:dlech)
no flags Details
second dialog displayed on draft save failure (15.82 KB, image/png)
2012-12-11 18:38 PST, David Lechner (:dlech)
no flags Details
third dialog displayed on draft save failure (after canceling retry) (16.39 KB, image/png)
2012-12-11 18:38 PST, David Lechner (:dlech)
no flags Details

Description Jason Paul Joines 2012-10-25 14:35:35 PDT
Working on a draft email message and saved it several times.  Switched to offline mode to continue working in location without internet access and saved the draft several mores.  Closed the draft message.  Went back into online mode when re-connected to the internet.  Selected Drafts folder and saw many copies of the draft message.

Then all copies of the draft message disappeared.  Selected several different mail folders to see if the draft had somehow ended up there including the Drafts folder under Local Mail but it was nowhere to be found.  Noticed that status bar said something along the lines of "Deleted 11 message from Drafts".  Tried Ctrl+Z but nothing was undone.

Apparently this is a an issue that has been going on for many years, in many versions of Thunderbird, on many platforms, but as far as I can tell noone has filed a bug - https://getsatisfaction.com/mozilla_messaging/topics/imap_draft_composed_offline_deleted_when_going_online-12lf7.

Either saving multiple copies of the draft message until sent or only the most recent copy until sent would be reasonable behavior.  Deleting the message is not acceptable behavior.
Comment 1 WADA 2012-10-25 21:56:15 PDT
Same problem as bug 782490 reported for Tb 14?
See bug 638358 for phenomenon in Tb 3.
See bug 397910 and duped bugs for phenomeno in older Tb.
Comment 2 Jonathan Kamens 2012-10-30 08:03:51 PDT
I was able to reproduce this issue, as was one of the users of my add-on, Send Later.

This is a data loss bug and is therefore rightly marked critical. I hope something is done about it as soon as possible!
Comment 3 Ludovic Hirlimann [:Usul] 2012-11-02 06:28:33 PDT
So we'de need more STR. Is this over POP3 or Imap ? Is it trigerred everytime or "randomly" , in the later case what brings the issue up ?
Comment 4 Jonathan Kamens 2012-11-02 06:36:00 PDT
IMAP. Every single time.
It's sort of obvious that it isn't POP3 because Drafts folders in POP3 are stored locally, and therefore aren't resynchronized when you come back online.
The GetSatisfaction article linked to gives the STR clearly and explicitly and also clearly and explicitly states that it's IMAP and that it's reproducible.
Of course, the GetSatisfaction article also claims that the issue is solved in Thunderbird 3.1, though I see no explanation for that claim in the comments, and if it was solved in 3.1, it has apparently regressed.
Comment 5 WADA 2012-11-02 09:58:30 PDT
(In reply to Ludovic Hirlimann [:Usul] from comment #3)
> So we'de need more STR.

STR is pretty simple. Read bug 782490 also, please.
(1) Use IMAP folder as Drafts folder
(2) Save composing mail(s) as draft mail(s) while Offline mode of Tb with a required Tb release or newer, then Go back to Online mode of Tb.

Does your question mean that you could'nt reproduce problem with any test procedure in any environment?
If so, could you please explain your tests and your test results?
Comment 6 Ludovic Hirlimann [:Usul] 2012-11-05 08:15:41 PST
My email is sent when I go back online.
Comment 7 Jonathan Kamens 2012-11-05 08:23:25 PST
Ludovic, we're not talking about email in the Outbox, we're talking about email in the Drafts folder.
Comment 8 Ludovic Hirlimann [:Usul] 2012-11-05 08:38:59 PST
I can't reproduce in the latest nightly - and yes sorry for the miss test Jonathan.
Comment 9 Ludovic Hirlimann [:Usul] 2012-11-05 08:39:19 PST
Though I saved manually I didn't wait for the auto-save to trigger.
Comment 10 Jonathan Kamens 2012-11-05 12:00:06 PST
I just did python client.py checkout and rebuilt trunk and was able to reproduce this issue:

1. File | Offline | Work Offline
2. Open Compose window
3. Put something in To, Subject and body
4. Click Save
5. Close Compose window
6. Select Drafts folder and observe draft saved there
7. File | Offline | Work Offline to go back online
8. Draft disappears from Drafts folder

Sometimes you have to focus away from the Drafts folder and back to it before the draft disappears.
Comment 11 WADA 2012-11-05 20:31:59 PST
Closing as dup of bug 782490.
If duping is wrong, please re-open this ug.

*** This bug has been marked as a duplicate of bug 782490 ***
Comment 12 Jonathan Kamens 2012-11-05 20:35:58 PST
I am reopening this bug, because I've read the scenario described by the user who filed bug 782490, and it's nothing like what I'm experiencing as reported in this bug. The two bugs may be related, but that has yet to be proven from code examination.
Comment 13 WADA 2012-11-05 20:50:07 PST
(In reply to Jonathan Kamens from comment #12)
STR of your comment #10 is same as bug 782490 comment #4 and phenomenon you saw in your comment #10 is a part of bug 782490 comment #4(offset=non-ZERO part), isn't it?
Comment 14 Jonathan Kamens 2012-11-05 20:52:48 PST
Wada, my point is that your comment 4 in that bug bears little resemblance to the initial bug description submitted by the user. Your comment 4 fails to reproduce a bunch of the symptoms described in the bug description.

Therefore, although comment 4 of that bug may indeed be identical to this one, the initial bug report is not. You should have posted that comment on _this_ bug. :-)
Comment 15 WADA 2012-11-05 21:26:37 PST
I see.
Setting dependency of that bug to this bug for ease of tracking and analysis.
Comment 16 WADA 2012-11-05 21:37:26 PST
As I wrote in bug 782490 comment #4, "Order Received" column value(==MsgKey) was "offset in offline-store file" instead of "negative 32bits signed integer in Tb 3".
This is perhaps phenomenon after changes for bug 402392(actual patch is by bug 462665) which was landed on Tb 12.
Comment 17 Joe Smith 2012-11-13 06:29:05 PST
Same on WinXP SP3 32 bit. The data loss is critical indeed.
Comment 18 Greg Wilson 2012-11-15 07:38:40 PST
Hit by this twice in the last week using TB 16.0.2 on Mac OS X 10.8; initially thought it was because I was composing/saving draft while on a very flaky WiFi connection, but had the same behavior when composing with network completely turned off, then going back online.
Comment 19 Will Rico 2012-11-15 09:13:13 PST
(In reply to Jonathan Kamens from comment #10)
I'm experiencing the exact behavior described in Comment #10.  I'm using Thunderbird 16.0.1 on Ubuntu 11.04.  I'm syncing my gmail.com account via IMAP. Under 'Account Settings' -> 'Copies & Folders,' I have the Drafts folder set to "Drafts on [emailaddress]@gmail.com."
Comment 20 David von Oheimb 2012-11-17 10:40:23 PST
I can confirm the existence of this bug, in the latest TB 16, on Linux and Mac.
I recently asked for re-opening bug 552208, which got closed for reasons that I do not understand, but so far it was not re-opened. Thus I'm repeating here the very simple steps to reproduce it:

1. Work offline by clicking the offline-icon on the lower left-hand corner
2. Save a draft message in IMAP folder; find the message in the drafts folder
3. Re-establish connection by clicking again on the icon
4. Watch as your messages disappear from the drafts folder

Let me stress again that this bug is critical, since it can cause several hours of work to get lost, for instance after editing long emails offline while traveling. 
I experienced this very unpleasant effect several times.
Comment 21 :Irving Reid (No longer working on Firefox) 2012-11-20 08:51:26 PST
Running a debug version of Thunderbird, when I save the draft in off-line mode I get the following two assertion failures:

###!!! ASSERTION: RemoveCurrentDraftMessage msgSend is null.: 'msgSend', file /Users/ireid/tbird/comm-central/mailnews/compose/src/nsMsgCompose.cpp, line 3909
###!!! ASSERTION: Uh, IsInModalState() called w/o a reachable top window?: 'Error', file /Users/ireid/tbird/comm-central/mozilla/dom/base/nsGlobalWindow.cpp, line 6850

The first assertion is in code that seems related to this issue, and it was introduced by the change in bug 299655. Unfortunately, backing out the change form 299655 doesn't make the drafts problem go away so there must be something else wrong...
Comment 22 Wayne Mery (:wsmwk, NI for questions) 2012-11-29 04:28:42 PST
If this and bug 782490 is a reproducible regression, then it should be easy for someone to find the regression range
Comment 23 David Lechner (:dlech) 2012-12-05 23:00:25 PST
Apparently, this was a Christmas present. regression occurred between c3df472badcd on 12/24/11 and 3b30e81cd037 on 12/25/11
Only commit that touches imap there is bug 402392.

It is a huge patch, so it will take some time to sort through.
I'll keep working on this - unless irving doesn't have anything better to do :-)

did not see this assertion, so I don't think it is related.
> ###!!! ASSERTION: RemoveCurrentDraftMessage msgSend is null.: 'msgSend', file /Users/ireid/tbird/comm-central/mailnews/compose/src/nsMsgCompose.cpp, line 3909
Comment 24 David Lechner (:dlech) 2012-12-05 23:02:16 PST
bienvenu, looks like you did most of the work on plugable storage. Any suggestions on where to start?
Comment 25 David Lechner (:dlech) 2012-12-06 15:03:07 PST
Created attachment 689412 [details] [diff] [review]
patch v1

I figured it out! Pretty excited about it.

When the plugable storage was implemented, in nsImapService::OfflineAppendFromFile the function used to create a new message header was nsIMsgDatabase::GetNewMsgOutputStream, which creates its own database key for the header rather than using the fake key that was generated. 

When returning online, nsImapMailFolder::CopyFileToOfflineStore goes to look up the message to copy it to the server, but cannot find it because it is looking for the fake key rather than the auto generated one.

I fixed nsImapService::OfflineAppendFromFile so that is uses the fake key instead of an auto-generated one.

Also while working on this, I would get the following assertion:
###!!! ASSERTION: didn't finish prev msg: 'm_streamOutstandingFolder != aFolder', file c:/dev/tb/obj/mailnews/local/src/../../../../comm-central/mailnews/local/src/nsMsgBrkMBoxStore.cpp, line 612

This was because we are not calling nsIMsgPluggableStore::FinishNewMessage when we are done creating/saving the draft message. Fixed this as well.
Comment 26 :Irving Reid (No longer working on Firefox) 2012-12-11 11:29:33 PST
Comment on attachment 689412 [details] [diff] [review]
patch v1

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

I like it a lot, especially the test case, but I'd like :bienvenu or maybe :standard8 to look it over.

::: mailnews/imap/src/nsImapService.cpp
@@ +2029,5 @@
>        aDstFolder->GetServer(getter_AddRefs(dstServer));
>        rv = dstServer->GetMsgStore(getter_AddRefs(msgStore));
>        NS_ENSURE_SUCCESS(rv, rv);
> +      rv = destDB->CreateNewHdr(fakeKey, getter_AddRefs(newMsgHdr));
> +      NS_ENSURE_SUCCESS(rv, rv);

What happens if we fail out here? Does the draft get thrown away, is there anything the user can do to recover?

@@ +2030,5 @@
>        rv = dstServer->GetMsgStore(getter_AddRefs(msgStore));
>        NS_ENSURE_SUCCESS(rv, rv);
> +      rv = destDB->CreateNewHdr(fakeKey, getter_AddRefs(newMsgHdr));
> +      NS_ENSURE_SUCCESS(rv, rv);
> +      rv = aDstFolder->GetOfflineStoreOutputStream(newMsgHdr, getter_AddRefs(offlineStore));

GetOfflineStoreOutputStream() is just a wrapper that gets the message store and calls msgStore->GetNewMsgOutputStream(), so after the CreateNewHdr() I think you could just leave the GetNewMsgOutputStream() call that was in the previous version.

@@ +2067,1 @@
>            msgParser->SetNewMsgHdr(newMsgHdr);

What's the reason for this change?

::: mailnews/imap/test/unit/test_offlinePlayback.js
@@ +60,5 @@
>      copyService.CopyFileMessage(file, gIMAPInbox, null, false, 0,
>                                  "", asyncCopyListener, null);
>      yield false;
>    },
> +  function goOnline() {

This is just cleaning up a misleading name, and doesn't change any part of the test execution, right?
Comment 27 David Lechner (:dlech) 2012-12-11 18:36:51 PST
(In reply to Irving Reid (:irving) from comment #26)
> Comment on attachment 689412 [details] [diff] [review]
> patch v1
> 
> Review of attachment 689412 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I like it a lot, especially the test case, but I'd like :bienvenu or maybe
> :standard8 to look it over.
> 
> ::: mailnews/imap/src/nsImapService.cpp
> @@ +2029,5 @@
> >        aDstFolder->GetServer(getter_AddRefs(dstServer));
> >        rv = dstServer->GetMsgStore(getter_AddRefs(msgStore));
> >        NS_ENSURE_SUCCESS(rv, rv);
> > +      rv = destDB->CreateNewHdr(fakeKey, getter_AddRefs(newMsgHdr));
> > +      NS_ENSURE_SUCCESS(rv, rv);
> 
> What happens if we fail out here? Does the draft get thrown away, is there
> anything the user can do to recover?

I artificially injected an error here by adding the line:
> rv = NS_MSG_INVALID_CUSTOM_HEADER;
NS_MSG_INVALID_CUSTOM_HEADER is an actual error that can be returned by GetMsgStore(), so this should be a valid test.

The result is that you get an error dialog saying that the copy failed. And then a confirm dialog asking you if you want to retry. I will attach screenshots so you can see the actual dialogs. If you don't retry, message compose window stays open, so data is preserved.

> 
> @@ +2030,5 @@
> >        rv = dstServer->GetMsgStore(getter_AddRefs(msgStore));
> >        NS_ENSURE_SUCCESS(rv, rv);
> > +      rv = destDB->CreateNewHdr(fakeKey, getter_AddRefs(newMsgHdr));
> > +      NS_ENSURE_SUCCESS(rv, rv);
> > +      rv = aDstFolder->GetOfflineStoreOutputStream(newMsgHdr, getter_AddRefs(offlineStore));
> 
> GetOfflineStoreOutputStream() is just a wrapper that gets the message store
> and calls msgStore->GetNewMsgOutputStream(), so after the CreateNewHdr() I
> think you could just leave the GetNewMsgOutputStream() call that was in the
> previous version.


No, GetNewMsgOutputStream() creates its own new message header @ https://mxr.mozilla.org/comm-central/source/mailnews/local/src/nsMsgBrkMBoxStore.cpp#663
We want to use the the message header that we created using CreateNewHdr() so that it uses the fakeKey that was generated.

> 
> @@ +2067,1 @@
> >            msgParser->SetNewMsgHdr(newMsgHdr);
> 
> What's the reason for this change?
> 
> ::: mailnews/imap/test/unit/test_offlinePlayback.js
> @@ +60,5 @@
> >      copyService.CopyFileMessage(file, gIMAPInbox, null, false, 0,
> >                                  "", asyncCopyListener, null);
> >      yield false;
> >    },
> > +  function goOnline() {
> 
> This is just cleaning up a misleading name, and doesn't change any part of
> the test execution, right?

Yes, this was just an ill named function that I ran across while working on this bug.
Comment 28 David Lechner (:dlech) 2012-12-11 18:37:40 PST
Created attachment 691179 [details]
First error displayed on draft save failure
Comment 29 David Lechner (:dlech) 2012-12-11 18:38:10 PST
Created attachment 691180 [details]
second dialog displayed on draft save failure
Comment 30 David Lechner (:dlech) 2012-12-11 18:38:44 PST
Created attachment 691182 [details]
third dialog displayed on draft save failure (after canceling retry)
Comment 31 mozilla 2012-12-12 06:14:31 PST
Why would you show a warning like "third dialogue"? We know that it is not about the mail & newsgroups account settings. With this kind of error, users will start to question and change their settings, mess things up and look on the wrong spot. Why don't you show a warning like "Could not save draft to IMAP folder, please save it in your local account". Thx
Comment 32 Jonathan Kamens 2012-12-12 06:18:19 PST
I don't know of any way to tell Thunderbird while editing a draft to save it in a drafts folder other than the one associated with the account. I totally agree with you that there should be a way to do this, because if the IMAP server suddenly becomes inaccessible for whatever reason, there is data loss if the user can't save the draft. There is a similar problem with a lost copy of an outgoing message, which I reported in bug 726377.
Comment 33 mozilla 2012-12-12 08:24:13 PST
Well, then pls just make it "Could not save draft to IMAP folder, this is a bug that is being worked on." I am just mentioning, that the error message "wrong mail & newsgroups account settings" is simply not true and will lead users to the wrong assumptions. Thank you for working on this.
Comment 34 David von Oheimb 2012-12-12 08:40:13 PST
(In reply to Jonathan Kamens from comment #32)
> I don't know of any way to tell Thunderbird while editing a draft to save it
> in a drafts folder other than the one associated with the account.

In this case, the user may still decide to save the draft as a file or as a template (where the latter is what I prefer, since it preserves the header etc.).

> I totally agree with you that there should be a way to do this, because if the IMAP
> server suddenly becomes inaccessible for whatever reason, there is data loss
> if the user can't save the draft.

I hope that the new patch still allows for (automatically or manually) saving the file to the offline copy of the drafts folder as long as the IMAP server is unreachable because the machine is offline, such that the above situation (with the error dialogue will) only occur if there is any /other/ problem with the IMAP server.
Comment 35 David Lechner (:dlech) 2012-12-12 10:11:14 PST
(In reply to mozilla from comment #31)
> Why would you show a warning like "third dialogue"?  We know that it is not 
> about the mail & newsgroups account settings.
I just injected a random error to make sure that *something* happened other than the draft message disappearing. The actual error messages are really outside of the scope of this particular bug.

> "Could not save draft to IMAP folder, please save it in your local account"
Something like this has been suggested in Bug 354064

(In reply to Jonathan Kamens from comment #32)
> I don't know of any way to tell Thunderbird while editing a draft to save it
> in a drafts folder other than the one associated with the account. 
This is very close to being implemented in Bug 743913 although the assignee appears to have gone missing before the patch actually landed.
Comment 36 Robin Lovelace 2012-12-30 03:16:15 PST
Gutted to find 3 long emails I composed on the train lost due to this bug.
Great to see the bug's been acknowledged and that a fix is imminent. 

For people like me clicking on the bug AFTER data loss:
Is there any way to recover the drafts lost due to this process?

Disappointed to see such a critical bug in software as mature as Thunderbird.

Robin
Comment 37 David von Oheimb 2012-12-30 07:48:36 PST
(In reply to Robin Lovelace from comment #36)
> Gutted to find 3 long emails I composed on the train lost due to this bug.

> Disappointed to see such a critical bug in software as mature as Thunderbird.

I fully agree. I had the very same experience and was pretty **** off, in particular as it is a regression and there is already an old bug report for this problem:
https://bugzilla.mozilla.org/show_bug.cgi?id=552208
Yet that one was (at least in may view) *wrongly* marked by two TB gurus as a duplicate of related issues, and thus it looks like this evil bug has been left open for more than 2.5 years. My complaints on that bug thread have been ignored, but after posting the problem again in this thread I was glad to see that others finally picked it up.

> For people like me clicking on the bug AFTER data loss:
> Is there any way to recover the drafts lost due to this process?

Yes, as long as you did not compact your Drafts folder in the meantime. The drafts should be in the file representing the offline copy of that IMAP folder, e.g.:
/Users/david/Library/Thunderbird/Profiles/david.default/ImapMail/imap.gmx.net/Drafts
Comment 38 Robin Lovelace 2012-12-30 11:02:06 PST
Many thanks for the quick response David. I'm glad to report that I managed to recover deleted emails from:

/home/robin/.thunderbird/0bf9xox7.default/ImapMail/imap.gmail.com/[Gmail].sbd/Drafts

I should have known this anyway, from the account settings where it asks where to keep local copies and how frequently to compact folders. Very useful advice!

If I'm a relatively competent TB user and struggle to find this backup, how many people must there be who have lost information but a) do not know how to report in a bug and b) do not know how to recover the information.

As pointed out above, https://bugzilla.mozilla.org/show_bug.cgi?id=805626#c23 , great Christmas present Thunderbird has provided for us!

Many thanks to people working on this problem, and look forward to a fix in the next version. In the meantime I recommend that people ensure that their draft folders are synced locally.
Comment 39 Mark Banner (:standard8, afk until Dec) 2012-12-31 04:35:46 PST
Comment on attachment 689412 [details] [diff] [review]
patch v1

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

This looks great. I can certainly agree with the analysis of not using the fake header key and the rest of it looks fine as well, so r=me.

I think we should also take this onto branches so that we can pick it up in the next releases.
Comment 40 Mark Banner (:standard8, afk until Dec) 2012-12-31 04:37:48 PST
Landed on trunk:

https://hg.mozilla.org/comm-central/rev/83db5d0dd5fc

I'll land this on branches once some of trunk has cycled green.
Comment 42 David von Oheimb 2013-01-17 02:14:47 PST
I just tried the latest available beta (18.0b1). The bug is still present there.
When is a publicly available release/beta going to be available including the fix?
Comment 43 :Irving Reid (No longer working on Firefox) 2013-01-22 13:17:37 PST
Thunderbird Beta 19.0b1 is now available; please try that: http://www.mozilla.org/en-US/thunderbird/all-beta.html

Note You need to log in before you can comment on or make changes to this bug.