Skip to content

Commit

Permalink
Bug 1864820 - Fix plaintext serializer space-stuffing stuff to avoid …
Browse files Browse the repository at this point in the history 
…breaking over and over in the inserted space. r=jfkthame

Treat it as an extra indentation level on the line.

Differential Revision: https://phabricator.services.mozilla.com/D193641
  • Loading branch information
@emilio
emilio committed Nov 15, 2023
1 parent 67fc31c commit 02f58e3
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 54 deletions.
30 changes: 30 additions & 0 deletions dom/base/test/gtest/TestPlainTextSerializer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,36 @@ TEST(PlainTextSerializer, ASCIIWithFlowedDelSp)
<< "Wrong HTML to ASCII text serialization with format=flowed; delsp=yes";
}

TEST(PlainTextSerializer, Bug1864820)
{
nsString test(
uR"#(
<html><body>
&gt;&nbsp;&nbsp;label=master&amp;label=experimental&amp;product=chrome&amp;product=firefox&amp;product=safari&amp;aligned&amp;view=interop&amp;q=label%3Ainterop-2023-property
<blockquote>
&gt;&nbsp;&nbsp;label=master&amp;label=experimental&amp;product=chrome&amp;product=firefox&amp;product=safari&amp;aligned&amp;view=interop&amp;q=label%3Ainterop-2023-property
</blockquote>
</body></html>
)#");

ConvertBufToPlainText(test,
nsIDocumentEncoder::OutputFormatted |
nsIDocumentEncoder::OutputPersistNBSP |
nsIDocumentEncoder::OutputFormatFlowed,
kDefaultWrapColumn);

nsString result(
uR"#(
>  label=master&label=experimental&product=chrome&product=firefox&product=safari&aligned&view=interop&q=label%3Ainterop-2023-property
>  label=master&label=experimental&product=chrome&product=firefox&product=safari&aligned&view=interop&q=label%3Ainterop-2023-property
)#");
result.Trim(" \n");
test.Trim(" \n");
ASSERT_TRUE(test.Equals(result))
<< "Shouldn't hang with format=flowed: " << NS_ConvertUTF16toUTF8(test).get();
}

