Closed Bug 39508 Opened 24 years ago Closed 24 years ago

Interleaved comments loose linebreaks during HTML output

Categories

(Core :: DOM: Editor, defect, P1)

Product:
Core
Component:
DOM: Editor
Type:
defect

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: BenB, Assigned: akkzilla)

Details

(Whiteboard: [nsdogfood+] FIX IN HAND) 

Reproduction:
1. In Mailnews, hit reply.
2. I interleave quotes with comments, i.e.
2.1. set the caret into the quote,
2.2. press return 3 times,
2.3. cursor-up once and
2.4. start typing. Use return several time in between.
3. Debug->OutputXIF
4. Debug->OutputHTML
5. Debug->OutputTXT

Actual result:
All linebreaks you typed are lost, i.e. the whole text is treated as one long
paragraph. See debug output below.

Expected result:
There're <p>s or at least <br>s in the HTML, linebreaks in the text output.

Debug output:

Getting XIF
<?xml version="1.0"?>
<!DOCTYPE xif>
<encode selection="1"/>
<section>
<section_head>
<document_info charset="ISO-8859-1" uri="about:blank"/>
</section_head>
<section_body>
<markup_declaration><content>DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0
Transitional//EN"</content>
</markup_declaration>
<container isa="body"><content>akkana@netscape.com (Akkana Peck)
wrote:</content>
<leaf isa="br">
<attr name="_moz_dirty" value=""/>
</leaf><!--br-->
<container isa="blockquote"><attr name="_moz_dirty" value=""/>
<attr name="type" value="cite"/>
<attr name="cite" value="3921C9A0.942A0C61@netscape.com"/>
<container isa="pre"><attr name="wrap" value=""/>
<attr name="_moz_dirty" value=""/>
<content></content>
<entity value="gt"/><content> - It occurs also on recent nightlies, so it's
*not* caused by my changes</content>
<leaf isa="br">
<attr name="_moz_dirty" value=""/>
</leaf><!--br-->
<content></content>
<entity value="gt"/><content> for bug 31906 - puh! (I also don't know, how it
could.)</content>
</container><!--pre-->
</container><!--blockquote-->
<content>gzui zui ziufi zi ui zuizui ri rzu rui ui u i</content>
<leaf isa="br">
<attr name="_moz_dirty" value=""/>
</leaf><!--br-->
<leaf isa="br">
<attr name="_moz_dirty" value=""/>
</leaf><!--br-->
<content>zu izui tzu itezu ezu tz jtz zt</content>
<leaf isa="br">
<attr name="_moz_dirty" value=""/>
</leaf><!--br-->
<leaf isa="br">
<attr name="_moz_dirty" value=""/>
</leaf><!--br-->
<container isa="blockquote"><attr name="_moz_dirty" value=""/>
<attr name="type" value="cite"/>
<attr name="cite" value="3921C9A0.942A0C61@netscape.com"/>
<container isa="pre"><attr name="wrap" value=""/>
<attr name="_moz_dirty" value=""/>
<content></content>
<entity value="gt"/><content> - It does not happen, if I send a new, fresh
mail</content>
<leaf isa="br">
<attr name="_moz_dirty" value=""/>
</leaf><!--br-->
<content></content>
<entity value="gt"/><content> - It does happen, if I interleave quotes with
comments, i.e.</content>
</container><!--pre-->
</container><!--blockquote-->
<leaf isa="br">
<attr name="_moz_dirty" value=""/>
</leaf><!--br-->
<leaf isa="br">
<attr name="_moz_dirty" value=""/>
<attr name="type" value="_moz"/>
</leaf><!--br-->
</container><!--body-->
</section_body>
</section>

Getting HTML
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN">
<body>akkana@netscape.com (Akkana Peck) wrote:<br>
<blockquote type="cite" cite="3921C9A0.942A0C61@netscape.com">
<pre wrap>&gt; - It occurs also on recent nightlies, so it's *not* caused by my
changes
&gt; for bug 31906 - puh! (I also don't know, how it
could.)</pre></blockquote>gzui zui ziufi zi ui zuizui ri rzu rui ui u i

zu izui tzu itezu ezu tz jtz zt

