浏览代码

LibWeb: Make align-content on flex container behave more correctly

In particular, this property now interacts correctly when the flex
container has flex-wrap: wrap-reverse.

This caused some "regressions" in WPT tests for negative overflow in
flex containers, but the previous behavior wasn't correct either,
it just happened to give false positives on tests.
Andreas Kling 9 月之前
父节点
当前提交
64f18a93c2

+ 19 - 19
Tests/LibWeb/Text/expected/wpt-import/css/css-flexbox/align-content-horiz-002.txt

@@ -6,8 +6,8 @@ Rerun
 
 
 Found 72 tests
 Found 72 tests
 
 
-54 Pass
-18 Fail
+71 Pass
+1 Fail
 Details
 Details
 Result	Test Name	MessagePass	.flexbox div 1	
 Result	Test Name	MessagePass	.flexbox div 1	
 Pass	.flexbox div 2	
 Pass	.flexbox div 2	
@@ -17,22 +17,22 @@ Pass	.flexbox div 5
 Pass	.flexbox div 6	
 Pass	.flexbox div 6	
 Pass	.flexbox div 7	
 Pass	.flexbox div 7	
 Pass	.flexbox div 8	
 Pass	.flexbox div 8	
-Fail	.flexbox div 9	
-Fail	.flexbox div 10	
-Fail	.flexbox div 11	
-Fail	.flexbox div 12	
-Fail	.flexbox div 13	
-Fail	.flexbox div 14	
-Fail	.flexbox div 15	
-Fail	.flexbox div 16	
-Fail	.flexbox div 17	
-Fail	.flexbox div 18	
-Fail	.flexbox div 19	
-Fail	.flexbox div 20	
-Fail	.flexbox div 21	
-Fail	.flexbox div 22	
-Fail	.flexbox div 23	
-Fail	.flexbox div 24	
+Pass	.flexbox div 9	
+Pass	.flexbox div 10	
+Pass	.flexbox div 11	
+Pass	.flexbox div 12	
+Pass	.flexbox div 13	
+Pass	.flexbox div 14	
+Pass	.flexbox div 15	
+Pass	.flexbox div 16	
+Pass	.flexbox div 17	
+Pass	.flexbox div 18	
+Pass	.flexbox div 19	
+Pass	.flexbox div 20	
+Pass	.flexbox div 21	
+Pass	.flexbox div 22	
+Pass	.flexbox div 23	
+Pass	.flexbox div 24	
 Pass	.flexbox div 25	
 Pass	.flexbox div 25	
 Pass	.flexbox div 26	
 Pass	.flexbox div 26	
 Pass	.flexbox div 27	
 Pass	.flexbox div 27	
@@ -41,7 +41,7 @@ Pass	.flexbox div 29
 Pass	.flexbox div 30	
 Pass	.flexbox div 30	
 Pass	.flexbox div 31	
 Pass	.flexbox div 31	
 Pass	.flexbox div 32	
 Pass	.flexbox div 32	
-Fail	.flexbox div 33	
+Pass	.flexbox div 33	
 Pass	.flexbox div 34	
 Pass	.flexbox div 34	
 Pass	.flexbox div 35	
 Pass	.flexbox div 35	
 Pass	.flexbox div 36	
 Pass	.flexbox div 36	

+ 19 - 19
Tests/LibWeb/Text/expected/wpt-import/css/css-flexbox/align-content-vert-002.txt

@@ -6,8 +6,8 @@ Rerun
 
 
 Found 72 tests
 Found 72 tests
 
 
-54 Pass
-18 Fail
+71 Pass
+1 Fail
 Details
 Details
 Result	Test Name	MessagePass	.flexbox div 1	
 Result	Test Name	MessagePass	.flexbox div 1	
 Pass	.flexbox div 2	
 Pass	.flexbox div 2	
@@ -17,22 +17,22 @@ Pass	.flexbox div 5
 Pass	.flexbox div 6	
 Pass	.flexbox div 6	
 Pass	.flexbox div 7	
 Pass	.flexbox div 7	
 Pass	.flexbox div 8	
 Pass	.flexbox div 8	
-Fail	.flexbox div 9	
-Fail	.flexbox div 10	
-Fail	.flexbox div 11	
-Fail	.flexbox div 12	
-Fail	.flexbox div 13	
-Fail	.flexbox div 14	
-Fail	.flexbox div 15	
-Fail	.flexbox div 16	
-Fail	.flexbox div 17	
-Fail	.flexbox div 18	
-Fail	.flexbox div 19	
-Fail	.flexbox div 20	
-Fail	.flexbox div 21	
-Fail	.flexbox div 22	
-Fail	.flexbox div 23	
-Fail	.flexbox div 24	
+Pass	.flexbox div 9	
+Pass	.flexbox div 10	
+Pass	.flexbox div 11	
+Pass	.flexbox div 12	
+Pass	.flexbox div 13	
+Pass	.flexbox div 14	
+Pass	.flexbox div 15	
+Pass	.flexbox div 16	
+Pass	.flexbox div 17	
+Pass	.flexbox div 18	
+Pass	.flexbox div 19	
+Pass	.flexbox div 20	
+Pass	.flexbox div 21	
+Pass	.flexbox div 22	
+Pass	.flexbox div 23	
+Pass	.flexbox div 24	
 Pass	.flexbox div 25	
 Pass	.flexbox div 25	
 Pass	.flexbox div 26	
 Pass	.flexbox div 26	
 Pass	.flexbox div 27	
 Pass	.flexbox div 27	
@@ -41,7 +41,7 @@ Pass	.flexbox div 29
 Pass	.flexbox div 30	
 Pass	.flexbox div 30	
 Pass	.flexbox div 31	
 Pass	.flexbox div 31	
 Pass	.flexbox div 32	
 Pass	.flexbox div 32	
-Fail	.flexbox div 33	
+Pass	.flexbox div 33	
 Pass	.flexbox div 34	
 Pass	.flexbox div 34	
 Pass	.flexbox div 35	
 Pass	.flexbox div 35	
 Pass	.flexbox div 36	
 Pass	.flexbox div 36	

+ 3 - 3
Tests/LibWeb/Text/expected/wpt-import/css/css-flexbox/negative-overflow.txt

@@ -6,8 +6,8 @@ Rerun
 
 
 Found 11 tests
 Found 11 tests
 
 
-5 Pass
-6 Fail
+4 Pass
+7 Fail
 Details
 Details
 Result	Test Name	MessageFail	.flexbox 1	
 Result	Test Name	MessageFail	.flexbox 1	
 Fail	.flexbox 2	
 Fail	.flexbox 2	
@@ -19,4 +19,4 @@ Fail	.flexbox 7
 Fail	.flexbox 8	
 Fail	.flexbox 8	
 Pass	.flexbox 9	
 Pass	.flexbox 9	
 Pass	.flexbox 10	
 Pass	.flexbox 10	
-Pass	.flexbox 11	
+Fail	.flexbox 11	

+ 109 - 57
Userland/Libraries/LibWeb/Layout/FlexFormattingContext.cpp

@@ -828,9 +828,6 @@ void FlexFormattingContext::collect_flex_items_into_flex_lines()
         line_main_size += main_gap();
         line_main_size += main_gap();
     }
     }
     m_flex_lines.append(move(line));
     m_flex_lines.append(move(line));
-
-    if (flex_container().computed_values().flex_wrap() == CSS::FlexWrap::WrapReverse)
-        m_flex_lines.reverse();
 }
 }
 
 
 // https://drafts.csswg.org/css-flexbox-1/#resolve-flexible-lengths
 // https://drafts.csswg.org/css-flexbox-1/#resolve-flexible-lengths
@@ -1517,13 +1514,17 @@ void FlexFormattingContext::align_all_flex_items_along_the_cross_axis()
     }
     }
 }
 }
 
 
