Skip to content

Commit 9c6dfde

Browse files
alanbaradlayalanbaradlay
committed
No gap between the hero image and text on amazon product page
https://bugs.webkit.org/show_bug.cgi?id=249871 <rdar://problem/103685984> Reviewed by Antti Koivisto. Only "margin: auto" is supposed to be resolved against the "available width adjusted with intrusive floats" (e.g. percent values are based on container width). (see webkit.org/b/83614) * LayoutTests/fast/block/percent-margin-start-end-with-floats-expected.html: Added. * LayoutTests/fast/block/percent-margin-start-end-with-floats.html: Added. * Source/WebCore/rendering/GridLayoutFunctions.cpp: (WebCore::GridLayoutFunctions::computeMarginLogicalSizeForChild): * Source/WebCore/rendering/RenderBox.cpp: (WebCore::RenderBox::computeLogicalWidthInFragment const): (WebCore::RenderBox::computeInlineDirectionMargins const): (WebCore::RenderBox::computeLogicalHeight const): * Source/WebCore/rendering/RenderBox.h: * Source/WebCore/rendering/RenderFlexibleBox.cpp: (WebCore::RenderFlexibleBox::mainAxisMarginExtentForChild const): (WebCore::RenderFlexibleBox::crossAxisMarginExtentForChild const): * Source/WebCore/rendering/RenderTable.cpp: (WebCore::RenderTable::updateLogicalWidth): Canonical link: https://commits.webkit.org/259125@main
1 parent 747785f commit 9c6dfde

File tree

7 files changed

+94
-46
lines changed

7 files changed

+94
-46
lines changed

‎LayoutTests/fast/block/percent-margin-start-end-with-floats-expected.html‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<style>
2+
div {
3+
width: 50px;
4+
height: 50px;
5+
}
6+
7+
.left {
8+
background-color: green;
9+
}
10+
.right {
11+
position: relative;
12+
top: -50px;
13+
left: 100px;
14+
background-color: blue;
15+
}
16+
</style>
17+
<div class=left></div><div class=right></div>

‎LayoutTests/fast/block/percent-margin-start-end-with-floats.html‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<style>
2+
.container {
3+
width: 100px;
4+
}
5+
.left {
6+
width: 50px;
7+
height: 50px;
8+
9+
float: left;
10+
background-color: green;
11+
}
12+
.right {
13+
width: 50px;
14+
height: 50px;
15+
16+
/* 100% of the 100px container */
17+
margin-left: 100%;
18+
overflow: hidden;
19+
background-color: blue;
20+
}
21+
</style>
22+
<div class=container>
23+
<div class=left></div>
24+
<div class=right></div>
25+
</div>

‎Source/WebCore/rendering/GridLayoutFunctions.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ LayoutUnit computeMarginLogicalSizeForChild(const RenderGrid& grid, GridTrackSiz
6060
LayoutUnit marginStart;
6161
LayoutUnit marginEnd;
6262
if (direction == ForColumns)
63-
child.computeInlineDirectionMargins(grid, child.containingBlockLogicalWidthForContentInFragment(nullptr), child.logicalWidth(), marginStart, marginEnd);
63+
child.computeInlineDirectionMargins(grid, child.containingBlockLogicalWidthForContentInFragment(nullptr), { }, child.logicalWidth(), marginStart, marginEnd);
6464
else
6565
child.computeBlockDirectionMargins(grid, marginStart, marginEnd);
6666
return marginStartIsAuto(child, flowAwareDirection) ? marginEnd : marginEndIsAuto(child, flowAwareDirection) ? marginStart : marginStart + marginEnd;

‎Source/WebCore/rendering/RenderBox.cpp‎

Lines changed: 47 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -2698,7 +2698,7 @@ void RenderBox::computeLogicalWidthInFragment(LogicalExtentComputedValues& compu
26982698
if (avoidsFloats() && cb.containsFloats())
26992699
containerLogicalWidthForAutoMargins = containingBlockAvailableLineWidthInFragment(fragment);
27002700
bool hasInvertedDirection = cb.style().isLeftToRightDirection() != style().isLeftToRightDirection();
2701-
computeInlineDirectionMargins(cb, containerLogicalWidthForAutoMargins, computedValues.m_extent,
2701+
computeInlineDirectionMargins(cb, containerLogicalWidth, containerLogicalWidthForAutoMargins, computedValues.m_extent,
27022702
hasInvertedDirection ? computedValues.m_margins.m_end : computedValues.m_margins.m_start,
27032703
hasInvertedDirection ? computedValues.m_margins.m_start : computedValues.m_margins.m_end);
27042704
}
@@ -2944,9 +2944,8 @@ LayoutUnit RenderBox::computeOrTrimInlineMargin(const RenderBlock& containingBlo
29442944
return computeInlineMargin();
29452945
}
29462946

2947-
void RenderBox::computeInlineDirectionMargins(const RenderBlock& containingBlock, LayoutUnit containerWidth, LayoutUnit childWidth, LayoutUnit& marginStart, LayoutUnit& marginEnd) const
2947+
void RenderBox::computeInlineDirectionMargins(const RenderBlock& containingBlock, LayoutUnit containerWidth, std::optional<LayoutUnit> availableSpaceAdjustedWithFloats, LayoutUnit childWidth, LayoutUnit& marginStart, LayoutUnit& marginEnd) const
29482948
{
2949-
29502949
const RenderStyle& containingBlockStyle = containingBlock.style();
29512950
Length marginStartLength = style().marginStartUsing(&containingBlockStyle);
29522951
Length marginEndLength = style().marginEndUsing(&containingBlockStyle);
@@ -2972,44 +2971,51 @@ void RenderBox::computeInlineDirectionMargins(const RenderBlock& containingBlock
29722971
marginEndLength = Length(0, LengthType::Fixed);
29732972
}
29742973

2975-
// Case One: The object is being centered in the containing block's available logical width.
2976-
if ((marginStartLength.isAuto() && marginEndLength.isAuto() && childWidth < containerWidth)
2977-
|| (!marginStartLength.isAuto() && !marginEndLength.isAuto() && containingBlock.style().textAlign() == TextAlignMode::WebKitCenter)) {
2978-
// Other browsers center the margin box for align=center elements so we match them here.
2979-
marginStart = computeOrTrimInlineMargin(containingBlock, MarginTrimType::InlineStart, [containerWidth, childWidth, marginStartLength, marginEndLength]() {
2980-
LayoutUnit marginStartWidth = minimumValueForLength(marginStartLength, containerWidth);
2981-
LayoutUnit marginEndWidth = minimumValueForLength(marginEndLength, containerWidth);
2982-
LayoutUnit centeredMarginBoxStart = std::max<LayoutUnit>(0, (containerWidth - childWidth - marginStartWidth - marginEndWidth) / 2);
2983-
return centeredMarginBoxStart + marginStartWidth;
2984-
});
2985-
marginEnd = computeOrTrimInlineMargin(containingBlock, MarginTrimType::InlineEnd, [containerWidth, childWidth, marginEndLength, marginStart]() {
2986-
LayoutUnit marginEndWidth = minimumValueForLength(marginEndLength, containerWidth);
2987-
return containerWidth - childWidth - marginStart + marginEndWidth;
2988-
});
2989-
return;
2990-
}
2991-
2992-
// Case Two: The object is being pushed to the start of the containing block's available logical width.
2993-
if (marginEndLength.isAuto() && childWidth < containerWidth) {
2994-
marginStart = valueForLength(marginStartLength, containerWidth);
2995-
marginEnd = containerWidth - childWidth - marginStart;
2996-
return;
2997-
}
2998-
2999-
// Case Three: The object is being pushed to the end of the containing block's available logical width.
3000-
bool pushToEndFromTextAlign = !marginEndLength.isAuto() && ((!containingBlockStyle.isLeftToRightDirection() && containingBlockStyle.textAlign() == TextAlignMode::WebKitLeft)
3001-
|| (containingBlockStyle.isLeftToRightDirection() && containingBlockStyle.textAlign() == TextAlignMode::WebKitRight));
3002-
if ((marginStartLength.isAuto() || pushToEndFromTextAlign) && childWidth < containerWidth) {
3003-
marginEnd = computeOrTrimInlineMargin(containingBlock, MarginTrimType::InlineEnd, [containerWidth, marginEndLength]() {
3004-
return valueForLength(marginEndLength, containerWidth);
3005-
});
3006-
marginStart = computeOrTrimInlineMargin(containingBlock, MarginTrimType::InlineStart, [containerWidth, childWidth, marginEnd]() {
3007-
return containerWidth - childWidth - marginEnd;
3008-
});
2974+
auto handleMarginAuto = [&] {
2975+
auto containerWidthForMarginAuto = availableSpaceAdjustedWithFloats.value_or(containerWidth);
2976+
// Case One: The object is being centered in the containing block's available logical width.
2977+
auto marginAutoCenter = marginStartLength.isAuto() && marginEndLength.isAuto() && childWidth < containerWidthForMarginAuto;
2978+
auto alignModeCenter = containingBlock.style().textAlign() == TextAlignMode::WebKitCenter && !marginStartLength.isAuto() && !marginEndLength.isAuto();
2979+
if (marginAutoCenter || alignModeCenter) {
2980+
// Other browsers center the margin box for align=center elements so we match them here.
2981+
marginStart = computeOrTrimInlineMargin(containingBlock, MarginTrimType::InlineStart, [containerWidthForMarginAuto, childWidth, marginStartLength, marginEndLength]() {
2982+
LayoutUnit marginStartWidth = minimumValueForLength(marginStartLength, containerWidthForMarginAuto);
2983+
LayoutUnit marginEndWidth = minimumValueForLength(marginEndLength, containerWidthForMarginAuto);
2984+
LayoutUnit centeredMarginBoxStart = std::max<LayoutUnit>(0, (containerWidthForMarginAuto - childWidth - marginStartWidth - marginEndWidth) / 2);
2985+
return centeredMarginBoxStart + marginStartWidth;
2986+
});
2987+
marginEnd = computeOrTrimInlineMargin(containingBlock, MarginTrimType::InlineEnd, [containerWidthForMarginAuto, childWidth, marginEndLength, marginStart]() {
2988+
LayoutUnit marginEndWidth = minimumValueForLength(marginEndLength, containerWidthForMarginAuto);
2989+
return containerWidthForMarginAuto - childWidth - marginStart + marginEndWidth;
2990+
});
2991+
return true;
2992+
}
2993+
2994+
// Case Two: The object is being pushed to the start of the containing block's available logical width.
2995+
if (marginEndLength.isAuto() && childWidth < containerWidthForMarginAuto) {
2996+
marginStart = valueForLength(marginStartLength, containerWidthForMarginAuto);
2997+
marginEnd = containerWidthForMarginAuto - childWidth - marginStart;
2998+
return true;
2999+
}
3000+
3001+
// Case Three: The object is being pushed to the end of the containing block's available logical width.
3002+
auto pushToEndFromTextAlign = !marginEndLength.isAuto() && ((!containingBlockStyle.isLeftToRightDirection() && containingBlockStyle.textAlign() == TextAlignMode::WebKitLeft)
3003+
|| (containingBlockStyle.isLeftToRightDirection() && containingBlockStyle.textAlign() == TextAlignMode::WebKitRight));
3004+
if ((marginStartLength.isAuto() || pushToEndFromTextAlign) && childWidth < containerWidthForMarginAuto) {
3005+
marginEnd = computeOrTrimInlineMargin(containingBlock, MarginTrimType::InlineEnd, [containerWidthForMarginAuto, marginEndLength]() {
3006+
return valueForLength(marginEndLength, containerWidthForMarginAuto);
3007+
});
3008+
marginStart = computeOrTrimInlineMargin(containingBlock, MarginTrimType::InlineStart, [containerWidthForMarginAuto, childWidth, marginEnd]() {
3009+
return containerWidthForMarginAuto - childWidth - marginEnd;
3010+
});
3011+
return true;
3012+
}
3013+
return false;
3014+
};
3015+
if (handleMarginAuto())
30093016
return;
3010-
}
30113017

3012-
// Case Four: Either no auto margins, or our width is >= the container width (css2.1, 10.3.3). In that case
3018+
// Case Four: Either no auto margins, or our width is >= the container width (css2.1, 10.3.3). In that case
30133019
// auto margins will just turn into 0.
30143020
marginStart = computeOrTrimInlineMargin(containingBlock, MarginTrimType::InlineStart, [containerWidth, marginStartLength] {
30153021
return minimumValueForLength(marginStartLength, containerWidth);
@@ -3132,7 +3138,7 @@ RenderBox::LogicalExtentComputedValues RenderBox::computeLogicalHeight(LayoutUni
31323138
computedValues.m_extent = blockSizeFromAspectRatio(horizontalBorderAndPaddingExtent(), verticalBorderAndPaddingExtent(), style().logicalAspectRatio(), style().boxSizingForAspectRatio(), logicalWidth(), style().aspectRatioType(), isRenderReplaced());
31333139
if (hasPerpendicularContainingBlock) {
31343140
bool shouldFlipBeforeAfter = shouldFlipBeforeAfterMargins(cb.style(), &style());
3135-
computeInlineDirectionMargins(cb, containingBlockLogicalWidthForContent(), computedValues.m_extent,
3141+
computeInlineDirectionMargins(cb, containingBlockLogicalWidthForContent(), { }, computedValues.m_extent,
31363142
shouldFlipBeforeAfter ? computedValues.m_margins.m_after : computedValues.m_margins.m_before,
31373143
shouldFlipBeforeAfter ? computedValues.m_margins.m_before : computedValues.m_margins.m_after);
31383144
}
@@ -3195,7 +3201,7 @@ RenderBox::LogicalExtentComputedValues RenderBox::computeLogicalHeight(LayoutUni
31953201

31963202
if (hasPerpendicularContainingBlock) {
31973203
bool shouldFlipBeforeAfter = shouldFlipBeforeAfterMargins(cb.style(), &style());
3198-
computeInlineDirectionMargins(cb, containingBlockLogicalWidthForContent(), heightResult,
3204+
computeInlineDirectionMargins(cb, containingBlockLogicalWidthForContent(), { }, heightResult,
31993205
shouldFlipBeforeAfter ? computedValues.m_margins.m_after : computedValues.m_margins.m_before,
32003206
shouldFlipBeforeAfter ? computedValues.m_margins.m_before : computedValues.m_margins.m_after);
32013207
}

‎Source/WebCore/rendering/RenderBox.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ class RenderBox : public RenderBoxModelObject {
392392
};
393393
// Resolve auto margins in the inline direction of the containing block so that objects can be pushed to the start, middle or end
394394
// of the containing block.
395-
void computeInlineDirectionMargins(const RenderBlock& containingBlock, LayoutUnit containerWidth, LayoutUnit childWidth, LayoutUnit& marginStart, LayoutUnit& marginEnd) const;
395+
void computeInlineDirectionMargins(const RenderBlock& containingBlock, LayoutUnit containerWidth, std::optional<LayoutUnit> availableSpaceAdjustedWithFloats, LayoutUnit childWidth, LayoutUnit& marginStart, LayoutUnit& marginEnd) const;
396396
LayoutUnit computeOrTrimInlineMargin(const RenderBlock& containingBlock, MarginTrimType marginSide, std::function<LayoutUnit()> computeInlineMargin) const;
397397

398398
// Used to resolve margins in the containing block's block-flow direction.

‎Source/WebCore/rendering/RenderFlexibleBox.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -910,7 +910,7 @@ LayoutUnit RenderFlexibleBox::mainAxisMarginExtentForChild(const RenderBox& chil
910910
LayoutUnit marginStart;
911911
LayoutUnit marginEnd;
912912
if (isHorizontalFlow())
913-
child.computeInlineDirectionMargins(*this, child.containingBlockLogicalWidthForContentInFragment(nullptr), child.logicalWidth(), marginStart, marginEnd);
913+
child.computeInlineDirectionMargins(*this, child.containingBlockLogicalWidthForContentInFragment(nullptr), child.logicalWidth(), { }, marginStart, marginEnd);
914914
else
915915
child.computeBlockDirectionMargins(*this, marginStart, marginEnd);
916916
return marginStart + marginEnd;
@@ -926,7 +926,7 @@ LayoutUnit RenderFlexibleBox::crossAxisMarginExtentForChild(const RenderBox& chi
926926
if (isHorizontalFlow())
927927
child.computeBlockDirectionMargins(*this, marginStart, marginEnd);
928928
else
929-
child.computeInlineDirectionMargins(*this, child.containingBlockLogicalWidthForContentInFragment(nullptr), child.logicalWidth(), marginStart, marginEnd);
929+
child.computeInlineDirectionMargins(*this, child.containingBlockLogicalWidthForContentInFragment(nullptr), child.logicalWidth(), { }, marginStart, marginEnd);
930930
return marginStart + marginEnd;
931931
}
932932

‎Source/WebCore/rendering/RenderTable.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ void RenderTable::updateLogicalWidth()
319319
containerLogicalWidthForAutoMargins = containingBlockAvailableLineWidthInFragment(0); // FIXME: Work with regions someday.
320320
ComputedMarginValues marginValues;
321321
bool hasInvertedDirection = cb.style().isLeftToRightDirection() == style().isLeftToRightDirection();
322-
computeInlineDirectionMargins(cb, containerLogicalWidthForAutoMargins, logicalWidth(),
322+
computeInlineDirectionMargins(cb, availableLogicalWidth, containerLogicalWidthForAutoMargins, logicalWidth(),
323323
hasInvertedDirection ? marginValues.m_start : marginValues.m_end,
324324
hasInvertedDirection ? marginValues.m_end : marginValues.m_start);
325325
setMarginStart(marginValues.m_start);

0 commit comments

Comments
 (0)