The default bug view has changed. See this FAQ.

Received messages contain parts of other messages with movemail account

RESOLVED FIXED in Thunderbird 15.0

Status

MailNews Core
Movemail
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Joshua Root, Assigned: Bienvenu)

Tracking

(Depends on: 1 bug, {regression})

Thunderbird 15.0
regression
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird12 fixed, thunderbird13 fixed, thunderbird14 fixed)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Created attachment 618210 [details]
message.txt

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:12.0) Gecko/20100101 Firefox/12.0
Build ID: 20120420145725

Steps to reproduce:

System: OS X 10.6.8. Account type: movemail.

Click "Get Mail" button.


Actual results:

A newly received message sometimes contains a portion of a different message.


Expected results:

Messages should stay separate.

I'm attaching one of the affected messages, with personal info redacted.

This never happened with previous releases. May be related to bug 748597.
Does what I wrote in the other bug also helps with this one ?
(Assignee)

Comment 2

5 years ago
movemail might be the issue - we have very few users who use that, and it probably didn't get tested in the nightly/alpha/beta periods.
(Reporter)

Comment 3

5 years ago
The bisection info in bug 748593 may also be relevant here. It's difficult to reproduce this one on demand. I wouldn't be surprised if the 3 bugs I've filed against TB 12 turned out to be symptoms of the same problem(s), but I don't have any real evidence of that yet so I filed them separately.
(Assignee)

Comment 4

5 years ago
Do you have any filters, or are these messages simply moved into your inbox from the spool? I looked at the movemail code and it's flushing the output stream correctly, so I'm not sure where things would go awry.
Blocks: 402392
(Reporter)

Comment 5

5 years ago
No message filters, but I do have junk filtering on.
(Assignee)

Comment 6

5 years ago
I take it back - we're only flushing the stream at the end of downloading all the messages...I'll look at this a bit and if I can find a potential fix, I'll put up a try server build for you to try.
(Assignee)

Comment 7

5 years ago
code level details for anyone interested - the reason this used to work is that nsMsgMailboxParser::HandleLine used to call PublishMsgHdr when it saw an envelope line here - http://mxr.mozilla.org/comm-esr10/source/mailnews/local/src/nsParseMailbox.cpp#501

If m_newMsgHdr is not set, this is a noop, so when we incorporate new pop3 mail, parsing the envelope line of the new message didn't cause any msg hdr to get published, and when parsing a mail folder or movemail spool file, we would publish the msg hdr for the previous message.

With the pluggable store work, I moved that PublishMsgHeader call into a virtual function OnNewMessage(), which nsParseNewMailState made a noop, and nsMsgMailboxParser called PublishMsgHdr from. The reason was that with pluggable stores, m_newMsgHdr gets set *before* we start parsing the message, which, in the new mail case meant we tried to publish the msg header after parsing the envelope line, which is the very first line.

I didn't realize that movemail counted on the old behavior. I'll have to tweak movemail a bit, I think. I'd love to have an xpcshell test for movemail, but it's extremely difficult if not impossible to set env vars in xpcshell tests, which movemail requires to find the spool file. I could tweak the movemail core code to check for a special pref that only the xpcshell test would set for the spool file, and if not set, the movemail code could use the env var stuff it does now.
(Assignee)

Comment 8

5 years ago
Created attachment 618416 [details] [diff] [review]
potential fix

I haven't tried this yet, but this is my first pass at a fix.
Assignee: nobody → dbienvenu

Updated

5 years ago
Component: Mail Window Front End → Movemail
Keywords: regression
OS: Mac OS X → All
Product: Thunderbird → MailNews Core
QA Contact: front-end → movemail
Hardware: x86 → All

Updated

5 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
Duplicate of this bug: 743889
(Assignee)

Updated

5 years ago
Duplicate of this bug: 713491
(Assignee)

Comment 11

5 years ago
Created attachment 618791 [details] [diff] [review]
fix with unit test

my one worry about this is if the test fails, we leave the .lock file in the profile, so the profile needs to be cleaned up before the next run...that happens for free, I assume.
Attachment #618416 - Attachment is obsolete: true
Attachment #618791 - Flags: review?(mbanner)
(Assignee)

Comment 12

