Skip to content

LibWeb: Decouple layout from caret position and editable state - #10962

Merged
kalenikaliaksandr merged 4 commits into
LadybirdBrowser:masterfrom
kalenikaliaksandr:decouple-layout-from-editing-state
Aug 1, 2026
Merged

LibWeb: Decouple layout from caret position and editable state#10962
kalenikaliaksandr merged 4 commits into
LadybirdBrowser:masterfrom
kalenikaliaksandr:decouple-layout-from-editing-state

Conversation

@kalenikaliaksandr

Copy link
Copy Markdown
Member

Line layout queried the DOM mid-layout through two FFI callbacks: whether the
caret sits on a node (to skip trailing-whitespace trimming while typing) and
whether an empty text node is in an editable context (to synthesize the
zero-width fragment that anchors the caret). The first is deleted outright —
its job is now done at the right layers, by giving text input inner editors
white-space: pre like other engines and relying on the editing layer's
existing NBSP canonicalization — which also makes geometry independent of
caret movement instead of stale until an unrelated relayout. The second
becomes a NodeFlag stamped at layout-node creation and refreshed by the
editable-subtree walk, which now also triggers a relayout when a stamp
actually flips, fixing a pre-existing bug where an element made editable at
runtime showed no caret and kept zero height. Covered by new regression
tests for caret advance past typed trailing spaces and for contenteditable
toggling.

The inner editor element of text-like <input> controls used
white-space: nowrap, which collapses interior whitespace runs and trims
trailing spaces from the rendered value. Input values are plain text
and other engines render them verbatim by styling the inner editor with
white-space: pre; with nowrap, <input value="a   b   "> lost the extra
spaces, and a trailing space typed by the user only stayed visible
because line layout special-cases trailing-whitespace trimming while
the caret sits on the node.

Switch the inner editor to white-space: pre so input values render
verbatim. This also lets the caret advance past freshly typed trailing
spaces without any help from layout, clearing the way for removing the
cursor-position dependency from line layout.
Trailing-whitespace trimming skipped its work whenever the document
cursor sat on the fragment's node, so that a trailing space typed into
a text control stayed visible. Nothing invalidates layout when the
caret moves, so this made geometry depend on whenever layout happened
to run: the preserved space would linger after the caret left the node
(or the element lost focus) until some unrelated relayout trimmed it,
and through trailing_whitespace_inline_size() the caret position could
even shift line breaking and float placement decisions.

The behaviors this special case provided are all covered at the right
layers now: text input inner editors preserve whitespace outright
(white-space: pre), textarea editors inherit pre-wrap, and typing into
an editing host canonicalizes a would-collapse space into a
non-breaking space per the editing spec's whitespace canonicalization.
A caret placed inside genuinely collapsed trailing whitespace renders
clamped to the line end, since caret matching already accepts offsets
into a fragment's trimmed trailing whitespace.

Remove the document_cursor_is_on_node FFI callback and the trimming
special case; layout geometry no longer depends on caret position, so
the repaint-only invalidation done on caret moves is now sufficient by
construction. Adds a regression test that the caret keeps advancing
while trailing spaces are typed into a text input and an editing host.
Inline layout called back into the DOM mid-layout to ask whether an
empty text node sits in an editable context (a text control's shadow
tree or under an editing host), deciding whether to synthesize the
zero-width fragment that keeps the line box alive with real font
metrics so the caret has an anchor and the control keeps its baseline.

That fact derives purely from DOM structure, so a query at layout time
is the wrong shape for it: compute it when the layout node is created
and stamp it as ProducesLineBoxFragmentWhenEmpty in NodeData, the same
pattern already used for IsEditingHost. The existing editable-subtree
walk refreshes the stamp when contenteditable or designMode change
without a layout tree rebuild, and the layout engine now reads the
flag with no knowledge of editing concepts. This removes the last
DOM-querying FFI callback from inline layout, and the existing
empty-editable layout expectations are unchanged by the conversion.

The stamp intentionally drops the old is_mutable() gate: empty
readonly and disabled text controls now produce the same zero-width
fragment as mutable ones, so a focused readonly control shows its
caret and empty controls keep the same baseline regardless of
mutability, matching other engines.
Toggling contenteditable (or designMode) re-stamped the editing-host
flag into layout NodeData but only requested a repaint, and nothing
ever dirtied layout. The geometry that depends on those stamps -- the
minimum block size of an empty editing host and the zero-width
fragment of an empty editable text node -- would keep its stale shape
until some unrelated relayout happened to run, so a div made editable
at runtime showed no caret and kept zero height until then.