<blockquote type="cite" cite="3921C9A0.942A0C61@netscape.com"><pre wrap>&gt; -
It does not happen, if I send a new, fresh mail
&gt; - It does happen, if I interleave quotes with comments,
i.e.</pre></blockquote>

</body>

Getting text
<<akkana@netscape.com (Akkana Peck) wrote:
> 
> > - It occurs also on recent nightlies, so it's *not* caused by my changes
> > for bug 31906 - puh! (I also don't know, how it could.)
gzui zui ziufi zi ui zuizui ri rzu rui ui u i

zu izui tzu itezu ezu tz jtz zt

> > - It does not happen, if I send a new, fresh mail
> > - It does happen, if I interleave quotes with comments, i.e.

>>
Note, that there *are* brs in the XIF output, but not HTML output.
assigning to akk
Assignee: beppe → akkana
Target Milestone: --- → M17
Apparently something changed recently which broke the handling of flags inside
the nsDocumentEncoder -- cmanske reported a similar bug with the
OutputSelectionOnly flag not being followed.  I'm investigating to see what
broke and why.
Status: NEW → ASSIGNED
This makes Mailnews pretty unusable. Interleaved comments are a must for good

netkeeping.



I think, Akk is aware of that. But adding DOGFOOD nevertheless, so she gets

checkin permission for M16.
Keywords: dogfood
Bumping priority to make sure it gets looked at by the triage gods.
Priority: P3 → P1
It looks like this was caused by the fix for 36188.  Backing out that fix (3
lines starting at line 634 of nsHTMLContentSinkStream.cpp) cures the problem. 
Again, nsHTMLToTXTSinkStream.cpp seems to do it right (Debug->Output Text shows
the newlines, but Output HTML lacks the br tags), so we must be outputting to
html and then converting html to plaintext ...?
Ops, didn't notice that. I saw the bug in *flowed* mails, that I sent out. Does
this matter?
> we must be outputting to html and then converting html to plaintext ...?

You mean XIF -> HTML -> TXT? Judging from the data paths I've seen in Mailnews
:-( : well possible.
[nsdogfood+]
Whiteboard: [nsdogfood+]
Changing milestone to M16 to reflect nsdogfood+ status.  I'm trying to look at
this bug now, but I'm not seeing it any more in mozilla -compose, and I can't
display a mail message today because I get a seemingly infinite number of 
###!!! ASSERTION: A Box's child is constantly growing!!!!!: 'passes < 10', file
/builds/moz/mozilla/layout/xul/base/src/nsSprocketLayout.cpp, line 461
when I try to display a mail message.

Ben: are you still seeing this?
Target Milestone: M17 → M16
yes, I just sent you a mail which shows this bug with my build from Tue May 23
14:26:36 CEST 2000.
I got a message from Ben shortly before I got the bugzilla mail with his last
comment, and the message I got looked okay.  I can't seem to make this happen
with the tests I'm doing today, which I think are the same tests I was doing
last week where I WAS seeing the bug.

Here's what I'm doing today.  Let's compare our steps so I can figure out how to
reproduce what you're seeing.

Run mozilla -compose, which brings up an html compose window.  (I can't read
mail with mozilla today -- already filed a bug on that.)

In 4.x, select some quoted text from this this bug report, e.g. the following:
--
> > - It occurs also on recent nightlies, so it's *not* caused by my changes
> > for bug 31906 - puh! (I also don't know, how it could.)
gzui zui ziufi zi ui zuizui ri rzu rui ui u i

zu izui tzu itezu ezu tz jtz zt

> > - It does not happen, if I send a new, fresh mail
> > - It does happen, if I interleave quotes with comments, i.e.
__

Copy in 4.x.

Click in the content area of the mozilla compose window, and "Paste as
Quotation".

Click at the end of one of the lines, e.g. at the end of the "gzui zui" line,
hit return three times and uparrow once.

Type asdf[return]
[return]
ghjk

Do Debug->Output HTML and notice that the correct number of <br> tags seem to be
there.

Send the message as plaintext, and look at the received message and verify that
it, too, seems to have returns in the right place.

How are you testing it?  We may have different pref settings somewhere, or maybe
one of us is using flowed and the other one isn't (how do I check that?  I still
don't know how to turn flowed on/off).  What happens if you send as html?
Whiteboard: [nsdogfood+] → [nsdogfood+] Need testcase
reassigning QA to Lisa and marking bug as fixed. Lisa, if someone on your team 
can come up with a test case and can show us how to reproduce this, then we will 
reopen it, for now we consider this fixed
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
QA Contact: sujay → lchiang
Resolution: --- → FIXED
ok, I will try.
beppe, akk *has* a test case. I will retry it with a recent build, but i heavily
doubt, that this is fixed.
BTW: Sorry that I didn't care more about this bug. More tomorrow.
I just tried it with my build from May 23 and the steps in initial description
(plus "send as HTML + plain text") and the bug is still the same.
I followed Akk's steps and I get the same result like her (i.e. linebreaks are
correct). REOPENing.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
ben - are you replying to a plain text message in your initial steps?  I'm 
trying to reproduce this.
Lisa, yes. (Doesn't seem to appear for replies to HTML msgs.)
BTW: I just tried a nightly I just downloaded and it still appears.
Marking worksforme again because I still don't have a testcase.  "In mailnews,
hit reply" isn't specific enough -- I've tried that with many different
messages, and I don't see the problem.

Ben, please tell me exactly what the message needs to look like, and how to do
the reply.  Obviously you're seeing something different from what you're
seeing.  E.g. "Send yourself a plaintext message consisting of the following
four lines.  Now, in html, reply to that message ..." or something like that. 
If there's some pref that controls whether we're doing flowed or not, specify
how that should be set, too.  Otherwise, I can't test possible fixes.  I believe
you that you're seeing the bug, but if no one else can see it and you can't tell
us how to reproduce it, then we can't fix it.
Status: REOPENED → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → WORKSFORME
Okay, I have a test case that reproduces it now.  One difference we found in our
profiles was that I had 
user_pref("mail.send_plaintext_flowed", true);
in my prefs.js while he didn't; so I deleted that line (which *shouldn't* have
anything to do with this bug ...)

Then, I sent myself a plaintext message (I used html compose, then sent as
plaintext) consisting of the following:
one
two
three
When the message arrived, I clicked reply to get an html compose window, then
clicked at the end of the "one" line, then hit return three times, arrowed up
once, then typed "aaa<return><return>bbb".

At this point, I took a shortcut and did Debug->Output HTML rather than sending
the message.  The html output looked like this:

<body>akkana@netscape.com (Akkana Peck) wrote:<br>
<blockquote type="cite" cite="39341D2E.8080108@netscape.com">
<pre wrap>one</pre></blockquote>
aaa

bbb

<blockquote type="cite" cite="39341D2E.8080108@netscape.com"><pre wrap>
two
three

</pre></blockquote>

</body>

It should have had <br> tags after the aaa and bbb lines, and another one in
between those lines (and probably one before the aaa as well).
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Status: REOPENED → ASSIGNED
I've removed the "need testcase" notation from the whiteboard since it sounds 
like you have a testcase.
Can you give an estimate of the difficulty for the fix... or best of all, an ETA 
guestimate in the status whiteboard?  Folks are trying to close down M16... and 
we want the dogfood count brought under control.

Thanks in advance,

Jim (telecommuter) Roskind
Whiteboard: [nsdogfood+] Need testcase → [nsdogfood+]
I have a fix.  The problem was the code that tried to detect nested pre tags,
which was looping from the wrong place (from the current tag rather than one
above the current tag).  My fix will clean up this code by changing the boolean
mInPre to an int mPreLevel, so no looping will be needed.
Whiteboard: [nsdogfood+] → [nsdogfood+] FIX IN HAND
Checked in the fix.  Ben, let me know if this fixes the problem for you!
Status: ASSIGNED → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → FIXED
Yes, it does. Tnx.

VERIFIED on Linux with custom build from today.
Suresh - can you help me verify this on Mac and Win32?  Thanks.
QA Contact: lchiang → suresh
Verified on Win32 using 2000-07-06-09-M17 commercial build. Used the testcase 
akkana provided on 2000-05-30 13:06. I could see the <br> tags in HTML output.

Mac optimized builds does not display the console window. I need to find one who  
has Mac debug build. 
suresh - any luck with getting a Mac build w/a console?  
Verified on Mac Debug build. I'm able to see '<br>' tags in HTML output!!
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.