From 16d134b89d0411a29e3ed137d3cd74f8a4ca4ff9 Mon Sep 17 00:00:00 2001 From: Jonathan Kew Date: Mon, 11 Oct 2021 12:20:41 +0000 Subject: [PATCH] Bug 1734590 - Eliminate use of LineBreaker::DeprecatedNext in nsXMLContentSerializer.cpp. r=TYLin The behavior change of Next() vs DeprecatedNext() here is OK, because if DeprecatedNext() failed to find a break position then we'd end up reaching the "simple fallback logic" below, which will just advance to the next space or end-of-text anyhow. The only case, then, where this results in a change of behavior would be if there's a space that the line-breaker does *not* consider a valid break, so that instead it advances to end-of-text, where previously we'd have reached the fallback code and used the (invalid) space as a break. So (a) this is a really obscure edge-case, and (b) the new behavior would be more correct anyhow. Differential Revision: https://phabricator.services.mozilla.com/D127805 --- dom/serializers/nsXMLContentSerializer.cpp | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/dom/serializers/nsXMLContentSerializer.cpp b/dom/serializers/nsXMLContentSerializer.cpp index 73193a137b61..e86541fdf9f1 100644 --- a/dom/serializers/nsXMLContentSerializer.cpp +++ b/dom/serializers/nsXMLContentSerializer.cpp @@ -1545,10 +1545,15 @@ bool nsXMLContentSerializer::AppendWrapped_NonWhitespaceSequence( if (wrapPosition != NS_LINEBREAKER_NEED_MORE_TEXT) { foundWrapPosition = true; } else { - wrapPosition = lineBreaker->DeprecatedNext(aSequenceStart, - (aEnd - aSequenceStart), - (aPos - aSequenceStart)); - if (wrapPosition != NS_LINEBREAKER_NEED_MORE_TEXT) { + wrapPosition = lineBreaker->Next(aSequenceStart, + (aEnd - aSequenceStart), + (aPos - aSequenceStart)); + MOZ_ASSERT(wrapPosition != NS_LINEBREAKER_NEED_MORE_TEXT, + "Next() always treats end-of-text as a break"); + // If the line-breaker returned end-of-text, we don't know that it + // is actually a good wrap position, so ignore it and continue to + // use the fallback code below. + if (wrapPosition < aEnd - aSequenceStart) { foundWrapPosition = true; } } @@ -1574,6 +1579,14 @@ bool nsXMLContentSerializer::AppendWrapped_NonWhitespaceSequence( // go forward up to the next whitespace position, // in the worst case this will be all the rest of the data + // XXX(jfkthame) Should we (conditionally) output indentation here? + // It makes for tidier-looking formatted output, at the cost of + // exceeding the target width by a greater amount on such lines. + // if (!mColPos && mDoFormat) { + // NS_ENSURE_TRUE(AppendIndentation(aOutputStr), false); + // mAddSpace = false; + // } + // we update the mColPos variable with the length of // the part already parsed. mColPos += length;