-// https://www.w3.org/TR/css-flexbox-1/#algo-line-align
+// https://drafts.csswg.org/css-flexbox-1/#algo-line-align
 void FlexFormattingContext::align_all_flex_lines()
 void FlexFormattingContext::align_all_flex_lines()
 {
 {
+    // Align all flex lines per align-content.
+
     if (m_flex_lines.is_empty())
     if (m_flex_lines.is_empty())
         return;
         return;
 
 
-    // FIXME: Support reverse
+    bool wrap_reverse = flex_container().computed_values().flex_wrap() == CSS::FlexWrap::WrapReverse;
+    bool place_items_backwards = false;
+    bool iterate_lines_backwards = false;
 
 
     CSSPixels cross_size_of_flex_container = inner_cross_size(m_flex_container_state);
     CSSPixels cross_size_of_flex_container = inner_cross_size(m_flex_container_state);
 
 
@@ -1534,76 +1535,117 @@ void FlexFormattingContext::align_all_flex_lines()
         for (auto& item : flex_line.items) {
         for (auto& item : flex_line.items) {
             item.cross_offset += center_of_line;
             item.cross_offset += center_of_line;
         }
         }
-    } else {
+        return;
+    }
 
 
-        CSSPixels sum_of_flex_line_cross_sizes = 0;
-        for (auto& line : m_flex_lines)
-            sum_of_flex_line_cross_sizes += line.cross_size;
+    CSSPixels sum_of_flex_line_cross_sizes = 0;
+    for (auto& line : m_flex_lines)
+        sum_of_flex_line_cross_sizes += line.cross_size;
 
 
-        // CSS-FLEXBOX-2: Account for gap between flex lines.
-        sum_of_flex_line_cross_sizes += cross_gap() * (m_flex_lines.size() - 1);
+    // CSS-FLEXBOX-2: Account for gap between flex lines.
+    sum_of_flex_line_cross_sizes += cross_gap() * (m_flex_lines.size() - 1);
 
 
-        CSSPixels start_of_current_line = 0;
-        CSSPixels gap_size = 0;
-        switch (flex_container().computed_values().align_content()) {
-        case CSS::AlignContent::FlexStart:
-        case CSS::AlignContent::Start:
+    CSSPixels start_of_current_line = 0;
+    CSSPixels gap_size = 0;
+    switch (flex_container().computed_values().align_content()) {
+    case CSS::AlignContent::Start:
+        start_of_current_line = 0;
+        iterate_lines_backwards = wrap_reverse;
+        break;
+    case CSS::AlignContent::End:
+        start_of_current_line = cross_size_of_flex_container;
+        place_items_backwards = true;
+        iterate_lines_backwards = !wrap_reverse;
+        break;
+    case CSS::AlignContent::FlexStart:
+        if (wrap_reverse) {
+            start_of_current_line = cross_size_of_flex_container;
+            place_items_backwards = true;
+        } else {
             start_of_current_line = 0;
             start_of_current_line = 0;
-            break;
-        case CSS::AlignContent::FlexEnd:
-        case CSS::AlignContent::End:
-            start_of_current_line = cross_size_of_flex_container - sum_of_flex_line_cross_sizes;
-            break;
-        case CSS::AlignContent::Center:
-            start_of_current_line = (cross_size_of_flex_container / 2) - (sum_of_flex_line_cross_sizes / 2);
-            break;
-        case CSS::AlignContent::SpaceBetween: {
+        }
+        break;
+    case CSS::AlignContent::FlexEnd:
+        iterate_lines_backwards = true;
+        if (wrap_reverse) {
+            start_of_current_line = 0;
+        } else {
+            start_of_current_line = cross_size_of_flex_container;
+            place_items_backwards = true;
+        }
+        break;
+    case CSS::AlignContent::Center:
+        iterate_lines_backwards = wrap_reverse;
+        start_of_current_line = (cross_size_of_flex_container / 2) - (sum_of_flex_line_cross_sizes / 2);
+        break;
+    case CSS::AlignContent::SpaceBetween: {
+        if (wrap_reverse) {
+            start_of_current_line = cross_size_of_flex_container;
+            place_items_backwards = true;
+        } else {
             start_of_current_line = 0;
             start_of_current_line = 0;
+        }
 
 
-            auto leftover_free_space = cross_size_of_flex_container - sum_of_flex_line_cross_sizes;
-            auto leftover_flex_lines_size = m_flex_lines.size();
-            if (leftover_free_space >= 0 && leftover_flex_lines_size > 1) {
-                int gap_count = leftover_flex_lines_size - 1;
-                gap_size = leftover_free_space / gap_count;
-            }
+        auto leftover_free_space = cross_size_of_flex_container - sum_of_flex_line_cross_sizes;
+        auto leftover_flex_lines_size = m_flex_lines.size();
+        if (leftover_free_space >= 0 && leftover_flex_lines_size > 1) {
+            int gap_count = leftover_flex_lines_size - 1;
+            gap_size = leftover_free_space / gap_count;
+        }
+        break;
+    }
+    case CSS::AlignContent::SpaceAround: {
+        iterate_lines_backwards = wrap_reverse;
+        auto leftover_free_space = cross_size_of_flex_container - sum_of_flex_line_cross_sizes;
+        if (leftover_free_space < 0) {
+            // If the leftover free-space is negative this value is identical to center.
+            start_of_current_line = (cross_size_of_flex_container / 2) - (sum_of_flex_line_cross_sizes / 2);
             break;
             break;
         }
         }
-        case CSS::AlignContent::SpaceAround: {
-            auto leftover_free_space = cross_size_of_flex_container - sum_of_flex_line_cross_sizes;
-            if (leftover_free_space < 0) {
-                // If the leftover free-space is negative this value is identical to center.
-                start_of_current_line = (cross_size_of_flex_container / 2) - (sum_of_flex_line_cross_sizes / 2);
-                break;
-            }
 
 
-            gap_size = leftover_free_space / m_flex_lines.size();
+        gap_size = leftover_free_space / m_flex_lines.size();
 
 
-            // The spacing between the first/last lines and the flex container edges is half the size of the spacing between flex lines.
-            start_of_current_line = gap_size / 2;
+        // The spacing between the first/last lines and the flex container edges is half the size of the spacing between flex lines.
+        start_of_current_line = gap_size / 2;
+        break;
+    }
+    case CSS::AlignContent::SpaceEvenly: {
+        iterate_lines_backwards = wrap_reverse;
+        auto leftover_free_space = cross_size_of_flex_container - sum_of_flex_line_cross_sizes;
+        if (leftover_free_space < 0) {
+            // If the leftover free-space is negative this value is identical to center.
+            start_of_current_line = (cross_size_of_flex_container / 2) - (sum_of_flex_line_cross_sizes / 2);
             break;
             break;
         }
         }
-        case CSS::AlignContent::SpaceEvenly: {
-            auto leftover_free_space = cross_size_of_flex_container - sum_of_flex_line_cross_sizes;
-            if (leftover_free_space < 0) {
-                // If the leftover free-space is negative this value is identical to center.
-                start_of_current_line = (cross_size_of_flex_container / 2) - (sum_of_flex_line_cross_sizes / 2);
-                break;
-            }
 
 
-            gap_size = leftover_free_space / (m_flex_lines.size() + 1);
+        gap_size = leftover_free_space / (m_flex_lines.size() + 1);
 
 
-            // The spacing between the first/last lines and the flex container edges is the size of the spacing between flex lines.
-            start_of_current_line = gap_size;
-            break;
-        }
+        // The spacing between the first/last lines and the flex container edges is the size of the spacing between flex lines.
+        start_of_current_line = gap_size;
+        break;
+    }
 
 
-        case CSS::AlignContent::Normal:
-        case CSS::AlignContent::Stretch:
+    case CSS::AlignContent::Normal:
+    case CSS::AlignContent::Stretch:
+        if (wrap_reverse) {
+            start_of_current_line = cross_size_of_flex_container;
+            place_items_backwards = true;
+        } else {
             start_of_current_line = 0;
             start_of_current_line = 0;
-            break;
         }
         }
+        break;
+    }
 
 
-        for (auto& flex_line : m_flex_lines) {
+    auto place_items = [&](FlexLine& flex_line) {
+        if (place_items_backwards) {
+            CSSPixels center_of_current_line = start_of_current_line - (flex_line.cross_size / 2);
+            for (auto& item : flex_line.items) {
+                item.cross_offset += center_of_current_line;
+            }
+            start_of_current_line -= flex_line.cross_size + gap_size;
+            // CSS-FLEXBOX-2: Account for gap between flex lines.
+            start_of_current_line -= cross_gap();
+        } else {
             CSSPixels center_of_current_line = start_of_current_line + (flex_line.cross_size / 2);
             CSSPixels center_of_current_line = start_of_current_line + (flex_line.cross_size / 2);
             for (auto& item : flex_line.items) {
             for (auto& item : flex_line.items) {
                 item.cross_offset += center_of_current_line;
                 item.cross_offset += center_of_current_line;
@@ -1612,6 +1654,16 @@ void FlexFormattingContext::align_all_flex_lines()
             // CSS-FLEXBOX-2: Account for gap between flex lines.
             // CSS-FLEXBOX-2: Account for gap between flex lines.
             start_of_current_line += cross_gap();
             start_of_current_line += cross_gap();
         }
         }
+    };
+
+    if (iterate_lines_backwards) {
+        for (auto& flex_line : m_flex_lines.in_reverse()) {
+            place_items(flex_line);
+        }
+    } else {
+        for (auto& flex_line : m_flex_lines) {
+            place_items(flex_line);
+        }
     }
     }
 }
 }