234935 - Label is only stored in .msf file and not in mbox file if line ending of file isn't the one of the system (tag)

Status: VERIFIED FIXED

(MailNews Core :: Database, defect)

Description

[Christian Eyrich]

On my Linux installation I included accounts from my Windows system. The files
from Windows have CRLF as line ending, the ones native in Linux only LF.
Labeling (by filter or by hand) only works if the file uses LF line endings when
working on Linux.

I edited some mbox files and changed them to LF line endings: now the label
appears in X-Mozilla-Status2. If I change a files line endings from LF to CRLF,
the label I set doesn't show up in X-Mozilla-Status2.

I guess it's vice versa on Windows, there only files with CRLF line endings will
become affected when setting the label. But I didn't test this for now.

Comment 1

[Christian Eyrich]

Yep, using Windows it's vice versa. Having a mbox file with LF line endings the
label will not get saved in the file, only in the .msf.

This is 1. annoying at least when using mailboxes accross different OS and 2.
strange since other changes like read/unread are written to the
X-Mozilla-Status, resp. X-Mozilla-Status2 in all cases.

Comment 2

[David :Bienvenu]

*** Bug 226952 has been marked as a duplicate of this bug. ***

Comment 3

[David :Bienvenu]

I think this is because we figure out where the x-mozilla-status2 line is by
seeking relative to the x-mozilla-status line, and use the native line
terminator in the offset calculation.

Comment 4

[:aceman]

So it seems easy to fix, just read the real line terminator...

Comment 5

[:aceman]

Would something like this do it?
file nsMailDatabase.cpp:
Somewhere around line 410:
         PRInt32 lineLen = PL_strlen(buf);
         fileStream->write(buf, lineLen);
         PRInt32 status2Pos = statusPos + lineLen;
           
          // time to update x-mozilla-status2
	// first find it
	PRInt32 EOLskip;
	// skip at most 2 chars of the linebreak
	 for (EOLskip=1; EOLskip<=2; EOLskip++) {
		status2Pos++;
	         fileStream->seek(status2Pos);
		 fileStream->readline(buf,1);
		if (PL_strlen(buf)) break; // we hit the first non-linebreak char
	}

        fileStream->seek(status2Pos);

I can make a real patch, if the idea is OK.

Comment 6

[Ray Booysen]

aceman, can you make a patch for testing?

Comment 7

[Ray Booysen]

woops, sorry for the bugspam, hit enter by accident.

Comment 8

[Wayne Mery (:wsmwk)]

David, is this OS=All?  Still valid?

Comment 9

[David :Bienvenu]

yes, looking at the code, it's still valid, and OS=ALL, and aceman's fix is roughly ok - I wouldn't use readline, though - I'd just use read and skip over 1 or 2 linebreak chars until I found an X.

Comment 10

[:aceman]

Sorry, I am not familiar with mozilla code and functions, so I just used functions , where I could guess their meaning ;) I could look into it deeper, but it won't be soon. And I don't suffer from the problem in the description, I didn't migrate mi files to linux so far ;) I just threw in my fix proposal. Can anybody take it sooner?

Comment 11

[Wayne Mery (:wsmwk)]

if someone can improve the patch please pick it up and run ... (even though bienvenu's assigned)

Comment 12

[Mark Banner (:standard8)]

This seems to be a using Thunderbird on cross-platform issue which isn't really supported in Thunderbird's default configuration. Therefore not blocking TB 3 on this but we'd probably take the patch if someone would pick it up.

Comment 13

[:aceman]

Attachment: Patch against TB 3.1.4

Comment 14

[:aceman]

Reporter, can you check this patch fixes your problem? Is it still relevant in Thunderbird 3.1?

Comment 15

[Christian Eyrich]

If no one else fixed it, It’s still relevant. But I can’t test the fix since I currently have no installation to build TB.

Comment 16

[David :Bienvenu]

Attachment: tweaked fix

I've taken your patch and tweaked it a little. I'll land it soon. Thx for the patch!

Comment 17

[David :Bienvenu]

I should point out that we don't store labels as flags anymore; we store them as keywords.

Comment 18

[David :Bienvenu]

fix checked in.

Comment 19

[:aceman]

Thanks! I agree with your tweaks, I didn't know how to properly do error checking.

Reporter, as the fix was checked in, it seems you could now try precompiled Thunderbird 3.3a1 nightlies. Please confirm if the patch works.

Comment 20

[Christian Eyrich]

I just noted that as David wrote, labels are stored differently now and I don't know how to trigger change of X-Mozilla-Status2 for an existing message.
At least everything works as it should when using files with "the other" file ending with the patch.

Comment 21

[:aceman]

The problem with X-Mozilla-Status2 may be bug 383308, please join it if that is what you meant.

Verifying this bug according to your comment.