Skip to content

LibWeb: Use FfiDisplay directly for table roles - #10946

Merged
kalenikaliaksandr merged 2 commits into
LadybirdBrowser:masterfrom
kalenikaliaksandr:drop-ffi-table-display
Jul 31, 2026
Merged

LibWeb: Use FfiDisplay directly for table roles#10946
kalenikaliaksandr merged 2 commits into
LadybirdBrowser:masterfrom
kalenikaliaksandr:drop-ffi-table-display

Conversation

@kalenikaliaksandr

Copy link
Copy Markdown
Member

Rust layout classified displays into table-specific enum before checking
their roles. This duplicated predicates already available on FfiDisplay
and forced every consumer through an unnecessary conversion layer.

Remove FfiTableDisplay and table_display_of. Tree construction and
formatting context selection, node facts, and table grid construction
now query FfiDisplay directly. Spec-level grouping helpers remain only
where they express table concepts instead of mirroring display roles.

Rust layout classified displays into table-specific enum before checking
their roles. This duplicated predicates already available on FfiDisplay
and forced every consumer through an unnecessary conversion layer.

Remove FfiTableDisplay and table_display_of. Tree construction and
formatting context selection, node facts, and table grid construction
now query FfiDisplay directly. Spec-level grouping helpers remain only
where they express table concepts instead of mirroring display roles.
Replacing a layout node transferred saved absolute-positioning inputs by
calling from Rust into C++ and back through an exported arena function.
TreeBuilder already has the arena and both slot IDs at that point.

Copy the inputs directly in the replacement branch and remove the C ABI
entry point and arena helper. Remove the dedicated unit test that only
exercised this transfer, along with all of its test-only imports.
@coderabbitai

coderabbitai Bot commented Jul 31, 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: 5e8420d9-66bd-4054-9bd0-7903b53ce37d

📥 Commits

Reviewing files that changed from the base of the PR and between fead442 and 06c5069.

📒 Files selected for processing (8)
  • Libraries/LibWeb/Layout/TreeBuilder.cpp
  • Libraries/LibWeb/Rust/src/layout/formatting_context.rs
  • Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs
  • Libraries/LibWeb/Rust/src/layout/layout_state.rs
  • Libraries/LibWeb/Rust/src/layout/node_facts.rs
  • Libraries/LibWeb/Rust/src/layout/style_facts.rs
  • Libraries/LibWeb/Rust/src/layout/table_formatting_context.rs
  • Libraries/LibWeb/Rust/src/layout/tree_builder.rs
💤 Files with no reviewable changes (3)
  • Libraries/LibWeb/Layout/TreeBuilder.cpp
  • Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs
  • Libraries/LibWeb/Rust/src/layout/style_facts.rs

📝 Walkthrough

Walkthrough

The layout system now classifies table roles directly from FfiDisplay values. The obsolete FfiTableDisplay APIs and saved-absolute-position FFI transfer path were removed. Box replacement preserves saved absolute-position inputs through local arena operations.

Changes

Layout classification and replacement

Layer / File(s) Summary
Direct display classification
Libraries/LibWeb/Rust/src/layout/style_facts.rs, Libraries/LibWeb/Rust/src/layout/formatting_context.rs, Libraries/LibWeb/Rust/src/layout/layout_state.rs, Libraries/LibWeb/Rust/src/layout/node_facts.rs
Removed FfiTableDisplay classification APIs. Formatting-context and layout-state checks now use direct FfiDisplay predicates.
Table tree and fixup classification
Libraries/LibWeb/Rust/src/layout/table_formatting_context.rs, Libraries/LibWeb/Rust/src/layout/tree_builder.rs
Table traversal, wrapper generation, parent and child fixup, and replaced-element handling now use current or pre-transformation FfiDisplay values.
Replacement state transfer cleanup
Libraries/LibWeb/Rust/src/layout/tree_builder.rs, Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs, Libraries/LibWeb/Layout/TreeBuilder.cpp
Box replacement copies saved absolute-position inputs locally. The Rust arena wrapper and C++ FFI call were removed.

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

Possibly related PRs

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The description directly matches the changes to remove FfiTableDisplay and use FfiDisplay for table-role handling.
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) July 31, 2026 06:15
@kalenikaliaksandr
kalenikaliaksandr merged commit 5780ba9 into LadybirdBrowser:master Jul 31, 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