17871 - nsHTMLToTXTSinkStream: last line of quote has no "> "

Status: VERIFIED FIXED

(Core :: DOM: Serializers, defect, P3)

Description

[Ben Bucksch (:BenB)]

Reproduce:
Debug|Composer with test page
Output HTML

Actual Result:
> Reruns
>
> Reruns are about as much fun,
> as your dad taking all your mun,
> and giving it to a nun,
as a contribution.

Expected Result:
> Reruns
>
> Reruns are about as much fun,
> as your dad taking all your mun,
> and giving it to a nun,
> as a contribution.

Output HTML gives:
<blockquote class="poem" type="cite">
<h4>Reruns</h4>
Reruns are about as much fun,<br>
as your dad taking all your mun,<br>
and giving it to a nun,<br>
as a contribution.
</blockquote>

Output XIF gives:
<container isa="blockquote"><attr name="class" value="poem"/>
<attr name="type" value="cite"/>
<content>
</content>
<container isa="h4"><content>Reruns</content>
</container><!--h4-->
<content>
Reruns are about as much fun,</content>
<leaf isa="br">
</leaf><!--br-->
<content>
as your dad taking all your mun,</content>
<leaf isa="br">
</leaf><!--br-->
<content>
and giving it to a nun,</content>
<leaf isa="br">
</leaf><!--br-->
<content>
as a contribution.
</content>
</container><!--blockquote-->

Comment 1

[Ben Bucksch (:BenB)]

Correction:
Reproduce:
Debug|Composer with test page
Output Text

Comment 2

[Akkana Peck]

Yes, agreed.  The quoting code gets confused if the text being quoted doesn't
end with a <br> or similar.

Comment 3

[Akkana Peck]

The "mailquote" automated output test has been changed to include this case (and
won't pass the auto test until this bug is fixed).

Comment 4

[Daniel Bratell]

This special case was cauesed by the quotelevel count being decreased before the
line is emitted. Could be fixed by a fast hack, but maybe there is a smarter
solution. Will think about it.
By the way, the quote is missing from Flushline too.

Patch:
diff -u -r3.34 nsHTMLToTXTSinkStream.cpp
--- nsHTMLToTXTSinkStream.cpp   1999/11/03 02:44:36     3.34
+++ nsHTMLToTXTSinkStream.cpp   1999/11/03 18:19:47
@@ -757,8 +757,14 @@
 void
 nsHTMLToTXTSinkStream::FlushLine()
 {
-  WriteSimple(mCurrentLine);
-  mCurrentLine.SetString("");
+  if(mCurrentLine.Length()>0) {
+    if(0 == mColPos)
+      WriteQuotesAndIndent();
+
+    WriteSimple(mCurrentLine);
+    mColPos += mCurrentLine.Length();
+    mCurrentLine.SetString("");
+  }
 }

Comment 5

[Daniel Bratell]

Now I've thought about it. A good patch that solves the problem is:

diff -u -r3.34 nsHTMLToTXTSinkStream.cpp
--- nsHTMLToTXTSinkStream.cpp   1999/11/03 02:44:36     3.34
+++ nsHTMLToTXTSinkStream.cpp   1999/11/03 23:07:12
@@ -549,10 +549,13 @@
   if (mTagStackIndex > 0)
     --mTagStackIndex;

-  if (type == eHTMLTag_ol)
+  if (type == eHTMLTag_ol) {
+    FlushLine(); // Doing this after decreasing OLStackIndex would be wrong.
     --mOLStackIndex;
+  }
   else if (type == eHTMLTag_blockquote)
   {
+    FlushLine(); // Doing this after decreasing quotelevel would be wrong.
     if (mCiteQuoteLevel>0)
       mCiteQuoteLevel--;
     else if(mIndent >= gTabSize)
@@ -757,8 +760,14 @@
 void
 nsHTMLToTXTSinkStream::FlushLine()
 {
-  WriteSimple(mCurrentLine);
-  mCurrentLine.SetString("");
+  if(mCurrentLine.Length()>0) {
+    if(0 == mColPos)
+      WriteQuotesAndIndent();
+
+    WriteSimple(mCurrentLine);
+    mColPos += mCurrentLine.Length();
+    mCurrentLine.SetString("");
+  }
 }

Comment 6

[Daniel Bratell]

Attachment: The patch (as in the text)

Comment 7

[Akkana Peck]

Daniel's patch has been checked in; this problem should be fixed now.

Comment 8

[sujay]

verified in 11/29 build.

Comment 9

[leger]

Bulk move of all "Output" component bugs to new "DOM to Test Conversion" 
component.  Output will be deleted as a component.

Comment 10

[Pulsebot]

Pushed by ffxbld-merge:
https://hg.mozilla.org/releases/mozilla-beta/rev/3ab90829f8da
[fenix] Closes https://github.com/mozilla-mobile/fenix/issues/17871 - use viewLifecycleOwner for TopSitesFeature owner on HomeFragment (https://github.com/mozilla-mobile/fenix/pull/17873)