Skip to content

innerText should emit blank lines around <p> elements regardless of CSS display value#63654

Merged
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
cdumez:313381_innerText_p_blank_lines
Apr 28, 2026
Merged

innerText should emit blank lines around <p> elements regardless of CSS display value#63654
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
cdumez:313381_innerText_p_blank_lines

Conversation

@cdumez
Copy link
Copy Markdown
Contributor

@cdumez cdumez commented Apr 27, 2026 •
edited by webkit-early-warning-system
Loading

0a2872f

innerText should emit blank lines around <p> elements regardless of CSS display value
https://bugs.webkit.org/show_bug.cgi?id=313381

Reviewed by Anne van Kesteren.

Per the WHATWG rendered text collection steps [1], the <p> element check
(step 8) runs before the CSS display check (step 9), so <p> elements
should always get a required line break count of 2 regardless of their
computed display value. WebKit was checking the renderer's display type
first, causing <p style="display:inline-block"> to emit no newlines at all.

[1] https://html.spec.whatwg.org/multipage/dom.html#rendered-text-collection-steps

No new tests, rebaselined existing subtest. This subtest was already
passing in Chrome and Firefox.

* LayoutTests/imported/w3c/web-platform-tests/html/dom/elements/the-innertext-and-outertext-properties/getter-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/dom/elements/the-innertext-and-outertext-properties/getter-tests.js:
* Source/WebCore/editing/TextIterator.cpp:
(WebCore::shouldEmitNewlinesBeforeAndAfterNode):
(WebCore::shouldEmitNewlineAfterNode):
(WebCore::shouldEmitNewlineBeforeNode):
(WebCore::TextIterator::representNodeOffsetZero):
(WebCore::TextIterator::exitNode):
* Source/WebCore/editing/TextIterator.h:

Canonical link: https://commits.webkit.org/312169@main

32389fd

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows Apple Internal
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe   🛠 win ❌ 🛠 ios-apple
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2   🧪 win-tests ❌ 🛠 mac-apple
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe ❌ 🛠 vision-apple
  🧪 ios-wk2-wpt   🧪 api-mac-debug ✅ 🛠 gtk3-libwebrtc
  🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🛠 ios-safer-cpp ✅ 🧪 mac-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision   🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🛠 playstation
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@cdumez cdumez self-assigned this Apr 27, 2026
@webkit-early-warning-system
Copy link
Copy Markdown
Collaborator

webkit-early-warning-system commented Apr 27, 2026 •
edited
Loading

EWS run on previous version of this PR (hash ac372d4)

Details

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows Apple Internal
✅ 🧪 style ✅ 🛠 ios   🛠 mac ✅ 🛠 wpe ✅ 🛠 win ✅ 🛠 ios-apple
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2 ✅ 🧪 win-tests ✅ 🛠 mac-apple
✅ 🧪 webkitperl   🧪 ios-wk2   🧪 api-mac ✅ 🧪 api-wpe ✅ 🛠 vision-apple
✅ 🧪 ios-wk2-wpt   🧪 api-mac-debug ✅ 🛠 gtk3-libwebrtc
✅ 🧪 api-ios   🧪 mac-wk1 ✅ 🛠 gtk
✅ 🛠 ios-safer-cpp   🧪 mac-wk2   🧪 gtk-wk2
✅ 🛠 vision   🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 vision-sim   🧪 mac-wk2-stress ✅ 🛠 playstation
  🧪 vision-wk2   🧪 mac-intel-wk2
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@cdumez cdumez added the DOM For bugs specific to XML/HTML DOM elements (including parsing). label Apr 27, 2026
@cdumez cdumez force-pushed the 313381_innerText_p_blank_lines branch from ac372d4 to 231cf4f Compare April 27, 2026 04:14
@webkit-early-warning-system
Copy link
Copy Markdown
Collaborator

webkit-early-warning-system commented Apr 27, 2026 •
edited
Loading

EWS run on previous version of this PR (hash 231cf4f)

Details

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows Apple Internal
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win ✅ 🛠 ios-apple
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2 ❌ 🧪 win-tests ✅ 🛠 mac-apple
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe ✅ 🛠 vision-apple
❌ 🧪 ios-wk2-wpt ✅ 🧪 api-mac-debug ✅ 🛠 gtk3-libwebrtc
✅ 🧪 api-ios ❌ 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🛠 ios-safer-cpp ❌ 🧪 mac-wk2 ❌ 🧪 gtk-wk2
✅ 🛠 vision ❌ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🛠 playstation
✅ 🧪 vision-wk2 ❌ 🧪 mac-intel-wk2
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Apr 27, 2026
@cdumez cdumez removed the merging-blocked Applied to prevent a change from being merged label Apr 27, 2026
@cdumez cdumez force-pushed the 313381_innerText_p_blank_lines branch from 231cf4f to a603592 Compare April 27, 2026 09:02
@webkit-early-warning-system
Copy link
Copy Markdown
Collaborator

webkit-early-warning-system commented Apr 27, 2026 •
edited
Loading

EWS run on previous version of this PR (hash a603592)

Details

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows Apple Internal
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win ✅ 🛠 ios-apple
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 win-tests ✅ 🛠 mac-apple
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe ✅ 🛠 vision-apple
✅ 🧪 ios-wk2-wpt ✅ 🧪 api-mac-debug ✅ 🛠 gtk3-libwebrtc
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🛠 ios-safer-cpp ✅ 🧪 mac-wk2 ❌ 🧪 gtk-wk2
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
  🛠 🧪 merge ✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🛠 playstation
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Apr 27, 2026
@cdumez cdumez marked this pull request as ready for review April 27, 2026 12:15
@cdumez cdumez requested a review from rniwa as a code owner April 27, 2026 12:15
@cdumez cdumez removed the merging-blocked Applied to prevent a change from being merged label Apr 27, 2026
@cdumez cdumez requested review from annevk and darinadler April 27, 2026 12:15
annevk
annevk approved these changes Apr 27, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be sure, this is after we dropped display: none p elements, right?

