Import from Apple Mail.app breaks messages and causes data loss.

RESOLVED FIXED in Thunderbird 3.3a2

Status

Thunderbird
Migration
--
critical
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: David Rekowski, Assigned: Rolandas Naujikas)

Tracking

({student-project, testcase})

Thunderbird 3.3a2
x86_64
Mac OS X
student-project, testcase
Bug Flags:
wanted-thunderbird +

Firefox Tracking Flags

(blocking-thunderbird3.1 -)

Details

(Whiteboard: [has protocol logs][good first bug])

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

8 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_2; de-de) AppleWebKit/531.21.8 (KHTML, like Gecko) Version/4.0.4 Safari/531.21.10
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; de; rv:1.9.1.5) Gecko/20091121 Thunderbird/3.0

I imported my emails from apple mail. Most folders now contain broken messages. Broken means, no sensible header information exist. 
Many messages contain parts of Apple PLists. 
Some contain a section of a mail header. 
Some show the bottom part of  a message and a PList (sometimes with another message appended). 
Sometimes there's binary data included. 
Some are completely empty.
They appear as dated from 1970-01-01. 

The number of messages is correct, i.e. for every message in Apple Mail there is _either_ a correct or a broken message in TB.

Reproducible: Didn't try

Steps to Reproduce:
1. Start Thunderbird 3rc1
2. Import emails from Apple Mail
3. Examine messages dated 1970-01-01
Actual Results:  
The result are broken messages without sender and subject information. 

Expected Results:  
All messages should have been imported correctly.

This was with a fresh installation without any add-ons.
(Reporter)

Updated

8 years ago
Version: unspecified → 3.0
is import log what we need here? https://wiki.mozilla.org/MailNews:Logging
(Reporter)

Comment 2

