WebKit Bugzilla
Attachment 358895 Details for
Bug 193346
: [LFC][BFC][MarginCollapsing] Adjust vertical position when box margin collapses through.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-193346-20190111074517.patch (text/plain), 18.41 KB, created by
zalan
on 2019-01-11 07:45:29 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
zalan
Created:
2019-01-11 07:45:29 PST
Size:
18.41 KB
patch
obsolete
>Subversion Revision: 239862 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index 76b62769033d12153a576529ab23321418a49b44..7522bd076351ee53eaae6109db0baf27614f2557 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,30 @@ >+2019-01-10 Zalan Bujtas <zalan@apple.com> >+ >+ [LFC][BFC][MarginCollapsing] Adjust vertical position when box margin collapses through. >+ https://bugs.webkit.org/show_bug.cgi?id=193346 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ If the top and bottom margins of a box are adjoining, then it is possible for margins to collapse through it. >+ In this case, the position of the element depends on its relationship with the other elements whose margins are being collapsed. >+ >+ 1. If the element's margins are collapsed with its parent's top margin, the top border edge of the box is defined to be the same as the parent's. >+ 2. Otherwise, either the element's parent is not taking part in the margin collapsing, or only the parent's bottom margin is involved. >+ The position of the element's top border edge is the same as it would have been if the element had a non-zero bottom border. >+ >+ Test: fast/block/block-only/collapsed-through-with-parent.html >+ >+ * layout/MarginTypes.h: >+ (WebCore::Layout::EstimatedMarginBefore::usedValue const): >+ * layout/blockformatting/BlockFormattingContext.cpp: >+ (WebCore::Layout::BlockFormattingContext::computeEstimatedMarginBefore const): >+ (WebCore::Layout::BlockFormattingContext::computeHeightAndMargin const): >+ (WebCore::Layout::BlockFormattingContext::adjustedVerticalPositionAfterMarginCollapsing const): >+ * layout/blockformatting/BlockFormattingContext.h: >+ * layout/blockformatting/BlockMarginCollapse.cpp: >+ (WebCore::Layout::BlockFormattingContext::MarginCollapse::estimatedMarginBefore): >+ (WebCore::Layout::BlockFormattingContext::MarginCollapse::marginBeforeIgnoringCollapsingThrough): >+ > 2019-01-10 Myles C. Maxfield <mmaxfield@apple.com> > > [WHLSL] Include the standard library >diff --git a/Source/WebCore/layout/MarginTypes.h b/Source/WebCore/layout/MarginTypes.h >index ac4aa8bc5eedb9e571e1adae0ab3c6cdfbc1075d..67b68ab074ac6f8b29a405083e8a079467869249 100644 >--- a/Source/WebCore/layout/MarginTypes.h >+++ b/Source/WebCore/layout/MarginTypes.h >@@ -75,7 +75,10 @@ struct UsedHorizontalMargin { > }; > > struct EstimatedMarginBefore { >- LayoutUnit usedValue; >+ LayoutUnit usedValue() const { return collapsedValue.valueOr(nonCollapsedValue); } >+ LayoutUnit nonCollapsedValue; >+ Optional<LayoutUnit> collapsedValue; >+ bool isCollapsedThrough { false }; > }; > > struct PositiveAndNegativeVerticalMargin { >diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp >index ecd8f87437b4804e9e407dbfdde1b19bc16e5d8f..f36414d8ebc8b329ec2c34f9896d2870bfd21896 100644 >--- a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp >+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp >@@ -191,7 +191,10 @@ void BlockFormattingContext::computeEstimatedMarginBefore(const Box& layoutBox) > blockFormattingState().setHasEstimatedMarginBefore(layoutBox); > > auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox); >- displayBox.setTop(adjustedVerticalPositionAfterMarginCollapsing(layoutBox, estimatedMarginBefore.usedValue)); >+ auto nonCollapsedValues = UsedVerticalMargin::NonCollapsedValues { estimatedMarginBefore.nonCollapsedValue, { } }; >+ auto collapsedValues = UsedVerticalMargin::CollapsedValues { estimatedMarginBefore.collapsedValue, { }, estimatedMarginBefore.isCollapsedThrough }; >+ auto verticalMargin = UsedVerticalMargin { nonCollapsedValues, collapsedValues }; >+ displayBox.setTop(adjustedVerticalPositionAfterMarginCollapsing(layoutBox, verticalMargin)); > #if !ASSERT_DISABLED > displayBox.setEstimatedMarginBefore(estimatedMarginBefore); > #endif >@@ -372,21 +375,33 @@ void BlockFormattingContext::computeHeightAndMargin(const Box& layoutBox) const > } > } > >+ // 1. Compute collapsed margins. >+ // 2. Adjust vertical position using the collaped values >+ // 3. Adjust previous in-flow sibling margin after using this margin. > auto collapsedMargin = MarginCollapse::collapsedVerticalValues(layoutState, layoutBox, heightAndMargin.nonCollapsedMargin); > auto verticalMargin = UsedVerticalMargin { heightAndMargin.nonCollapsedMargin, collapsedMargin }; > auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox); >+ >+ // Out of flow boxes don't need vertical adjustment after margin collapsing. >+ if (layoutBox.isOutOfFlowPositioned()) { >+ displayBox.setContentBoxHeight(heightAndMargin.height); >+ displayBox.setVerticalMargin(verticalMargin); >+ return; >+ } >+ > auto& formattingState = this->blockFormattingState(); > if (formattingState.hasEstimatedMarginBefore(layoutBox)) { > formattingState.clearHasEstimatedMarginBefore(layoutBox); >- ASSERT(displayBox.estimatedMarginBefore()->usedValue == verticalMargin.before()); >+ ASSERT(displayBox.estimatedMarginBefore()->usedValue() == verticalMargin.before()); > } else >- displayBox.setTop(adjustedVerticalPositionAfterMarginCollapsing(layoutBox, verticalMargin.before())); >+ displayBox.setTop(adjustedVerticalPositionAfterMarginCollapsing(layoutBox, verticalMargin)); > >- displayBox.setContentBoxHeight(heightAndMargin.height); >- displayBox.setVerticalMargin(verticalMargin); > // Adjust the previous sibling's margin bottom now that this box's vertical margin is computed. > if (MarginCollapse::marginBeforeCollapsesWithPreviousSiblingMarginAfter(layoutState, layoutBox)) > MarginCollapse::updateCollapsedMarginAfter(layoutState, *layoutBox.previousInFlowSibling(), verticalMargin); >+ >+ displayBox.setContentBoxHeight(heightAndMargin.height); >+ displayBox.setVerticalMargin(verticalMargin); > } > > FormattingContext::InstrinsicWidthConstraints BlockFormattingContext::instrinsicWidthConstraints() const >@@ -453,8 +468,9 @@ FormattingContext::InstrinsicWidthConstraints BlockFormattingContext::instrinsic > return instrinsicWidthConstraints; > } > >-LayoutUnit BlockFormattingContext::adjustedVerticalPositionAfterMarginCollapsing(const Box& layoutBox, LayoutUnit marginBefore) const >+LayoutUnit BlockFormattingContext::adjustedVerticalPositionAfterMarginCollapsing(const Box& layoutBox, const UsedVerticalMargin& verticalMargin) const > { >+ ASSERT(!layoutBox.isOutOfFlowPositioned()); > // Now that we've computed the final margin before, let's shift the box's vertical position. > // 1. Check if the margin before collapses with the previous box's margin after. if not -> return previous box's bottom inlcuding margin after + marginBefore > // 2. Check if the previous box's margins collapse through. If not -> return previous box' bottom excluding margin after + marginBefore (they are supposed to be equal) >@@ -467,26 +483,45 @@ LayoutUnit BlockFormattingContext::adjustedVerticalPositionAfterMarginCollapsing > auto& previousInFlowSibling = *currentLayoutBox->previousInFlowSibling(); > if (!MarginCollapse::marginBeforeCollapsesWithPreviousSiblingMarginAfter(layoutState, *currentLayoutBox)) { > auto& previousDisplayBox = layoutState.displayBoxForLayoutBox(previousInFlowSibling); >- return previousDisplayBox.rectWithMargin().bottom() + marginBefore; >+ return previousDisplayBox.rectWithMargin().bottom() + verticalMargin.before(); > } > > if (!MarginCollapse::marginsCollapseThrough(layoutState, previousInFlowSibling)) { > auto& previousDisplayBox = layoutState.displayBoxForLayoutBox(previousInFlowSibling); >- return previousDisplayBox.bottom() + marginBefore; >+ return previousDisplayBox.bottom() + verticalMargin.before(); > } > currentLayoutBox = &previousInFlowSibling; > } > > auto& containingBlock = *layoutBox.containingBlock(); > auto containingBlockContentBoxTop = layoutState.displayBoxForLayoutBox(containingBlock).contentBoxTop(); >- // If we reached the parent through collapsed sibling(s), this box (may) collapses the before margin indirectly with the parent. >- auto* collapsingCandidate = layoutBox.previousInFlowSibling() ? containingBlock.firstInFlowChild() : &layoutBox; >- auto marginBeforeCollapsesWithParent = MarginCollapse::marginBeforeCollapsesWithParentMarginBefore(layoutState, *collapsingCandidate) >- || MarginCollapse::marginBeforeCollapsesWithParentMarginAfter(layoutState, *collapsingCandidate); >- if (marginBeforeCollapsesWithParent) >+ // Adjust vertical position depending whether this box directly or indirectly adjoins with its parent. >+ auto directlyAdjoinsParent = !layoutBox.previousInFlowSibling(); >+ if (directlyAdjoinsParent) { >+ // If the top and bottom margins of a box are adjoining, then it is possible for margins to collapse through it. >+ // In this case, the position of the element depends on its relationship with the other elements whose margins are being collapsed. >+ if (verticalMargin.collapsedValues().isCollapsedThrough) { >+ // If the element's margins are collapsed with its parent's top margin, the top border edge of the box is defined to be the same as the parent's. >+ if (MarginCollapse::marginBeforeCollapsesWithParentMarginBefore(layoutState, layoutBox)) >+ return containingBlockContentBoxTop; >+ // Otherwise, either the element's parent is not taking part in the margin collapsing, or only the parent's bottom margin is involved. >+ // The position of the element's top border edge is the same as it would have been if the element had a non-zero bottom border. >+ auto beforeMarginWithBottomBorder = MarginCollapse::marginBeforeIgnoringCollapsingThrough(layoutState, layoutBox, verticalMargin.nonCollapsedValues()); >+ return containingBlockContentBoxTop + beforeMarginWithBottomBorder; >+ } >+ // Non-collapsed through box vertical position depending whether the margin collapses. >+ if (MarginCollapse::marginBeforeCollapsesWithParentMarginBefore(layoutState, layoutBox)) >+ return containingBlockContentBoxTop; >+ >+ return containingBlockContentBoxTop + verticalMargin.before(); >+ } >+ // At this point this box indirectly (via collapsed through previous in-flow siblings) adjoins the parent. Let's check if it margin collapses with the parent. >+ ASSERT(containingBlock.firstInFlowChild()); >+ ASSERT(containingBlock.firstInFlowChild() != &layoutBox); >+ if (MarginCollapse::marginBeforeCollapsesWithParentMarginBefore(layoutState, *containingBlock.firstInFlowChild())) > return containingBlockContentBoxTop; > >- return containingBlockContentBoxTop + marginBefore; >+ return containingBlockContentBoxTop + verticalMargin.before(); > } > > } >diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContext.h b/Source/WebCore/layout/blockformatting/BlockFormattingContext.h >index 500d0649c25e237ec00de72fbdeb92b05e758ed5..a2cfdad2017fae77103e8081c311e6f556608e10 100644 >--- a/Source/WebCore/layout/blockformatting/BlockFormattingContext.h >+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContext.h >@@ -69,7 +69,7 @@ private: > void precomputeVerticalPositionForFormattingRootIfNeeded(const Box&) const; > > InstrinsicWidthConstraints instrinsicWidthConstraints() const override; >- LayoutUnit adjustedVerticalPositionAfterMarginCollapsing(const Box&, LayoutUnit marginBefore) const; >+ LayoutUnit adjustedVerticalPositionAfterMarginCollapsing(const Box&, const UsedVerticalMargin&) const; > > // This class implements positioning and sizing for boxes participating in a block formatting context. > class Geometry : public FormattingContext::Geometry { >@@ -95,6 +95,7 @@ private: > static UsedVerticalMargin::CollapsedValues collapsedVerticalValues(const LayoutState&, const Box&, const UsedVerticalMargin::NonCollapsedValues&); > > static EstimatedMarginBefore estimatedMarginBefore(const LayoutState&, const Box&); >+ static LayoutUnit marginBeforeIgnoringCollapsingThrough(const LayoutState&, const Box&, const UsedVerticalMargin::NonCollapsedValues&); > static void updateCollapsedMarginAfter(const LayoutState&, const Box&, const UsedVerticalMargin& nextSiblingVerticalMargin); > > static bool marginBeforeCollapsesWithParentMarginBefore(const LayoutState&, const Box&); >diff --git a/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp b/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp >index f9ab127f9aa48c20d0a7f4fa1e7b4c445ad44c1f..f10ba4641dfe239a00cef9f8fa248315ec7489ad 100644 >--- a/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp >+++ b/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp >@@ -593,13 +593,21 @@ EstimatedMarginBefore BlockFormattingContext::MarginCollapse::estimatedMarginBef > ASSERT(!layoutBox.establishesBlockFormattingContext()); > > auto computedVerticalMargin = Geometry::computedVerticalMargin(layoutState, layoutBox); >- auto nonCollapsedMargin = UsedVerticalMargin::NonCollapsedValues { computedVerticalMargin.before.valueOr(0), computedVerticalMargin.after.valueOr(0) }; >+ auto nonCollapsedMargin = UsedVerticalMargin::NonCollapsedValues { computedVerticalMargin.before.valueOr(0), computedVerticalMargin.after.valueOr(0) }; >+ auto marginsCollapseThrough = MarginCollapse::marginsCollapseThrough(layoutState, layoutBox); >+ auto positiveNegativeMarginBefore = MarginCollapse::positiveNegativeMarginBefore(layoutState, layoutBox, nonCollapsedMargin); > >- if (!marginsCollapseThrough(layoutState, layoutBox)) >- return { marginValue(positiveNegativeMarginBefore(layoutState, layoutBox, nonCollapsedMargin)).valueOr(0) }; >- >- return { marginValue(computedPositiveAndNegativeMargin(positiveNegativeMarginBefore(layoutState, layoutBox, nonCollapsedMargin), >- positiveNegativeMarginAfter(layoutState, layoutBox, nonCollapsedMargin))).valueOr(0) }; >+ auto collapsedMarginBefore = marginValue(!marginsCollapseThrough ? positiveNegativeMarginBefore >+ : computedPositiveAndNegativeMargin(positiveNegativeMarginBefore, positiveNegativeMarginAfter(layoutState, layoutBox, nonCollapsedMargin))); >+ >+ return { nonCollapsedMargin.before, collapsedMarginBefore, marginsCollapseThrough }; >+} >+ >+LayoutUnit BlockFormattingContext::MarginCollapse::marginBeforeIgnoringCollapsingThrough(const LayoutState& layoutState, const Box& layoutBox, const UsedVerticalMargin::NonCollapsedValues& nonCollapsedValues) >+{ >+ ASSERT(!layoutBox.isAnonymous()); >+ ASSERT(layoutBox.isBlockLevelBox()); >+ return marginValue(positiveNegativeMarginBefore(layoutState, layoutBox, nonCollapsedValues)).valueOr(nonCollapsedValues.before); > } > > UsedVerticalMargin::CollapsedValues BlockFormattingContext::MarginCollapse::collapsedVerticalValues(const LayoutState& layoutState, const Box& layoutBox, const UsedVerticalMargin::NonCollapsedValues& nonCollapsedValues) >diff --git a/Tools/ChangeLog b/Tools/ChangeLog >index 2e3a51c702ebebdbb8a3a13739c60922f35c8c36..03e98ad0f255163e845cf8ef55957599842a1570 100644 >--- a/Tools/ChangeLog >+++ b/Tools/ChangeLog >@@ -1,3 +1,12 @@ >+2019-01-10 Zalan Bujtas <zalan@apple.com> >+ >+ [LFC][BFC][MarginCollapsing] Adjust vertical position when box margin collapses through. >+ https://bugs.webkit.org/show_bug.cgi?id=193346 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ * LayoutReloaded/misc/LFC-passing-tests.txt: >+ > 2019-01-10 Ryan Haddad <ryanhaddad@apple.com> > > Unreviewed, fix typo that breaks dashboard link. >diff --git a/Tools/LayoutReloaded/misc/LFC-passing-tests.txt b/Tools/LayoutReloaded/misc/LFC-passing-tests.txt >index e3c8518416d32bde445b4d2ab9ecd97016906d2b..d7d55a646428beb28fb214787e7b4ca4c12fe842 100644 >--- a/Tools/LayoutReloaded/misc/LFC-passing-tests.txt >+++ b/Tools/LayoutReloaded/misc/LFC-passing-tests.txt >@@ -64,6 +64,7 @@ fast/block/block-only/margin-propagation-simple-content-height.html > fast/block/block-only/margin-sibling-collapse-propagated.html > fast/block/block-only/margin-simple.html > fast/block/block-only/collapsed-through-siblings.html >+fast/block/block-only/collapsed-through-with-parent.html > fast/block/block-only/min-max-height-percentage.html > fast/block/block-only/negative-margin-simple.html > fast/block/block-only/padding-nested.html >diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog >index a6e591f84fdc65f7ad0699f850d637b0a1af4794..a79322d4297cd841ac4456cb554dd61287f211c5 100644 >--- a/LayoutTests/ChangeLog >+++ b/LayoutTests/ChangeLog >@@ -1,3 +1,13 @@ >+2019-01-10 Zalan Bujtas <zalan@apple.com> >+ >+ [LFC][BFC][MarginCollapsing] Adjust vertical position when box margin collapses through. >+ https://bugs.webkit.org/show_bug.cgi?id=193346 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ * fast/block/block-only/collapsed-through-with-parent-expected.txt: Added. >+ * fast/block/block-only/collapsed-through-with-parent.html: Added. >+ > 2019-01-11 Carlos Garcia Campos <cgarcia@igalia.com> > > Unreviewed GTK gardening. Rebaseline several tests after r239822. >diff --git a/LayoutTests/fast/block/block-only/collapsed-through-with-parent-expected.txt b/LayoutTests/fast/block/block-only/collapsed-through-with-parent-expected.txt >new file mode 100644 >index 0000000000000000000000000000000000000000..61050a58e40aa6b46ba6c4e2d71e39eeced08842 >--- /dev/null >+++ b/LayoutTests/fast/block/block-only/collapsed-through-with-parent-expected.txt >@@ -0,0 +1,8 @@ >+layer at (0,0) size 800x600 >+ RenderView at (0,0) size 800x600 >+layer at (0,0) size 800x600 >+ RenderBlock {HTML} at (0,0) size 800x600 >+ RenderBody {BODY} at (8,8) size 784x584 >+ RenderBlock {DIV} at (0,0) size 402x402 [border: (1px solid #FF0000)] >+ RenderBlock {DIV} at (1,11) size 100x0 >+ RenderBlock {DIV} at (1,81) size 100x0 >diff --git a/LayoutTests/fast/block/block-only/collapsed-through-with-parent.html b/LayoutTests/fast/block/block-only/collapsed-through-with-parent.html >new file mode 100644 >index 0000000000000000000000000000000000000000..0a5d4d89133de7dd170266ff79b3af359463c06a >--- /dev/null >+++ b/LayoutTests/fast/block/block-only/collapsed-through-with-parent.html >@@ -0,0 +1,4 @@ >+<div style="border: 1px solid red; width: 400px; height: 400px"> >+ <div style="outline: 2px solid red; width: 100px; margin-top: 10px; margin-bottom: 70px"></div> >+ <div style="outline: 1px solid green; width: 100px; margin-top: 80px"></div> >+</div>
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Flags:
koivisto
:
review+
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 193346
:
358876
| 358895