5 years ago
try server builds with this patch will be here: http://ftp.mozilla.org/pub/mozilla.org/thunderbird/try-builds/bienvenu@nventure.com-e4cba7544872
(Reporter)

Comment 13

5 years ago
Bug 748593 is fixed in this build, and there's no sign of this bug or bug 748597 so far. I'll keep using it and see what happens.
(Assignee)

Comment 14

5 years ago
(In reply to Joshua Root from comment #13)
> Bug 748593 is fixed in this build, and there's no sign of this bug or bug
> 748597 so far. I'll keep using it and see what happens.

Thx, Joshua, that's very helpful.
Bienvenu: could you please request a linux64 try-build of comm-central SeaMonkey with this patch, so I could test it with the profile where I noticed bug 713491 (duped to here)?
(Assignee)

Comment 16

5 years ago
sorry, tony, I have no idea about how to do that. Serge could probably do it, though.
(In reply to David :Bienvenu from comment #16)
> (In reply to Tony Mechelynck [:tonymec] from comment #15)
> > Bienvenu: could you please request a linux64 try-build of comm-central
> > SeaMonkey with this patch, so I could test it with the profile where I
> > noticed bug 713491 (duped to here)?
>
> sorry, tony, I have no idea about how to do that. Serge could probably do
> it, though.

Well, if it isn't possible I'll wait for an hourly or nightly with the fix and check that the bug is gone from it.
(In reply to Tony Mechelynck [:tonymec] from comment #17)
> Well, if it isn't possible I'll wait for an hourly or nightly with the fix
> and check that the bug is gone from it.

I am not going to look at (MoMe) Try again until bug 698843 is (fully) fixed :-|
(Assignee)

Updated

5 years ago
Summary: Received messages contain parts of other messages → Received messages contain parts of other messages with movemail account
Attachment #618791 - Flags: review?(mbanner) → review+
Comment on attachment 618791 [details] [diff] [review]
fix with unit test

[Triage Comment]
a=me for landing regression fix on branches.
Attachment #618791 - Flags: approval-comm-release+
Attachment #618791 - Flags: approval-comm-beta+
Attachment #618791 - Flags: approval-comm-aurora+
Checked in:

https://hg.mozilla.org/comm-central/rev/14888d952b42
https://hg.mozilla.org/releases/comm-aurora/rev/fc567a79a47a
https://hg.mozilla.org/releases/comm-beta/rev/947483070d72
https://hg.mozilla.org/releases/comm-release/rev/fc4c8114809d
Status: NEW → RESOLVED
Last Resolved: 5 years ago
status-thunderbird12: --- → fixed
status-thunderbird13: --- → fixed
status-thunderbird14: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 15.0
Just pushed a minor unit-test only bustage fix for this to beta & release (TB 14 had changed the rules so that QI isn't needed so frequently):

http://hg.mozilla.org/releases/comm-beta/rev/3745a9f5d228
http://hg.mozilla.org/releases/comm-release/rev/b5ed052dcc54
Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120429 Firefox/15.0a1 SeaMonkey/2.12a1 ID:20120429003037

Bug 713491 (duped to here) has disappeared. I'll go VERIFY the dupe, leaving it to Thunderbird QA to decide if more verification is necessary on this bug.

Comment 23

5 years ago
Is the printf() in the patch intentional?
http://hg.mozilla.org/releases/comm-release/file/fc4c8114809d/mailnews/local/src/nsMovemailService.cpp#l406

Updated

5 years ago
Duplicate of this bug: 750100
(Assignee)

Comment 25

5 years ago
Created attachment 619719 [details] [diff] [review]
follow on patch to remove printf - checked in
Attachment #619719 - Flags: review?(mbanner)
Attachment #619719 - Flags: review?(mbanner) → review+
(Assignee)

Comment 26

5 years ago
Comment on attachment 619719 [details] [diff] [review]
follow on patch to remove printf - checked in

printf removal landed on trunk - http://hg.mozilla.org/comm-central/rev/d3a9bb8b85f9
Attachment #619719 - Attachment description: follow on patch to remove printf → follow on patch to remove printf - checked in
Duplicate of this bug: 751833
Depends on: 787605
(Reporter)

Updated

4 years ago
Duplicate of this bug: 748593
(Reporter)

Updated

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