Detect in the editable-subtree walk whether a node's stamped
editing-host status or a text node's empty-text fragment behavior
actually changed, and request a layout update for that node when it
did. Adds a regression test that an empty text node in a freshly
editable host immediately anchors a caret with line height, and that
the host's minimum block size appears and collapses as contenteditable
is toggled.
@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4da7cb0e-c1b0-4b9b-b5eb-2ce792a0d69e

📥 Commits

Reviewing files that changed from the base of the PR and between 5aa0645 and b086fc8.

📒 Files selected for processing (17)
  • Libraries/LibWeb/DOM/Node.cpp
  • Libraries/LibWeb/DOM/Node.h
  • Libraries/LibWeb/HTML/HTMLInputElement.cpp
  • Libraries/LibWeb/Layout/LayoutRustBridge.cpp
  • Libraries/LibWeb/Layout/Node.h
  • Libraries/LibWeb/Layout/TextNode.cpp
  • Libraries/LibWeb/Layout/TextNode.h
  • Libraries/LibWeb/Rust/src/layout/formatting_context.rs
  • Libraries/LibWeb/Rust/src/layout/inline_formatting_context.rs
  • Libraries/LibWeb/Rust/src/layout/inline_level_iterator.rs
  • Libraries/LibWeb/Rust/src/layout/layout_state.rs
  • Libraries/LibWeb/Rust/src/layout/line_box.rs
  • Libraries/LibWeb/Rust/src/layout/node_data.rs
  • Tests/LibWeb/Text/expected/Editing/caret-in-empty-text-node-after-contenteditable-toggle.txt
  • Tests/LibWeb/Text/expected/input-caret-advances-past-typed-trailing-space.txt
  • Tests/LibWeb/Text/input/Editing/caret-in-empty-text-node-after-contenteditable-toggle.html
  • Tests/LibWeb/Text/input/input-caret-advances-past-typed-trailing-space.html
💤 Files with no reviewable changes (4)
  • Libraries/LibWeb/Rust/src/layout/line_box.rs
  • Libraries/LibWeb/Rust/src/layout/inline_formatting_context.rs
  • Libraries/LibWeb/Rust/src/layout/formatting_context.rs
  • Libraries/LibWeb/Layout/LayoutRustBridge.cpp

📝 Walkthrough

Walkthrough

The change propagates editable-state updates into layout flags, synthesizes line-box fragments for qualifying empty text nodes, removes obsolete Rust callbacks, preserves input whitespace, and adds caret regression tests.

Changes

Editable text layout

Layer / File(s) Summary
Editable state and layout flags
Libraries/LibWeb/DOM/Node.*, Libraries/LibWeb/Layout/Node.h, Libraries/LibWeb/Layout/TextNode.*, Libraries/LibWeb/Rust/src/layout/node_data.rs, Libraries/LibWeb/Rust/src/layout/layout_state.rs
Editable-state recomputation now invalidates layout when editing-host state changes. TextNode computes and updates the ProducesLineBoxFragmentWhenEmpty flag for editing hosts and form-control shadow roots.
Rust inline text processing
Libraries/LibWeb/Layout/LayoutRustBridge.cpp, Libraries/LibWeb/Rust/src/layout/inline_formatting_context.rs, Libraries/LibWeb/Rust/src/layout/inline_level_iterator.rs
Empty text nodes with the new layout fact produce zero-length chunks. Obsolete editable-text and cursor-position callbacks are removed.
Input styling and caret regressions
Libraries/LibWeb/HTML/HTMLInputElement.cpp, Tests/LibWeb/Text/input/..., Tests/LibWeb/Text/expected/...
Input shadow text uses white-space: pre. Tests cover editable empty-text layout and caret movement past trailing spaces.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant DOMNode
  participant TextNode
  participant NodeFacts
  participant InlineLevelIterator
  DOMNode->>TextNode: update empty line-box flag
  TextNode->>NodeFacts: publish ProducesLineBoxFragmentWhenEmpty
  InlineLevelIterator->>NodeFacts: read empty line-box fact
  NodeFacts-->>InlineLevelIterator: return flag state
  InlineLevelIterator->>InlineLevelIterator: synthesize zero-length text chunk
Loading

Possibly related PRs

Suggested reviewers: awesomekling

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The description directly explains the layout, editability, whitespace, FFI, and regression-test changes in the pull request.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@kalenikaliaksandr
kalenikaliaksandr enabled auto-merge (rebase) August 1, 2026 23:12
@kalenikaliaksandr
kalenikaliaksandr merged commit 209d206 into LadybirdBrowser:master Aug 1, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant