LibWeb: Make foreignObject establish a containing block for abspos

Absolutely positioned elements inside SVG foreignObject were being
positioned relative to an ancestor containing block outside the SVG,
instead of relative to the foreignObject itself. Per a W3C resolution
and the behavior of other browsers, foreignObject should establish a
containing block for absolutely and fixed positioned elements.

With this fix, the `has_abspos_with_external_containing_block` check
in `set_needs_layout_update()` and the abspos preservation loop in
`relayout_svg_root()` become dead code — remove both and simplify the
ancestor loops. Rename related tests to reflect the new behavior.

Fixes https://github.com/LadybirdBrowser/ladybird/issues/3241
This commit is contained in:
Aliaksandr Kalenik 2026-02-17 04:59:31 +01:00 committed by Alexander Kalenik
parent fa49de8956
commit b3231ea2a0
8 changed files with 61 additions and 50 deletions

View file

@ -1354,33 +1354,14 @@ static void relayout_svg_root(Layout::SVGSVGBox& svg_root)
if (auto const* paintable = svg_root.paintable_box())
layout_state.populate_from_paintable(svg_root, *paintable);
// Pre-populate ancestors outside the subtree:
// - SVGGraphicsBox ancestors (up to outer SVG) for get_parent_svg_transform()
// - Abspos boxes inside the subtree whose layout is managed by ancestor formatting contexts (not re-laid-out
// during SVG relayout, so their existing paintable state must be preserved)
bool found_outer_svg = false;
// Pre-populate SVGGraphicsBox ancestors (up to outer SVG) for get_parent_svg_transform().
for (auto* ancestor = svg_root.parent(); ancestor; ancestor = ancestor->parent()) {
if (!found_outer_svg) {
if (auto const* svg_graphics_ancestor = as_if<Layout::SVGGraphicsBox>(*ancestor)) {
if (auto const* paintable = svg_graphics_ancestor->paintable_box())
layout_state.populate_from_paintable(*svg_graphics_ancestor, *paintable);
}
if (is<Layout::SVGSVGBox>(*ancestor))
found_outer_svg = true;
}
if (auto const* box = as_if<Layout::Box>(*ancestor)) {
// Pre-populate abspos boxes that are inside the SVG subtree but whose containing block is outside it.
// These boxes are laid out by ancestor formatting contexts (not during SVG relayout), so we must preserve
// their existing paintable state to prevent commit() from destroying their paintables.
for (auto const& abspos_child : box->contained_abspos_children()) {
if (svg_root.is_inclusive_ancestor_of(abspos_child) && is<Layout::Box>(*abspos_child)) {
auto const& abspos_box = static_cast<Layout::Box const&>(*abspos_child);
if (auto const* abspos_paintable = abspos_box.paintable_box())
layout_state.populate_from_paintable(abspos_box, *abspos_paintable);
}
}
if (auto const* svg_graphics_ancestor = as_if<Layout::SVGGraphicsBox>(*ancestor)) {
if (auto const* paintable = svg_graphics_ancestor->paintable_box())
layout_state.populate_from_paintable(*svg_graphics_ancestor, *paintable);
}
if (is<Layout::SVGSVGBox>(*ancestor))
break;
}
auto const& svg_state = layout_state.get(svg_root);

View file

@ -160,6 +160,11 @@ bool Node::establishes_an_absolute_positioning_containing_block() const
if (is<Viewport>(*this))
return true;
// https://github.com/w3c/fxtf-drafts/issues/307#issuecomment-499612420
// foreignObject establishes a containing block for absolutely and fixed positioned elements.
if (is_svg_foreign_object_box())
return true;
return computed_values_establish_absolute_positioning_containing_block();
}
@ -1467,29 +1472,11 @@ void Node::set_needs_layout_update(DOM::SetNeedsLayoutReason reason)
return IterationDecision::Continue;
});
auto has_abspos_with_external_containing_block = [](SVGSVGBox const& svg_box) {
for (auto const* ancestor = svg_box.parent(); ancestor; ancestor = ancestor->parent()) {
auto const* box = as_if<Box>(ancestor);
if (!box)
continue;
for (auto const& abspos_child : box->contained_abspos_children()) {
if (svg_box.is_inclusive_ancestor_of(abspos_child))
return true;
}
}
return false;
};
for (auto* ancestor = parent(); ancestor; ancestor = ancestor->parent()) {
if (ancestor->m_needs_layout_update)
break;
ancestor->m_needs_layout_update = true;
if (auto* svg_box = as_if<SVGSVGBox>(ancestor)) {
// Absolutely positioned elements inside the SVG subtree whose containing
// block is outside the SVG can't be properly relaid out during partial SVG
// relayout — their layout depends on formatting contexts outside the subtree.
if (has_abspos_with_external_containing_block(*svg_box))
continue;
document().mark_svg_root_as_needing_relayout(*svg_box);
break;
}

View file

@ -0,0 +1,29 @@
Viewport <#document> at [0,0] [0+0+0 800 0+0+0] [0+0+0 600 0+0+0] [BFC] children: not-inline
BlockContainer <html> at [0,0] [0+0+0 800 0+0+0] [0+0+0 216 0+0+0] [BFC] children: not-inline
BlockContainer <body> at [8,8] [8+0+0 784 0+0+8] [8+0+0 200 0+0+8] children: inline
frag 0 from SVGSVGBox start: 0, length: 0, rect: [8,8 200x200] baseline: 200
SVGSVGBox <svg> at [8,8] [0+0+0 200 0+0+0] [0+0+0 200 0+0+0] [SVG] children: inline
TextNode <#text> (not painted)
SVGForeignObjectBox <foreignObject> at [58,58] [0+0+0 100 0+0+0] [0+0+0 100 0+0+0] [BFC] children: not-inline
BlockContainer <(anonymous)> at [58,58] [0+0+0 100 0+0+0] [0+0+0 0 0+0+0] children: inline
TextNode <#text> (not painted)
BlockContainer <div> at [58,58] positioned [0+0+0 100 0+0+0] [0+0+0 100 0+0+0] [BFC] children: inline
frag 0 from TextNode start: 0, length: 12, rect: [58,58 94.15625x18] baseline: 13.796875
"I am offset!"
TextNode <#text> (not painted)
TextNode <#text> (not painted)
TextNode <#text> (not painted)
TextNode <#text> (not painted)
ViewportPaintable (Viewport<#document>) [0,0 800x600]
PaintableWithLines (BlockContainer<HTML>) [0,0 800x216]
PaintableWithLines (BlockContainer<BODY>) [8,8 784x200]
SVGSVGPaintable (SVGSVGBox<svg>) [8,8 200x200]
SVGForeignObjectPaintable (SVGForeignObjectBox<foreignObject>) [58,58 100x100]
PaintableWithLines (BlockContainer(anonymous)) [58,58 100x0]
PaintableWithLines (BlockContainer<DIV>) [58,58 100x100]
TextPaintable (TextNode<#text>)
SC for Viewport<#document> [0,0 800x600] [children: 1] (z-index: auto)
SC for BlockContainer<HTML> [0,0 800x216] [children: 1] (z-index: auto)
SC for SVGForeignObjectBox<foreignObject> [58,58 100x100] [children: 0] (z-index: auto)

View file

@ -0,0 +1,15 @@
<!DOCTYPE html>
<style>
div {
position: absolute;
top: 0;
left: 0;
width: 100px;
height: 100px;
}
</style>
<svg width="200" height="200">
<foreignObject width="100" height="100" x="50" y="50">
<div>I am offset!</div>
</foreignObject>
</svg>

View file

@ -1,11 +1,11 @@
Before SVG relayout:
abspos width: 50
abspos height: 50
abspos width: 25
abspos height: 25
abspos x: 18
abspos y: 18
After SVG relayout:
abspos width: 50
abspos height: 50
abspos width: 25
abspos height: 25
abspos x: 18
abspos y: 18
Abspos preserved after SVG relayout:

View file

@ -32,9 +32,8 @@
// Change the abspos element's size via inline style.
// Since this element is inside the SVG subtree (via foreignObject),
// set_needs_layout_update() breaks at SVGSVGBox and triggers partial SVG relayout.
// However, the abspos element's containing block is the .container div OUTSIDE
// the SVG subtree, so the formatting context responsible for laying out the abspos
// won't run during partial SVG relayout. This means the size change won't take effect.
// The abspos element's containing block is the foreignObject itself, so the size
// change should take effect during partial SVG relayout.
const abspos = document.getElementById("abspos");
abspos.style.width = "100px";
abspos.style.height = "100px";