// Test for CJK with format=flowed; delsp=yes
TEST(PlainTextSerializer, CJKWithFlowedDelSp)
{
Expand Down
94 changes: 43 additions & 51 deletions dom/serializers/nsPlainTextSerializer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1227,16 +1227,14 @@ void nsPlainTextSerializer::MaybeWrapAndOutputCompleteLines() {
return;
}

const uint32_t prefixwidth = mCurrentLine.DeterminePrefixWidth();

// Yes, wrap!
// The "+4" is to avoid wrap lines that only would be a couple
// of letters too long. We give this bonus only if the
// wrapcolumn is more than 20.
const uint32_t wrapColumn = mSettings.GetWrapColumn();
uint32_t bonuswidth = (wrapColumn > 20) ? 4 : 0;

while (!mCurrentLine.mContent.IsEmpty()) {
const uint32_t prefixwidth = mCurrentLine.DeterminePrefixWidth();
// The width of the line as it will appear on the screen (approx.).
const uint32_t currentLineContentWidth =
GetUnicharStringWidth(mCurrentLine.mContent);
Expand All @@ -1248,41 +1246,34 @@ void nsPlainTextSerializer::MaybeWrapAndOutputCompleteLines() {
mCurrentLine.FindWrapIndexForContent(wrapColumn, mUseLineBreaker);

const int32_t contentLength = mCurrentLine.mContent.Length();
if ((goodSpace < contentLength) && (goodSpace > 0)) {
// Found a place to break

// -1 (trim a char at the break position)
// only if the line break was a space.
nsAutoString restOfContent;
if (nsCRT::IsAsciiSpace(mCurrentLine.mContent.CharAt(goodSpace))) {
mCurrentLine.mContent.Right(restOfContent,
contentLength - goodSpace - 1);
} else {
mCurrentLine.mContent.Right(restOfContent, contentLength - goodSpace);
}
// if breaker was U+0020, it has to consider for delsp=yes support
const bool breakBySpace = mCurrentLine.mContent.CharAt(goodSpace) == ' ';
mCurrentLine.mContent.Truncate(goodSpace);
EndLine(true, breakBySpace);
mCurrentLine.mContent.Truncate();
// Space stuff new line?
if (mSettings.HasFlag(nsIDocumentEncoder::OutputFormatFlowed)) {
if (!restOfContent.IsEmpty() && IsSpaceStuffable(restOfContent.get()) &&
mCurrentLine.mCiteQuoteLevel ==
0 // We space-stuff quoted lines anyway
) {
// Space stuffing a la RFC 2646 (format=flowed).
mCurrentLine.mContent.Append(char16_t(' '));
// XXX doesn't seem to work correctly for ' '
}
}
mCurrentLine.mContent.Append(restOfContent);
mEmptyLines = -1;
} else {
// Nothing to do. Hopefully we get more data later
// to use for a place to break line
if (goodSpace <= 0 || goodSpace >= contentLength) {
// Nothing to do. Hopefully we get more data later to use for a place to
// break line.
break;
}
// Found a place to break
// -1 (trim a char at the break position) only if the line break was a
// space.
nsAutoString restOfContent;
if (nsCRT::IsAsciiSpace(mCurrentLine.mContent.CharAt(goodSpace))) {
mCurrentLine.mContent.Right(restOfContent, contentLength - goodSpace - 1);
} else {
mCurrentLine.mContent.Right(restOfContent, contentLength - goodSpace);
}
// if breaker was U+0020, it has to consider for delsp=yes support
const bool breakBySpace = mCurrentLine.mContent.CharAt(goodSpace) == ' ';
mCurrentLine.mContent.Truncate(goodSpace);
EndLine(true, breakBySpace);
mCurrentLine.mContent.Truncate();
// Space stuffing a la RFC 2646 (format=flowed)
if (mSettings.HasFlag(nsIDocumentEncoder::OutputFormatFlowed)) {
mCurrentLine.mSpaceStuffed = !restOfContent.IsEmpty() &&
IsSpaceStuffable(restOfContent.get()) &&
// We space-stuff quoted lines anyway
mCurrentLine.mCiteQuoteLevel == 0;
}
mCurrentLine.mContent.Append(restOfContent);
mEmptyLines = -1;
}
}

Expand All @@ -1302,13 +1293,10 @@ void nsPlainTextSerializer::AddToLine(const char16_t* aLineFragment,
}

if (mSettings.HasFlag(nsIDocumentEncoder::OutputFormatFlowed)) {
if (IsSpaceStuffable(aLineFragment) &&
mCurrentLine.mCiteQuoteLevel ==
0 // We space-stuff quoted lines anyway
) {
// Space stuffing a la RFC 2646 (format=flowed).
mCurrentLine.mContent.Append(char16_t(' '));
}
// Space stuffing a la RFC 2646 (format=flowed).
// We space-stuff quoted lines anyway
mCurrentLine.mSpaceStuffed =
IsSpaceStuffable(aLineFragment) && mCurrentLine.mCiteQuoteLevel == 0;
}
mEmptyLines = -1;
}
Expand Down Expand Up @@ -1354,7 +1342,7 @@ void nsPlainTextSerializer::EndLine(bool aSoftLineBreak, bool aBreakBySpace) {

if (aSoftLineBreak &&
mSettings.HasFlag(nsIDocumentEncoder::OutputFormatFlowed) &&
(mCurrentLine.mIndentation.mLength == 0)) {
!mCurrentLine.mIndentation.mLength) {
// Add the soft part of the soft linebreak (RFC 2646 4.1)
// We only do this when there is no indentation since format=flowed
// lines and indentation doesn't work well together.
Expand Down Expand Up @@ -1420,11 +1408,16 @@ void nsPlainTextSerializer::CurrentLine::CreateQuotesAndIndent(

// Indent if necessary
int32_t indentwidth = mIndentation.mLength - mIndentation.mHeader.Length();
if (indentwidth > 0 && HasContentOrIndentationHeader()
// Don't make empty lines look flowed
) {
if (mSpaceStuffed) {
indentwidth += 1;
}

// Don't make empty lines look flowed
if (indentwidth > 0 && HasContentOrIndentationHeader()) {
nsAutoString spaces;
for (int i = 0; i < indentwidth; ++i) spaces.Append(char16_t(' '));
for (int i = 0; i < indentwidth; ++i) {
spaces.Append(char16_t(' '));
}
aResult += spaces;
}

Expand Down Expand Up @@ -1515,9 +1508,8 @@ void nsPlainTextSerializer::ConvertToLinesAndOutput(const nsAString& aString) {
!IsQuotedLine(stringpart) && !IsSignatureSeparator(stringpart)) {
stringpart.Trim(" ", false, true, true);
}
if (IsSpaceStuffable(stringpart.get()) && !IsQuotedLine(stringpart)) {
mCurrentLine.mContent.Append(char16_t(' '));
}
mCurrentLine.mSpaceStuffed =
IsSpaceStuffable(stringpart.get()) && !IsQuotedLine(stringpart);
}
mCurrentLine.mContent.Append(stringpart);

Expand Down
8 changes: 5 additions & 3 deletions dom/serializers/nsPlainTextSerializer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@
#ifndef nsPlainTextSerializer_h__
#define nsPlainTextSerializer_h__

#include "mozilla/Attributes.h"
#include "mozilla/Maybe.h"
#include "nsCOMPtr.h"
#include "nsAtom.h"
#include "nsCycleCollectionParticipant.h"
#include "nsIContentSerializer.h"
Expand Down Expand Up @@ -244,14 +242,18 @@ class nsPlainTextSerializer final : public nsIContentSerializer {
uint32_t DeterminePrefixWidth() const {
// XXX: Should calculate prefixwidth with GetUnicharStringWidth
return (mCiteQuoteLevel > 0 ? mCiteQuoteLevel + 1 : 0) +
mIndentation.mLength;
mIndentation.mLength + uint32_t(mSpaceStuffed);
}

Indentation mIndentation;

// The number of '>' characters.
int32_t mCiteQuoteLevel = 0;

// Whether this line is getting space-stuffed, see
// https://datatracker.ietf.org/doc/html/rfc2646#section-4.4
bool mSpaceStuffed = false;

// Excludes indentation and quotes.
nsString mContent;
};
Expand Down

0 comments on commit 02f58e3

Please sign in to comment.