Bladeren bron

LibWeb: Only include containing blocks in coordinate space translation

Layout box offset coordinates are always relative to their containing
block. Therefore, the functions that convert between coordinate spaces
should only visit containing blocks and apply their offsets, not *every*
box in the parent chain.

This fixes an issue where some floating boxes were unexpectedly far away
from their containing block.
Andreas Kling 2 jaren geleden
bovenliggende
commit
d43ef27761
2 gewijzigde bestanden met toevoegingen van 76 en 21 verwijderingen
  1. 52 0
      Base/res/html/misc/float-stress-3.html
  2. 24 21
      Userland/Libraries/LibWeb/Layout/LayoutState.cpp

+ 52 - 0
Base/res/html/misc/float-stress-3.html

@@ -0,0 +1,52 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <title>float horror show</title>
+    <style type="text/css">
+html {
+    font: 16px/1 Arial;
+}
+      .outer {
+        border: 1px solid black;
+        width: 300px;
+        height: 250px;
+      }
+
+      .one {
+        float: left;
+        height: 50px;
+        width: 200px;
+        margin: 0;
+        border: 1px solid black;
+        background-color: #fc0;
+      }
+      .two {
+        float: right;
+        height: 50px;
+        width: 200px;
+        margin: 0;
+        border: 1px solid black;
+        background-color: pink;
+      }
+      .lefty {
+        float: left;
+        height: 50px;
+        width: 50px;
+        margin: 0;
+        border: 1px solid black;
+        background-color: lime;
+      }
+      .righty {
+        float: right;
+        height: 30px;
+        width: 30px;
+        margin: 0;
+        border: 1px solid black;
+        background-color: magenta;
+      }
+</style></head><body>
+    <div class=outer>
+        foo bar baz foo bar baz
+        <div class=lefty></div>
+        <div class=one></div>
+        <div class=two></div>

+ 24 - 21
Userland/Libraries/LibWeb/Layout/LayoutState.cpp

@@ -121,15 +121,16 @@ Gfx::FloatRect border_box_rect(Box const& box, LayoutState const& state)
 Gfx::FloatRect border_box_rect_in_ancestor_coordinate_space(Box const& box, Box const& ancestor_box, LayoutState const& state)
 {
     auto rect = border_box_rect(box, state);
-    for (auto const* current = box.parent(); current; current = current->parent()) {
+    if (&box == &ancestor_box)
+        return rect;
+    for (auto const* current = box.containing_block(); current; current = current->containing_block()) {
         if (current == &ancestor_box)
-            break;
-        if (is<Box>(*current)) {
-            auto const& current_state = state.get(static_cast<Box const&>(*current));
-            rect.translate_by(current_state.offset);
-        }
+            return rect;
+        auto const& current_state = state.get(static_cast<Box const&>(*current));
+        rect.translate_by(current_state.offset);
     }
-    return rect;
+    // If we get here, ancestor_box was not a containing block ancestor of `box`!
+    VERIFY_NOT_REACHED();
 }
 
 Gfx::FloatRect content_box_rect(Box const& box, LayoutState const& state)
@@ -141,29 +142,31 @@ Gfx::FloatRect content_box_rect(Box const& box, LayoutState const& state)
 Gfx::FloatRect content_box_rect_in_ancestor_coordinate_space(Box const& box, Box const& ancestor_box, LayoutState const& state)
 {
     auto rect = content_box_rect(box, state);
-    for (auto const* current = box.parent(); current; current = current->parent()) {
+    if (&box == &ancestor_box)
+        return rect;
+    for (auto const* current = box.containing_block(); current; current = current->containing_block()) {
         if (current == &ancestor_box)
-            break;
-        if (is<Box>(*current)) {
-            auto const& current_state = state.get(static_cast<Box const&>(*current));
-            rect.translate_by(current_state.offset);
-        }
+            return rect;
+        auto const& current_state = state.get(static_cast<Box const&>(*current));
+        rect.translate_by(current_state.offset);
     }
-    return rect;
+    // If we get here, ancestor_box was not a containing block ancestor of `box`!
+    VERIFY_NOT_REACHED();
 }
 
 Gfx::FloatRect margin_box_rect_in_ancestor_coordinate_space(Box const& box, Box const& ancestor_box, LayoutState const& state)
 {
     auto rect = margin_box_rect(box, state);
-    for (auto const* current = box.parent(); current; current = current->parent()) {
+    if (&box == &ancestor_box)
+        return rect;
+    for (auto const* current = box.containing_block(); current; current = current->containing_block()) {
         if (current == &ancestor_box)
-            break;
-        if (is<Box>(*current)) {
-            auto const& current_state = state.get(static_cast<Box const&>(*current));
-            rect.translate_by(current_state.offset);
-        }
+            return rect;
+        auto const& current_state = state.get(static_cast<Box const&>(*current));
+        rect.translate_by(current_state.offset);
     }
-    return rect;
+    // If we get here, ancestor_box was not a containing block ancestor of `box`!
+    VERIFY_NOT_REACHED();
 }
 
 Gfx::FloatRect absolute_content_rect(Box const& box, LayoutState const& state)