LibWeb+Compositor: Support horizontal scrolling in RTL scroll containers - #11023
Open
tcl3 wants to merge 6 commits into
Open
LibWeb+Compositor: Support horizontal scrolling in RTL scroll containers#11023tcl3 wants to merge 6 commits into
tcl3 wants to merge 6 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (44)
🚧 Files skipped from review as they are similar to previous changes (42)
📝 WalkthroughWalkthroughScroll bounds now support negative minimum offsets and directional overflow. Possibly related PRs
Mergeability Score: ⚪ Minimal · up to The PR changes RTL horizontal scrolling behavior and includes related tests; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Libraries/LibWeb/HTML/LocalNavigable.cpp`:
- Around line 3820-3826: The programmatic viewport scrolling path in
perform_a_scroll_of_the_viewport() still clamps offsets against zero instead of
the paintable’s actual scroll range. Replace the hardcoded axis bounds with
Paintable::minimum_scroll_offset() and Paintable::maximum_scroll_offset(),
preserving shared bounds for window.scrollTo(), smooth scrolling, and viewport
wheel fallback, including negative RTL offsets.
In `@Tests/LibWeb/Text/expected/wpt-import/css/cssom-view/scrollLeftTop.txt`:
- Around line 10-12: Fix the CSSOM View scroll offset handling for vertical
writing modes and RTL direction so scrollLeft/scrollTop setters and readers
clamp correctly across negative and positive scroll-area bounds, including
vertical-lr/RTL and both vertical-rl cases. Validate the corrected values
against the expected negative offsets, then remove the corresponding Fail
entries from scrollLeftTop.txt only once all cases pass.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e3aaa565-50ec-4dc7-a9c6-6294267cffff
📒 Files selected for processing (41)
Libraries/LibWeb/Compositor/AsyncScrollTree.cppLibraries/LibWeb/Compositor/AsyncScrollingState.cppLibraries/LibWeb/Compositor/AsyncScrollingState.hLibraries/LibWeb/HTML/LocalNavigable.cppLibraries/LibWeb/HTML/Window.cppLibraries/LibWeb/Layout/ScrollableOverflow.cppLibraries/LibWeb/Layout/ScrollableOverflow.hLibraries/LibWeb/Painting/DisplayListCommand.cppLibraries/LibWeb/Painting/DisplayListCommand.hLibraries/LibWeb/Painting/Paintable.cppLibraries/LibWeb/Painting/Paintable.hLibraries/LibWeb/Painting/Scrollbar.cppLibraries/LibWeb/Rust/src/layout/block_formatting_context.rsLibraries/LibWeb/Rust/src/layout/line_builder.rsServices/Compositor/ViewportScrollbarController.cppTests/LibWeb/Layout/expected/block-and-inline/rtl-overflowing-line-inline-position.txtTests/LibWeb/Layout/input/block-and-inline/rtl-overflowing-line-inline-position.htmlTests/LibWeb/Ref/input/abspos-after-rtl-inline-static-position.htmlTests/LibWeb/Text/expected/async-scrolling/blocking-wheel-event-regions.txtTests/LibWeb/Text/expected/async-scrolling/blocking-wheel-listener-invalidation.txtTests/LibWeb/Text/expected/async-scrolling/nested-navigable-wheel-admission.txtTests/LibWeb/Text/expected/async-scrolling/nested-scroller-keeps-sync-wheel.txtTests/LibWeb/Text/expected/async-scrolling/scroll-node-metadata-cache-replay.txtTests/LibWeb/Text/expected/async-scrolling/scroll-tree-parent-nodes.txtTests/LibWeb/Text/expected/async-scrolling/scroll-tree-shape.txtTests/LibWeb/Text/expected/async-scrolling/sticky-areas.txtTests/LibWeb/Text/expected/async-scrolling/viewport-wheel-with-nested-scroller.txtTests/LibWeb/Text/expected/async-scrolling/wheel-scroll-admission.txtTests/LibWeb/Text/expected/display_list/background-attachment-fixed.txtTests/LibWeb/Text/expected/display_list/nested-overflow-containers.txtTests/LibWeb/Text/expected/display_list/scrollable-in-fixed.txtTests/LibWeb/Text/expected/display_list/sibling-scrollables-in-fixed.txtTests/LibWeb/Text/expected/display_list/svg-foreignobject-scroller-after-relayout.txtTests/LibWeb/Text/expected/input-scrolls-caret-width-into-view.txtTests/LibWeb/Text/expected/scrollable-overflow-webkit-box-wrap-reverse.txtTests/LibWeb/Text/expected/wpt-import/css/css-overflow/overflow-rtl-scroll-left.txtTests/LibWeb/Text/expected/wpt-import/css/cssom-view/scrollLeftTop.txtTests/LibWeb/Text/input/input-scrolls-caret-width-into-view.htmlTests/LibWeb/Text/input/scrollable-overflow-webkit-box-wrap-reverse.htmlTests/LibWeb/Text/input/wpt-import/css/css-overflow/overflow-rtl-scroll-left.htmlTests/LibWeb/Text/input/wpt-import/css/cssom-view/scrollLeftTop.html
tcl3
changed the title
tcl3
force-pushed
the
libweb_rtl_horizontal_scrolling
branch
from
August 6, 2026 15:12
cce3071 to
8273597
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Libraries/LibWeb/DOM/Document.cpp`:
- Around line 1898-1903: The body-to-viewport propagation guard in Document.cpp
currently checks only ComputedValues::contain(); update
propagation_is_disabled_by_containment to use the engine’s active-containment
predicate for both root_element and body_element, and apply the same predicate
to related overflow propagation checks. Add regression coverage for
content-visibility: auto on html and body, preserving propagation only when
containment is inactive.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c1c62ddb-1f86-4d24-b4f4-cce0b4c5eadc
📒 Files selected for processing (44)
Libraries/LibWeb/Compositor/AsyncScrollTree.cppLibraries/LibWeb/Compositor/AsyncScrollingState.cppLibraries/LibWeb/Compositor/AsyncScrollingState.hLibraries/LibWeb/DOM/Document.cppLibraries/LibWeb/HTML/LocalNavigable.cppLibraries/LibWeb/HTML/Window.cppLibraries/LibWeb/Layout/ScrollableOverflow.cppLibraries/LibWeb/Layout/ScrollableOverflow.hLibraries/LibWeb/Painting/DisplayListCommand.cppLibraries/LibWeb/Painting/DisplayListCommand.hLibraries/LibWeb/Painting/Paintable.cppLibraries/LibWeb/Painting/Paintable.hLibraries/LibWeb/Painting/Scrollbar.cppLibraries/LibWeb/Rust/src/layout/block_formatting_context.rsLibraries/LibWeb/Rust/src/layout/line_builder.rsServices/Compositor/ViewportScrollbarController.cppTests/LibWeb/Layout/expected/block-and-inline/rtl-overflowing-line-inline-position.txtTests/LibWeb/Layout/input/block-and-inline/rtl-overflowing-line-inline-position.htmlTests/LibWeb/Ref/input/abspos-after-rtl-inline-static-position.htmlTests/LibWeb/Text/expected/async-scrolling/blocking-wheel-event-regions.txtTests/LibWeb/Text/expected/async-scrolling/blocking-wheel-listener-invalidation.txtTests/LibWeb/Text/expected/async-scrolling/nested-navigable-wheel-admission.txtTests/LibWeb/Text/expected/async-scrolling/nested-scroller-keeps-sync-wheel.txtTests/LibWeb/Text/expected/async-scrolling/scroll-node-metadata-cache-replay.txtTests/LibWeb/Text/expected/async-scrolling/scroll-tree-parent-nodes.txtTests/LibWeb/Text/expected/async-scrolling/scroll-tree-shape.txtTests/LibWeb/Text/expected/async-scrolling/sticky-areas.txtTests/LibWeb/Text/expected/async-scrolling/viewport-wheel-with-nested-scroller.txtTests/LibWeb/Text/expected/async-scrolling/wheel-scroll-admission.txtTests/LibWeb/Text/expected/display_list/background-attachment-fixed.txtTests/LibWeb/Text/expected/display_list/nested-overflow-containers.txtTests/LibWeb/Text/expected/display_list/scrollable-in-fixed.txtTests/LibWeb/Text/expected/display_list/sibling-scrollables-in-fixed.txtTests/LibWeb/Text/expected/display_list/svg-foreignobject-scroller-after-relayout.txtTests/LibWeb/Text/expected/input-scrolls-caret-width-into-view.txtTests/LibWeb/Text/expected/rtl-viewport-programmatic-scroll.txtTests/LibWeb/Text/expected/scrollable-overflow-webkit-box-wrap-reverse.txtTests/LibWeb/Text/expected/wpt-import/css/css-overflow/overflow-rtl-scroll-left.txtTests/LibWeb/Text/expected/wpt-import/css/cssom-view/scrollLeftTop.txtTests/LibWeb/Text/input/input-scrolls-caret-width-into-view.htmlTests/LibWeb/Text/input/rtl-viewport-programmatic-scroll.htmlTests/LibWeb/Text/input/scrollable-overflow-webkit-box-wrap-reverse.htmlTests/LibWeb/Text/input/wpt-import/css/css-overflow/overflow-rtl-scroll-left.htmlTests/LibWeb/Text/input/wpt-import/css/cssom-view/scrollLeftTop.html
🚧 Files skipped from review as they are similar to previous changes (40)
- Libraries/LibWeb/Painting/DisplayListCommand.cpp
- Libraries/LibWeb/Rust/src/layout/block_formatting_context.rs
- Tests/LibWeb/Text/expected/async-scrolling/scroll-tree-shape.txt
- Tests/LibWeb/Layout/expected/block-and-inline/rtl-overflowing-line-inline-position.txt
- Libraries/LibWeb/Painting/Paintable.h
- Libraries/LibWeb/Compositor/AsyncScrollingState.h
- Libraries/LibWeb/Compositor/AsyncScrollTree.cpp
- Tests/LibWeb/Text/input/input-scrolls-caret-width-into-view.html
- Tests/LibWeb/Text/expected/async-scrolling/sticky-areas.txt
- Tests/LibWeb/Text/expected/async-scrolling/nested-navigable-wheel-admission.txt
- Tests/LibWeb/Text/expected/async-scrolling/scroll-node-metadata-cache-replay.txt
- Tests/LibWeb/Text/expected/async-scrolling/nested-scroller-keeps-sync-wheel.txt
- Tests/LibWeb/Layout/input/block-and-inline/rtl-overflowing-line-inline-position.html
- Tests/LibWeb/Text/expected/display_list/scrollable-in-fixed.txt
- Tests/LibWeb/Text/expected/async-scrolling/blocking-wheel-event-regions.txt
- Libraries/LibWeb/Layout/ScrollableOverflow.cpp
- Tests/LibWeb/Text/expected/display_list/background-attachment-fixed.txt
- Tests/LibWeb/Text/expected/async-scrolling/scroll-tree-parent-nodes.txt
- Tests/LibWeb/Text/expected/async-scrolling/viewport-wheel-with-nested-scroller.txt
- Tests/LibWeb/Text/expected/display_list/nested-overflow-containers.txt
- Tests/LibWeb/Text/input/wpt-import/css/css-overflow/overflow-rtl-scroll-left.html
- Libraries/LibWeb/Painting/DisplayListCommand.h
- Libraries/LibWeb/Layout/ScrollableOverflow.h
- Libraries/LibWeb/Rust/src/layout/line_builder.rs
- Tests/LibWeb/Text/expected/wpt-import/css/css-overflow/overflow-rtl-scroll-left.txt
- Tests/LibWeb/Text/expected/async-scrolling/wheel-scroll-admission.txt
- Tests/LibWeb/Text/expected/input-scrolls-caret-width-into-view.txt
- Libraries/LibWeb/Compositor/AsyncScrollingState.cpp
- Libraries/LibWeb/HTML/Window.cpp
- Libraries/LibWeb/HTML/LocalNavigable.cpp
- Tests/LibWeb/Text/expected/wpt-import/css/cssom-view/scrollLeftTop.txt
- Tests/LibWeb/Text/expected/display_list/svg-foreignobject-scroller-after-relayout.txt
- Tests/LibWeb/Text/input/scrollable-overflow-webkit-box-wrap-reverse.html
- Tests/LibWeb/Text/expected/display_list/sibling-scrollables-in-fixed.txt
- Services/Compositor/ViewportScrollbarController.cpp
- Tests/LibWeb/Text/expected/scrollable-overflow-webkit-box-wrap-reverse.txt
- Libraries/LibWeb/Painting/Paintable.cpp
- Tests/LibWeb/Text/expected/async-scrolling/blocking-wheel-listener-invalidation.txt
- Libraries/LibWeb/Painting/Scrollbar.cpp
- Tests/LibWeb/Ref/input/abspos-after-rtl-inline-static-position.html
github-actions
Bot
added
the
conflicts
|
Your pull request has conflicts that need to be resolved before it can be reviewed and merged. Make sure to rebase your branch on top of the latest |
The scroll offset range was previously clamped to begin at zero. A scroll container whose scrollable overflow extends towards the inline start requires negative offsets to reach that content. Compute both bounds from the position of the scrollable overflow rectangle relative to the scrollport.
Thumb geometry previously assumed the scroll offset range begins at zero. Measure thumb travel from the minimum scroll offset so that a container scrolled to a negative offset shows the thumb in the correct position.
Async scroll nodes and viewport scrollbars only carried a maximum scroll offset. Record the minimum as well, so that wheel scrolling and scrollbar dragging on the compositor thread can reach negative offsets.
CSS 2 assigns leftover inline space to the end-side margin, which is the left margin when the containing block's direction is `rtl`. Block-level boxes in right-to-left containers are now aligned to the right, and boxes wider than their container overflow to the left.
Previously, a line box wider than its container was kept flush with the left content edge. Content in a right-to-left container therefore overflowed to the right, where it is unreachable. We now ignore `text-align` for overflowing lines and align them to the inline start edge. In a right-to-left container the overflow extends past the left content edge, where it can be scrolled to.
The viewport's computed values previously kept the initial writing mode and direction, so the leftward overflow of a right-to-left document was clipped as unreachable and the viewport could not scroll to it. The viewport now takes the principal writing mode from the root element, or from its first body child in HTML documents.
tcl3
force-pushed
the
libweb_rtl_horizontal_scrolling
branch
from
August 13, 2026 14:03
8273597 to
47fc2c8
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
github-actions
Bot
removed
the
conflicts
Member
Author
|
Rebased to resolve conflicts. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, RTL scroll containers couldn't be scrolled in the correct direction, making their text unreachable. This PR ensures that horizontal scrolling works correctly for RTL elements.
See individual commits for details.