Closed
Bug 418727
Opened 17 years ago
Closed 17 years ago
Save as text only includes the email subject
Categories
(Thunderbird :: General, defect)
Thunderbird
General
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: philor, Assigned: smaug)
References
Details
(Keywords: regression)
Attachments
(2 files, 1 obsolete file)
1.98 KB,
patch
|
peterv
:
review+
peterv
:
superreview+
beltzner
:
approval1.9+
|
Details | Diff | Splinter Review |
4.81 KB,
patch
|
Details | Diff | Splinter Review |
STR:
1. Send yourself a plaintext email, with the subject "Subject" and the body "Body"
2. Select it in the threadpane, press ctrl+S
3. Change the filename from Subject.eml to Subject.txt
4. Save, then open in a text editor
Expected: file contains "Body"
Actual: file contains nothing but "Subject"
Regressed between 2008-02-19-03 and 2008-02-20-03 Windows builds, my first suspicion is bug 331530 but I haven't had a chance to test other platforms or try backing it out yet.
Reporter | ||
Comment 1•17 years ago
|
||
Grr, where by bug 331530 I *mean* bug 417384.
Assignee | ||
Comment 3•17 years ago
|
||
Ok, I was wrong. Sorry. OpenHead does get called. Thunderbird serializes
messages so that CNavDTD::AddHeadContent opens head.
Adding only NS_WARN_IF_FALSE to deconstructor, because that is sort of valid
case if something has caused serialization to stop while in <head>.
Attachment #304632 -
Flags: superreview?(peterv)
Attachment #304632 -
Flags: review?(peterv)
Assignee | ||
Updated•17 years ago
|
Severity: normal → major
Assignee | ||
Updated•17 years ago
|
Attachment #304632 -
Flags: approval1.9?
Comment 5•17 years ago
|
||
Shouldn't you add a testcase for correct behavior, too, to complement the one added in the bug that caused this regression?
Flags: in-testsuite?
Comment 6•17 years ago
|
||
Comment on attachment 304632 [details] [diff] [review]
proposed patch
Needs test.
Attachment #304632 -
Flags: superreview?(peterv)
Attachment #304632 -
Flags: superreview+
Attachment #304632 -
Flags: review?(peterv)
Attachment #304632 -
Flags: review+
Assignee | ||
Comment 7•17 years ago
|
||
Yes, tests are needed, if I just found some strange way to test this.
The relevant interfaces (nsIParser, nsIContentSink, nsIHTMLToText) aren't
scriptable.
Assignee | ||
Comment 8•17 years ago
|
||
The test is basically (succeeds without bug 417384 or with the patch in this bug):
void
ConvertBufToPlainText(nsString &aConBuf)
{
nsCOMPtr<nsIParser> parser = do_CreateInstance(kCParserCID);
if (parser) {
nsCOMPtr<nsIContentSink> sink;
sink = do_CreateInstance(NS_PLAINTEXTSINK_CONTRACTID);
if (sink) {
nsCOMPtr<nsIHTMLToTextSink> textSink(do_QueryInterface(sink));
if (textSink) {
nsAutoString convertedText;
textSink->Initialize(&convertedText, 0, 72);
parser->SetContentSink(sink);
parser->Parse(aConBuf, 0, NS_LITERAL_CSTRING("text/html"), PR_TRUE);
aConBuf = convertedText;
}
}
}
}
nsString test;
test.AppendLiteral("<html><base>base</base><head><span>span</span></head><body>body</body></html>");
ConvertBufToPlainText(test);
printf("Test %s \n", test.EqualsLiteral("basespanbody") ? "succeeded" : "failed");
Assignee | ||
Comment 9•17 years ago
|
||
Will add a binary test
Assignee | ||
Comment 10•17 years ago
|
||
Assignee | ||
Comment 11•17 years ago
|
||
Attachment #304704 -
Attachment is obsolete: true
Comment 12•17 years ago
|
||
Comment on attachment 304632 [details] [diff] [review]
proposed patch
a=beltzner for 1.9
Attachment #304632 -
Flags: approval1.9? → approval1.9+
Assignee | ||
Comment 13•17 years ago
|
||
Checked in the patch and test, and test run successfully on tboxes.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Verified FIXED using both:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b4pre) Gecko/2008022203 Thunderbird/3.0a1pre
and
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008022206 Thunderbird/3.0a1pre ID:2008022206
Smaug: since you've landed tests, should you mark |in-testsuite +|?
Status: RESOLVED → VERIFIED
OS: Windows XP → All
Hardware: PC → All
Assignee | ||
Updated•17 years ago
|
Flags: in-testsuite? → in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•