LibWeb: Use static position for abspos box axes with auto insets

When both insets in a given axis are auto, we should use the static position for absolutely positioned elements. By doing this correctly, we exposed a bunch of other small bugs which had to be fixed to compensate for new test failures. Those fixes are included here as well: - Don't apply margins twice. - Compute the static position containing block chain correctly. This makes https://brave.com/ look much better. :^)
awesomekling · Apr 11, 2024 · f6fbb64 · f6fbb64
1 parent ee3dd79
commit f6fbb64
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 10 deletions.
diff --git a/Tests/LibWeb/Layout/expected/abspos-with-static-position-in-one-axis.txt b/Tests/LibWeb/Layout/expected/abspos-with-static-position-in-one-axis.txt
@@ -0,0 +1,15 @@
+Viewport <#document> at (0,0) content-size 800x600 children: not-inline
+  BlockContainer <html> at (0,0) content-size 800x33 [BFC] children: not-inline
+    BlockContainer <body> at (8,8) content-size 784x17 children: not-inline
+      BlockContainer <div> at (8,8) content-size 784x17 children: inline
+        frag 0 from TextNode start: 0, length: 13, rect: [8,8 100.203125x17] baseline: 13.296875
+            "hello friends"
+        TextNode <#text>
+      ImageBox <img> at (50,25) content-size 100x100 positioned children: not-inline
+
+ViewportPaintable (Viewport<#document>) [0,0 800x600]
+  PaintableWithLines (BlockContainer<HTML>) [0,0 800x33] overflow: [0,0 800x125]
+    PaintableWithLines (BlockContainer<BODY>) [8,8 784x17] overflow: [8,8 784x117]
+      PaintableWithLines (BlockContainer<DIV>) [8,8 784x17]
+        TextPaintable (TextNode<#text>)
+      ImagePaintable (ImageBox<IMG>) [50,25 100x100]
diff --git a/Tests/LibWeb/Layout/input/abspos-with-static-position-in-one-axis.html b/Tests/LibWeb/Layout/input/abspos-with-static-position-in-one-axis.html
@@ -0,0 +1,10 @@
+<!doctype html><style>
+* { outline: 1px solid black; }
+img {
+    position: absolute;
+    width: 100px;
+    height: 100px;
+    background: green;
+    left: 50px;
+}
+</style><div>hello friends</div><img>
diff --git a/Userland/Libraries/LibWeb/Layout/FormattingContext.cpp b/Userland/Libraries/LibWeb/Layout/FormattingContext.cpp
@@ -1163,7 +1163,7 @@ CSSPixelRect FormattingContext::content_box_rect_in_static_position_ancestor_coo
     auto rect = content_box_rect(box);
     if (&box == &ancestor_box)
         return rect;
-    for (auto const* current = box.parent(); current; current = current->parent()) {
+    for (auto const* current = box.static_position_containing_block(); current; current = current->static_position_containing_block()) {
         if (current == &ancestor_box)
             return rect;
         auto const& current_state = m_state.get(*current);
@@ -1210,9 +1210,8 @@ CSSPixelPoint FormattingContext::calculate_static_position(Box const& box) const
         }
     } else {
         auto const& box_state = m_state.get(box);
-        x = box_state.margin_left;
         // We're among block siblings, Y can be calculated easily.
-        y = box_state.margin_top + box_state.vertical_offset_of_parent_block_container;
+        y = box_state.vertical_offset_of_parent_block_container;
     }
     auto offset_to_static_parent = content_box_rect_in_static_position_ancestor_coordinate_space(box, *box.containing_block());
     return offset_to_static_parent.location().translated(x, y);
@@ -1257,15 +1256,27 @@ void FormattingContext::layout_absolutely_positioned_element(Box const& box, Ava
     compute_height_for_absolutely_positioned_element(box, available_space, BeforeOrAfterInsideLayout::After);
 
     CSSPixelPoint used_offset;
-    used_offset.set_x(box_state.inset_left + box_state.margin_box_left());
-    used_offset.set_y(box_state.inset_top + box_state.margin_box_top());
-    // NOTE: Absolutely positioned boxes are relative to the *padding edge* of the containing block.
-    //       Padding offset only need to be compensated when top/left/bottom/right is not auto because otherwise
-    //       the box is positioned at the static position of the containing block.
-    if (!box.computed_values().inset().top().is_auto() || !box.computed_values().inset().bottom().is_auto())
+
+    auto static_position = calculate_static_position(box);
+
+    if (box.computed_values().inset().top().is_auto() && box.computed_values().inset().bottom().is_auto()) {
+        used_offset.set_y(static_position.y());
+    } else {
+        used_offset.set_y(box_state.inset_top);
+        // NOTE: Absolutely positioned boxes are relative to the *padding edge* of the containing block.
         used_offset.translate_by(0, -containing_block_state.padding_top);
-    if (!box.computed_values().inset().left().is_auto() || !box.computed_values().inset().right().is_auto())
+    }
+
+    if (box.computed_values().inset().left().is_auto() && box.computed_values().inset().right().is_auto()) {
+        used_offset.set_x(static_position.x());
+    } else {
+        used_offset.set_x(box_state.inset_left);
+        // NOTE: Absolutely positioned boxes are relative to the *padding edge* of the containing block.
         used_offset.translate_by(-containing_block_state.padding_left, 0);
+    }
+
+    used_offset.translate_by(box_state.margin_box_left(), box_state.margin_box_top());
+
     box_state.set_content_offset(used_offset);
 
     if (independent_formatting_context)

diff --git a/Userland/Libraries/LibWeb/Layout/Node.cpp b/Userland/Libraries/LibWeb/Layout/Node.cpp
@@ -126,6 +126,11 @@ Box const* Node::containing_block() const
     return nearest_ancestor_capable_of_forming_a_containing_block(*this);
 }
 
+Box const* Node::static_position_containing_block() const
+{
+    return nearest_ancestor_capable_of_forming_a_containing_block(*this);
+}
+
 Box const* Node::non_anonymous_containing_block() const
 {
     auto nearest_ancestor_box = containing_block();

diff --git a/Userland/Libraries/LibWeb/Layout/Node.h b/Userland/Libraries/LibWeb/Layout/Node.h
@@ -132,6 +132,9 @@ class Node
     Box const* containing_block() const;
     Box* containing_block() { return const_cast<Box*>(const_cast<Node const*>(this)->containing_block()); }
 
+    [[nodiscard]] Box const* static_position_containing_block() const;
+    [[nodiscard]] Box* static_position_containing_block() { return const_cast<Box*>(const_cast<Node const*>(this)->static_position_containing_block()); }
+
     // Closest non-anonymous ancestor box, to be used when resolving percentage values.
     // Anonymous block boxes are ignored when resolving percentage values that would refer to it:
     // the closest non-anonymous ancestor box is used instead.