@cdumez
Copy link
Copy Markdown
Contributor Author

cdumez commented Apr 27, 2026

To be sure, this is after we dropped display: none p elements, right?

I don't know. All I can say is it matches the spec (I provided the link / steps in the commit message) and Chromium/Firefox.

@cdumez cdumez added the merge-queue Applied to send a pull request to merge-queue label Apr 27, 2026
@annevk
Copy link
Copy Markdown
Contributor

annevk commented Apr 27, 2026

Right, it's just that step 3 in the linked specification is also a display check.

@cdumez cdumez added merging-blocked Applied to prevent a change from being merged and removed merge-queue Applied to send a pull request to merge-queue labels Apr 27, 2026
@darinadler
Copy link
Copy Markdown
Member

darinadler commented Apr 27, 2026

To be sure, this is after we dropped display: none p elements, right?

Is there a way to turn this question into a question about web platform test coverage?

@annevk
Copy link
Copy Markdown
Contributor

annevk commented Apr 27, 2026

Not quite in terms of WPT, but my ask is really if https://software.hixie.ch/utilities/js/live-dom-viewer/saved/14687 continues logging true for both cases as that's interoperable today.

@cdumez
Copy link
Copy Markdown
Contributor Author

cdumez commented Apr 27, 2026

Not quite in terms of WPT, but my ask is really if https://software.hixie.ch/utilities/js/live-dom-viewer/saved/14687 continues logging true for both cases as that's interoperable today.

I'll test. If there is an issue, then we likely lack test coverage and I will add it.

@cdumez
Copy link
Copy Markdown
Contributor Author

cdumez commented Apr 28, 2026

Not quite in terms of WPT, but my ask is really if https://software.hixie.ch/utilities/js/live-dom-viewer/saved/14687 continues logging true for both cases as that's interoperable today.

I'll test. If there is an issue, then we likely lack test coverage and I will add it.

Your test is still passing. I couldn't find test coverage for it in WPT so I'll extend the WPT test.

@cdumez cdumez removed the merging-blocked Applied to prevent a change from being merged label Apr 28, 2026
@cdumez cdumez force-pushed the 313381_innerText_p_blank_lines branch from a603592 to 32389fd Compare April 28, 2026 00:45
@webkit-early-warning-system
Copy link
Copy Markdown
Collaborator

webkit-early-warning-system commented Apr 28, 2026 •
edited
Loading

EWS run on current version of this PR (hash 32389fd)

Details

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows Apple Internal
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe   🛠 win ❌ 🛠 ios-apple
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2   🧪 win-tests ❌ 🛠 mac-apple
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe ❌ 🛠 vision-apple
  🧪 ios-wk2-wpt   🧪 api-mac-debug ✅ 🛠 gtk3-libwebrtc
  🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🛠 ios-safer-cpp ✅ 🧪 mac-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision   🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🛠 playstation
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@cdumez cdumez added the merge-queue Applied to send a pull request to merge-queue label Apr 28, 2026
@cdumez
innerText should emit blank lines around <p> elements regardless of C…
0a2872f 
…SS display value

https://bugs.webkit.org/show_bug.cgi?id=313381

Reviewed by Anne van Kesteren.

Per the WHATWG rendered text collection steps [1], the <p> element check
(step 8) runs before the CSS display check (step 9), so <p> elements
should always get a required line break count of 2 regardless of their
computed display value. WebKit was checking the renderer's display type
first, causing <p style="display:inline-block"> to emit no newlines at all.

[1] https://html.spec.whatwg.org/multipage/dom.html#rendered-text-collection-steps

No new tests, rebaselined existing subtest. This subtest was already
passing in Chrome and Firefox.

* LayoutTests/imported/w3c/web-platform-tests/html/dom/elements/the-innertext-and-outertext-properties/getter-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/dom/elements/the-innertext-and-outertext-properties/getter-tests.js:
* Source/WebCore/editing/TextIterator.cpp:
(WebCore::shouldEmitNewlinesBeforeAndAfterNode):
(WebCore::shouldEmitNewlineAfterNode):
(WebCore::shouldEmitNewlineBeforeNode):
(WebCore::TextIterator::representNodeOffsetZero):
(WebCore::TextIterator::exitNode):
* Source/WebCore/editing/TextIterator.h:

Canonical link: https://commits.webkit.org/312169@main
@webkit-commit-queue webkit-commit-queue force-pushed the 313381_innerText_p_blank_lines branch from 32389fd to 0a2872f Compare April 28, 2026 04:19
@webkit-commit-queue
Copy link
Copy Markdown
Collaborator

webkit-commit-queue commented Apr 28, 2026

Committed 312169@main (0a2872f): https://commits.webkit.org/312169@main

Reviewed commits have been landed. Closing PR #63654 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 0a2872f into WebKit:main Apr 28, 2026
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Apr 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@annevk annevk annevk approved these changes

@rniwa rniwa Awaiting requested review from rniwa rniwa is a code owner

@darinadler darinadler Awaiting requested review from darinadler

Assignees

@cdumez cdumez

Labels

DOM For bugs specific to XML/HTML DOM elements (including parsing).

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@cdumez @webkit-early-warning-system @annevk @darinadler @webkit-commit-queue @webkit-ews-buildbot