8 years ago
I tried again, with no change in the result. Did I miss something? The log was empty.
$ env
NSPR_LOG_MODULES=import:5,appleimportlog:5
NSPR_LOG_FILE=tb-import.log
(In reply to comment #2)
> I tried again, with no change in the result. Did I miss something? The log was
> empty.
> $ env
> NSPR_LOG_MODULES=import:5,appleimportlog:5
> NSPR_LOG_FILE=tb-import.log

That probably requires a special build with import being enabled, David could you provide a try build ?

Comment 4

8 years ago
import log should be working fine in a release build, but it's tricky to get working on the mac. For one thing, the applemail import log module is "APPLEMAILIMPORTLOG"
(Reporter)

Comment 5

8 years ago
Could not get a log using

NSPR_LOG_MODULES=APPLEMAILIMPORTLOG:5
NSPR_LOG_MODULES=applemailimportlog:5

Comment 6

8 years ago
Are you setting the env vars using export? I verified that this works for me with a release build.
(Reporter)

Comment 7

8 years ago
Yes, these lines above are copied from evn output. This is what I enter:

$ export NSPR_LOG_MODULES=applemailimportlog:5
$ export NSPR_LOG_FILE=tb-import.log
$ /Applications/Thunderbird.app/Contents/MacOS/thunderbird-bin
(Reporter)

Comment 8

8 years ago
s/evn/env/ , sorry

Comment 9

8 years ago
Where are you looking for the log? I've been cd'ing into the MacOS directory and running thunderbird-bin from there, and the log shows up there.
(Reporter)

Comment 10

8 years ago
This worked. Maybe missed something else before. However, the log is full of these (middle part) messages:

-1600793344[140b860]: nsAppleMailImportModule Created
-1600793344[140b860]: nsAppleMailImportModule Deleted
-1600793344[140b860]: nsAppleMailImportModule Created
-1600793344[140b860]: nsAppleMailImportMail created
-1600793344[140b860]: FindMailboxes for Apple mail invoked
-1600793344[140b860]: Found account: myemail@example.com
...
-1600793344[140b860]: Adding .mbox dir: spam.mbox
-1600793344[140b860]: Will import spam with approx 311 messages, depth is 2
-1600793344[140b860]: trying to locate a '/Users/myusername/Library/Mail/Mailboxes/website/spam'
...
-1600793344[140b860]: nsAppleMailImportModule Deleted
-1600793344[140b860]: nsAppleMailImportMail destroyed

No hints on any problems.
(Reporter)

Comment 11

8 years ago
With *working* I was refering to creating the log. The messagers are still broken as before.
Whiteboard: [has protocol logs]
(Reporter)

Comment 12

8 years ago
I think I found the source for this behaviour: I could confirm that there is a wrong header in a message that broke at import:

>From - Mon Dec 10 10:09:13 2007

Removing this line causes the import to work as expected. Since it is not very practicable to remove those lines by hand I strongly recommend to find a way to deal with those broken header lines, i.e. remove them on import or even better remove them whenever they are encountered. Since the format is very clear, there should be a safe way to do it.

Note: There is another bug report regarding this issue, though in a different context: https://bugzilla.mozilla.org/show_bug.cgi?id=310583
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking-thunderbird3.1?
David Rekowski, if you could create a tiny Mail.app test case that could be used by whoever wants to take that on, it'd be super helpful.
Keywords: student-project
Whiteboard: [has protocol logs] → [has protocol logs][good first bug]

Comment 14

8 years ago
David R, was that line in the middle of a message? And was it really escaped with > in the original, or did it just appear as "From - ..." in the original mail.app message.
(Reporter)

Comment 15

8 years ago
Created attachment 422763 [details]
example Mail.app message file

I obfuscated header and content
(Reporter)

Comment 16

8 years ago
As you can see in the attachment, the line is within the header section and it's escaped. Looking at the source in Mail.app does not show the line, it seems to be removed by Mail.app.
Keywords: testcase

Comment 17

8 years ago
It rather looks like mail.app imported those messages from Thunderbird to start with, since they have x-mozilla-status headers. Two sets of them, even. That makes me disinclined to block on this, though I'd love to see a fix.
blocking-thunderbird3.1: --- → -
status-thunderbird3.1: --- → wanted
Flags: blocking-thunderbird3.1?
(Reporter)

Comment 18

8 years ago
Yes, it was previously imported from TB, but the incorrect line is still in the message file. Only mail.app ignores it, whereas the TB importer does not.

Comment 19

8 years ago
Yeah, I just meant that's going to be a relatively rare round trip. I would love to see this fixed, and it shouldn't be too hard. The importer could just ignore lines that aren't continuation lines and don't have a ':' in them in the header part of the message, before the first blank line.
(Reporter)

Comment 20

7 years ago
Is there anything I can do to help this get fixed? I might even get down to the code if someone points me to where I have to look.
(In reply to comment #20)
> Is there anything I can do to help this get fixed? I might even get down to the
> code if someone points me to where I have to look.

http://mxr.mozilla.org/comm-central/source/mailnews/import/applemail/src/

And you might want to read https://developer.mozilla.org/en/comm-central#Requirements too .
(Reporter)

Comment 22

7 years ago
Well, this won't do for me, I lack the required skills and knowledge of the codebase. Please can someone else pick that up. If you're familiar with the code, I suppose it's quite straightforward.
status-thunderbird3.1: wanted → ---
Flags: wanted-thunderbird+
(Reporter)

Comment 23

7 years ago
Will something be done in regard to this problem? Or can someone explain to me, why this is difficult to implement for TB developers, so I can understand? Thank you.
(Assignee)

Comment 24

7 years ago
By looking to mail import code I found at least one place where the source looks suspicious. In nsEmlxHelperUtils.mm there are lines
...
  // go through foundFromLines
  if (foundFromLines.Length()) {
    // pre-grow the string to the right size
    aOutBuffer.SetLength((aMessageBufferEnd-aMessageBufferStart) + foundFromLines.Length());

    const char *chunkStart = aMessageBufferStart;
    for (unsigned i=0; i<foundFromLines.Length(); ++i) {
      aOutBuffer.Append(chunkStart, (foundFromLines[i]-chunkStart));
      aOutBuffer.Append(NS_LITERAL_CSTRING(">"));

      chunkStart = foundFromLines[i];
    }
  }
...
I think there is missing aOutBuffer.Append(chunkStart, (aMessageBufferEnd-chunkStart)); after loop "for" to append the rest of message to aOutBuffer.

Probably that could lead to random memory content at the end of aOutBuffer written to output mbox.
(In reply to comment #24)
> By looking to mail import code I found at least one place where the source
> looks suspicious. In nsEmlxHelperUtils.mm there are lines

Feel like making a patch ?

Comment 26

7 years ago
Thx for looking into this. Rolandas, if you're not set up to build Thunderbird, we could probably generate a try server build with your suggested change and you could download that and try it. But if you are setup to build Thunderbird, then the best thing would be for you to make the change, test it, and then propose a patch.
(Assignee)

Comment 27

7 years ago
Created attachment 486485 [details] [diff] [review]
proposed patch to this problem

Submitting patch, but I cannot to test it. If developpers could provide new build, I could test it if this problem is solved.

Comment 28

7 years ago
I've requested a try server build with this patch - it should be available in a couple hours. I'll add a link here once its finished.
(Assignee)

Comment 29

7 years ago
Created attachment 486622 [details] [diff] [review]
second proposed patch to this problem

I have built and debug this problem and found several more problems.
With this patch I tried and it works much better.
aOutbuffer.Append appends after mark set by SetLength and there was a mistake in foundFromLines.AppendElement.
Attachment #486485 - Attachment is obsolete: true
(Assignee)

Comment 30

7 years ago
Created attachment 486623 [details] [diff] [review]
third proposed patch to this problem

That corrects situation when "From " appears at the begining.
That should be all for the moment.
Attachment #486622 - Attachment is obsolete: true

Comment 31

7 years ago
Comment on attachment 486623 [details] [diff] [review]
third proposed patch to this problem

OK, cool, so you don't need a try server build, which is good because of course it failed for me, as it nearly always does.

Thx very much for looking at this. We're going to want the fromLineStart++ on its own line, and spaces around the - in aMessageBufferEnd-chunkStart.  Requesting review from Standard8.
Attachment #486623 - Flags: review?(bugzilla)
(Assignee)

Comment 32

7 years ago
OK for cosmetic changes. My idea was about shortest patch possible. About spaces - there also you can look to foundFromLines[i]-chunkStart also.
Comment on attachment 486623 [details] [diff] [review]
third proposed patch to this problem

I've just been trying this and it seemed to fix at least one issue I saw from my Mac import, which is excellent!

r=Standard8 with David's comments addressed. Thanks for the patch.
Attachment #486623 - Flags: review?(bugzilla) → review+
Rolandas, if you're able to attach a new patch, when you do so add checkin-needed to the list of keywords and it'll then get checked in to the tree for you soon after. Thanks again.
Assignee: nobody → rolnas
(Assignee)

Updated

7 years ago
Keywords: checkin-needed
(In reply to comment #34)
> Rolandas, if you're able to attach a new patch, when you do so add

Mark wants a patch were david's issues are addressed.
Keywords: checkin-needed
(Assignee)

Comment 36

7 years ago
My patch solves already most (if not all) problems discussed in this bug report.
(In reply to comment #36)
> My patch solves already most (if not all) problems discussed in this bug
> report.

It doesn't fix the style issues asked for in comment 31. We generally prefer to have complete patches for check in as it makes it easier all round.
I found time to address the style issues in comment 31 and checked this in:

http://hg.mozilla.org/comm-central/rev/c43894b3547a
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.3a2
You need to log in before you can comment on or make changes to this bug.