LibWeb: Don't overflow flex containers on margin auto

In case flex items had `margin: auto` on the primary flex axis, we were
still also distributing remaining space according to `justify-content`
rules. This lead to duplicated spacing in various places and overflows.

It looks like this issue was observed previously but missidentified
because there was logic to ignore margins at the start and end which
would partially paper over the root cause. However this created other
bugs (like for example not having a margin at beginning and end ;-)) and
I can find nothing in the spec or other browser behaviour that indicates
that this is something that should be done.

Now we skip justify-content space distribution alltogether if it has
already been distributed to auto margins.

1 files changed, 44 insertions, 43 deletions

diff --git a/Userland/Libraries/LibWeb/Layout/FlexFormattingContext.cpp b/Userland/Libraries/LibWeb/Layout/FlexFormattingContext.cpp
index 2dd053c07c..95abfb3bad 100644
--- a/Userland/Libraries/LibWeb/Layout/FlexFormattingContext.cpp
+++ b/Userland/Libraries/LibWeb/Layout/FlexFormattingContext.cpp
@@ -1255,82 +1255,83 @@ void FlexFormattingContext::distribute_any_remaining_free_space()
         CSSPixels initial_offset = 0;
         auto number_of_items = flex_line.items.size();
 
+        if (auto_margins == 0) {
+            switch (flex_container().computed_values().justify_content()) {
+            case CSS::JustifyContent::Start:
+            case CSS::JustifyContent::FlexStart:
+                if (is_direction_reverse()) {
+                    initial_offset = inner_main_size(flex_container());
+                } else {
+                    initial_offset = 0;
+                }
+                break;
+            case CSS::JustifyContent::End:
+            case CSS::JustifyContent::FlexEnd:
+                if (is_direction_reverse()) {
+                    initial_offset = 0;
+                } else {
+                    initial_offset = inner_main_size(flex_container());
+                }
+                break;
+            case CSS::JustifyContent::Center:
+                initial_offset = (inner_main_size(flex_container()) - used_main_space) / 2.0f;
+                break;
+            case CSS::JustifyContent::SpaceBetween:
+                space_between_items = flex_line.remaining_free_space / (number_of_items - 1);
+                break;
+            case CSS::JustifyContent::SpaceAround:
+                space_between_items = flex_line.remaining_free_space / number_of_items;
+                initial_offset = space_between_items / 2.0f;
+                break;
+            }
+        }
+
+        // For reverse, we use FlexRegionRenderCursor::Right
+        // to indicate the cursor offset is the end and render backwards
+        // Otherwise the cursor offset is the 'start' of the region or initial offset
         enum class FlexRegionRenderCursor {
             Left,
             Right
         };
-
         auto flex_region_render_cursor = FlexRegionRenderCursor::Left;
-        bool justification_is_centered = false;
 
         switch (flex_container().computed_values().justify_content()) {
         case CSS::JustifyContent::Start:
         case CSS::JustifyContent::FlexStart:
             if (is_direction_reverse()) {
                 flex_region_render_cursor = FlexRegionRenderCursor::Right;
-                initial_offset = inner_main_size(flex_container());
-            } else {
-                initial_offset = 0;
             }
             break;
         case CSS::JustifyContent::End:
         case CSS::JustifyContent::FlexEnd:
-            if (is_direction_reverse()) {
-                initial_offset = 0;
-            } else {
+            if (!is_direction_reverse()) {
                 flex_region_render_cursor = FlexRegionRenderCursor::Right;
-                initial_offset = inner_main_size(flex_container());
             }
             break;
-        case CSS::JustifyContent::Center:
-            initial_offset = (inner_main_size(flex_container()) - used_main_space) / 2.0f;
-            justification_is_centered = true;
-            break;
-        case CSS::JustifyContent::SpaceBetween:
-            space_between_items = flex_line.remaining_free_space / (number_of_items - 1);
-            break;
-        case CSS::JustifyContent::SpaceAround:
-            space_between_items = flex_line.remaining_free_space / number_of_items;
-            initial_offset = space_between_items / 2.0f;
-            justification_is_centered = true;
+        default:
             break;
         }
 
-        // For reverse, we use FlexRegionRenderCursor::Right
-        // to indicate the cursor offset is the end and render backwards
-        // Otherwise the cursor offset is the 'start' of the region or initial offset
         CSSPixels cursor_offset = initial_offset;
 
-        auto place_item = [&](FlexItem& item, bool is_first_item, bool is_last_item) {
-            // NOTE: For centered justifications (`center` and `space-around`) we ignore any margin
-            //       before the first item, and after the last item.
-
-            auto item_margin_before = item.margins.main_before;
-            auto item_margin_after = item.margins.main_after;
-            if (justification_is_centered) {
-                if (is_first_item)
-                    item_margin_before = 0;
-                if (is_last_item)
-                    item_margin_after = 0;
-            }
-
+        auto place_item = [&](FlexItem& item) {
             auto amount_of_main_size_used = item.main_size.value()
-                + item_margin_before
+                + item.margins.main_before
                 + item.borders.main_before
                 + item.padding.main_before
-                + item_margin_after
+                + item.margins.main_after
                 + item.borders.main_after
                 + item.padding.main_after
                 + space_between_items;
 
             if (is_direction_reverse()) {
-                item.main_offset = cursor_offset - item.main_size.value() - item_margin_after - item.borders.main_after - item.padding.main_after;
+                item.main_offset = cursor_offset - item.main_size.value() - item.margins.main_after - item.borders.main_after - item.padding.main_after;
                 cursor_offset -= amount_of_main_size_used;
             } else if (flex_region_render_cursor == FlexRegionRenderCursor::Right) {
                 cursor_offset -= amount_of_main_size_used;
-                item.main_offset = cursor_offset + item_margin_before + item.borders.main_before + item.padding.main_before;
+                item.main_offset = cursor_offset + item.margins.main_before + item.borders.main_before + item.padding.main_before;
             } else {
-                item.main_offset = cursor_offset + item_margin_before + item.borders.main_before + item.padding.main_before;
+                item.main_offset = cursor_offset + item.margins.main_before + item.borders.main_before + item.padding.main_before;
                 cursor_offset += amount_of_main_size_used;
             }
         };
@@ -1338,12 +1339,12 @@ void FlexFormattingContext::distribute_any_remaining_free_space()
         if (is_direction_reverse() || flex_region_render_cursor == FlexRegionRenderCursor::Right) {
             for (ssize_t i = flex_line.items.size() - 1; i >= 0; --i) {
                 auto& item = flex_line.items[i];
-                place_item(item, i == static_cast<ssize_t>(flex_line.items.size()) - 1, i == 0);
+                place_item(item);
             }
         } else {
             for (size_t i = 0; i < flex_line.items.size(); ++i) {
                 auto& item = flex_line.items[i];
-                place_item(item, i == 0, i == flex_line.items.size() - 1);
+                place_item(item);
             }
         }
     }