mirror of
https://github.com/RGBCube/serenity
synced 2025-07-25 15:27:35 +00:00
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.
This commit is contained in:
Mathis Wiehl
Andreas Kling
parent
3716e1b8a0
commit
ab4cf7c57d
3 changed files with 92 additions and 43 deletions
|
|
@ -0,0 +1,21 @@
|
|||
Viewport <#document> at (0,0) content-size 800x600 children: not-inline
|
||||
BlockContainer <html> at (0,0) content-size 800x70 children: not-inline
|
||||
BlockContainer <body> at (8,8) content-size 784x54 children: not-inline
|
||||
Box <div.container> at (9,9) content-size 600x52 flex-container(row) children: not-inline
|
||||
BlockContainer <div.box> at (20,10) content-size 150x50 flex-item children: inline
|
||||
line 0 width: 86.359375, height: 17.46875, bottom: 17.46875, baseline: 13.53125
|
||||
frag 0 from TextNode start: 0, length: 11, rect: [20,10 86.359375x17.46875]
|
||||
"left margin"
|
||||
TextNode <#text>
|
||||
BlockContainer <div.box> at (172,10) content-size 150x50 flex-item children: inline
|
||||
line 0 width: 141.28125, height: 17.46875, bottom: 17.46875, baseline: 13.53125
|
||||
frag 0 from TextNode start: 0, length: 18, rect: [172,10 141.28125x17.46875]
|
||||
"follow immediately"
|
||||
TextNode <#text>
|
||||
BlockContainer <div.box> at (458,10) content-size 150x50 flex-item children: inline
|
||||
line 0 width: 138.296875, height: 17.46875, bottom: 17.46875, baseline: 13.53125
|
||||
frag 0 from TextNode start: 0, length: 17, rect: [458,10 138.296875x17.46875]
|
||||
"over at the right"
|
||||
TextNode <#text>
|
||||
BlockContainer <(anonymous)> at (8,62) content-size 784x0 children: inline
|
||||
TextNode <#text>
|
||||
|
|
@ -0,0 +1,27 @@
|
|||
<style>
|
||||
body {
|
||||
font-family: "SerenitySans";
|
||||
}
|
||||
|
||||
.container {
|
||||
display: flex;
|
||||
justify-content: space-between;
|
||||
border: 1px solid salmon;
|
||||
width: 600px;
|
||||
}
|
||||
|
||||
.box {
|
||||
width: 150px;
|
||||
height: 50px;
|
||||
border: 1px solid black;
|
||||
}
|
||||
|
||||
.box:nth-child(1) {
|
||||
margin-left: 10px;
|
||||
}
|
||||
|
||||
.box:nth-child(2) {
|
||||
margin-right: auto;
|
||||
}
|
||||
</style>
|
||||
<div class="container"><div class="box">left margin</div><div class="box">follow immediately</div><div class="box">over at the right</div></div>
|
||||
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue