1927709 - GetRenderedText with white-space:pre-line does not properly respect all newlines

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Jonathan Kew [:jfkthame]]

(See https://github.com/web-platform-tests/wpt/pull/48678#issuecomment-2438706928.)

The recent fix for nsTextFrame::GetRenderedText in bug 1923829 (and the associated WPT test) was not quite correct, in that it fails to preserve blank lines in the white-space: pre-line content. The second assertion in https://wpt.live/html/dom/elements/the-innertext-and-outertext-properties/innertext-whitespace-pre-line.html is therefore currently wrong.

Given

<div id="c" style="white-space: pre-line">
 one
  two
    <!-- comment -->
   three
    four
</div>

we should expect c.innerText to return "\none\ntwo\n\nthree\nfour\n", preserving the initial newline and the blank line in the middle. This matches Chrome's result, and corresponds to

the element's text content "as rendered"

which is what the spec calls for.

The problem occurs because when the line is empty, we fail to set trimmedSignificantNewline here; and even if we had set that flag, we then return early from here so we'd fail to restore the required newline at the end of the method.

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1927709 - Properly preserve newlines in GetRenderedText for textframes with white-space:pre-line. r=#layout

This makes innerText better reflect the actual rendered text, and matches the result
seen in Blink for the testcase.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d9d099719789
Properly preserve newlines in GetRenderedText for textframes with white-space:pre-line. r=jwatt

Comment 3

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/48858 for changes under testing/web-platform/tests

Comment 4

[Atila Butkovits]

Backed out for causing failures at browser_textleafpoint.js.

Backout link: https://hg.mozilla.org/integration/autoland/rev/4e464842e53849beaeb1667ce3afc2d10860841e

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=fChJwQGAR4qs4Gp6oHFYjA.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=d9d099719789f8be169b20bfdc958b072a4c28ca

Failure log: https://treeherder.mozilla.org/logviewer?job_id=480311051&repo=autoland&lineNumber=5969

Comment 5

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 6

[Jonathan Kew [:jfkthame]]

Ah, I see the issue -- accessibility code calls GetRenderedText with TrailingWhitespace::NoTrim, in which case we don't need the "fix" here because the trailing newline isn't getting trimmed. So we need to make the change conditional on the TrailingWhitespace option, otherwise we end up adding unwanted extra newlines to the a11y TextLeaf nodes.

I'll run the fixed version through tryserver again before re-landing...

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3f4f1d264c3a
Properly preserve newlines in GetRenderedText for textframes with white-space:pre-line. r=jwatt

Comment 8

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/3f4f1d264c3a

Comment 9

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot