WebKit Bugzilla
Attachment 356445 Details for
Bug 192345
: [LFC][BFC][MarginCollapsing] HeightAndMargin::margin is always the non-collapsed margin value.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
Patch.txt (text/plain), 10.77 KB, created by
zalan
on 2018-12-03 19:02:16 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
zalan
Created:
2018-12-03 19:02:16 PST
Size:
10.77 KB
patch
obsolete
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index 6d9eef69724..01aee25f18f 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,26 @@ >+2018-12-03 Zalan Bujtas <zalan@apple.com> >+ >+ [LFC][BFC][MarginCollapsing] HeightAndMargin::margin is always the non-collapsed margin value. >+ https://bugs.webkit.org/show_bug.cgi?id=192345 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Rename HeightAndMargin::margin to HeightAndMargin::nonCollapsedMargin. >+ >+ * layout/FormattingContext.cpp: >+ (WebCore::Layout::FormattingContext::computeOutOfFlowVerticalGeometry const): >+ * layout/LayoutUnits.h: >+ (WebCore::Layout::HeightAndMargin::usedMarginValues const): >+ * layout/blockformatting/BlockFormattingContext.cpp: >+ (WebCore::Layout::BlockFormattingContext::computeHeightAndMargin const): >+ * layout/blockformatting/BlockFormattingContextGeometry.cpp: >+ (WebCore::Layout::BlockFormattingContext::Geometry::inFlowNonReplacedHeightAndMargin): >+ (WebCore::Layout::BlockFormattingContext::Geometry::inFlowHeightAndMargin): >+ * layout/blockformatting/BlockFormattingContextQuirks.cpp: >+ (WebCore::Layout::BlockFormattingContext::Quirks::stretchedHeight): >+ * layout/inlineformatting/InlineFormattingContext.cpp: >+ (WebCore::Layout::InlineFormattingContext::computeHeightAndMargin const): >+ > 2018-12-02 Zalan Bujtas <zalan@apple.com> > > [LFC][BFC][MarginCollapsing] Add MarginCollapse::establishesBlockFormattingContext >diff --git a/Source/WebCore/layout/FormattingContext.cpp b/Source/WebCore/layout/FormattingContext.cpp >index 0f8b02c8c40..ff41200ae7b 100644 >--- a/Source/WebCore/layout/FormattingContext.cpp >+++ b/Source/WebCore/layout/FormattingContext.cpp >@@ -115,11 +115,13 @@ void FormattingContext::computeOutOfFlowVerticalGeometry(const Box& layoutBox) c > } > > auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox); >- displayBox.setTop(verticalGeometry.top + verticalGeometry.heightAndMargin.margin.top); >- displayBox.setContentBoxHeight(verticalGeometry.heightAndMargin.height); >+ // Margins of absolutely positioned boxes do not collapse > ASSERT(!verticalGeometry.heightAndMargin.collapsedMargin); >- displayBox.setVerticalMargin(verticalGeometry.heightAndMargin.margin); >- displayBox.setVerticalNonCollapsedMargin(verticalGeometry.heightAndMargin.margin); >+ auto nonCollapsedVerticalMargins = verticalGeometry.heightAndMargin.usedMarginValues(); >+ displayBox.setTop(verticalGeometry.top + nonCollapsedVerticalMargins.top); >+ displayBox.setContentBoxHeight(verticalGeometry.heightAndMargin.height); >+ displayBox.setVerticalMargin(nonCollapsedVerticalMargins); >+ displayBox.setVerticalNonCollapsedMargin(nonCollapsedVerticalMargins); > } > > void FormattingContext::computeBorderAndPadding(const Box& layoutBox) const >diff --git a/Source/WebCore/layout/LayoutUnits.h b/Source/WebCore/layout/LayoutUnits.h >index b1c3d521d45..3e1d8446f75 100644 >--- a/Source/WebCore/layout/LayoutUnits.h >+++ b/Source/WebCore/layout/LayoutUnits.h >@@ -107,10 +107,10 @@ struct WidthAndMargin { > }; > > struct HeightAndMargin { >- VerticalEdges usedMarginValues() const { return collapsedMargin.value_or(margin); } >+ VerticalEdges usedMarginValues() const { return collapsedMargin.value_or(nonCollapsedMargin); } > > LayoutUnit height; >- VerticalEdges margin; >+ VerticalEdges nonCollapsedMargin; > std::optional<VerticalEdges> collapsedMargin; > }; > >diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp >index f80e31ff2dd..26a82b73354 100644 >--- a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp >+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp >@@ -358,7 +358,7 @@ void BlockFormattingContext::computeHeightAndMargin(const Box& layoutBox) const > auto maxHeightAndMargin = compute(maxHeight); > // Used height should remain the same. > ASSERT((layoutState.inQuirksMode() && (layoutBox.isBodyBox() || layoutBox.isDocumentBox())) || maxHeightAndMargin.height == *maxHeight); >- heightAndMargin = { *maxHeight, maxHeightAndMargin.margin, maxHeightAndMargin.collapsedMargin }; >+ heightAndMargin = { *maxHeight, maxHeightAndMargin.nonCollapsedMargin, maxHeightAndMargin.collapsedMargin }; > } > } > >@@ -367,18 +367,18 @@ void BlockFormattingContext::computeHeightAndMargin(const Box& layoutBox) const > auto minHeightAndMargin = compute(minHeight); > // Used height should remain the same. > ASSERT((layoutState.inQuirksMode() && (layoutBox.isBodyBox() || layoutBox.isDocumentBox())) || minHeightAndMargin.height == *minHeight); >- heightAndMargin = { *minHeight, minHeightAndMargin.margin, minHeightAndMargin.collapsedMargin }; >+ heightAndMargin = { *minHeight, minHeightAndMargin.nonCollapsedMargin, minHeightAndMargin.collapsedMargin }; > } > } > > auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox); > displayBox.setContentBoxHeight(heightAndMargin.height); >- displayBox.setVerticalNonCollapsedMargin(heightAndMargin.margin); >- displayBox.setVerticalMargin(heightAndMargin.collapsedMargin.value_or(heightAndMargin.margin)); >+ displayBox.setVerticalNonCollapsedMargin(heightAndMargin.nonCollapsedMargin); >+ displayBox.setVerticalMargin(heightAndMargin.usedMarginValues()); > > // If this box has already been moved by the estimated vertical margin, no need to move it again. > if (layoutBox.isFloatingPositioned() || !displayBox.estimatedMarginTop()) >- displayBox.moveVertically(heightAndMargin.collapsedMargin.value_or(heightAndMargin.margin).top); >+ displayBox.moveVertically(heightAndMargin.usedMarginValues().top); > } > > FormattingContext::InstrinsicWidthConstraints BlockFormattingContext::instrinsicWidthConstraints() const >diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp >index 819f0303e09..3b09f30cf1b 100644 >--- a/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp >+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp >@@ -104,7 +104,7 @@ HeightAndMargin BlockFormattingContext::Geometry::inFlowNonReplacedHeightAndMarg > > auto heightAndMargin = compute(); > >- LOG_WITH_STREAM(FormattingContextLayout, stream << "[Height][Margin] -> inflow non-replaced -> height(" << heightAndMargin.height << "px) margin(" << heightAndMargin.margin.top << "px, " << heightAndMargin.margin.bottom << "px) -> layoutBox(" << &layoutBox << ")"); >+ LOG_WITH_STREAM(FormattingContextLayout, stream << "[Height][Margin] -> inflow non-replaced -> height(" << heightAndMargin.height << "px) margin(" << heightAndMargin.usedMarginValues().top << "px, " << heightAndMargin.usedMarginValues().bottom << "px) -> layoutBox(" << &layoutBox << ")"); > return heightAndMargin; > } > >@@ -263,7 +263,7 @@ HeightAndMargin BlockFormattingContext::Geometry::inFlowHeightAndMargin(const La > > heightAndMargin = Quirks::stretchedHeight(layoutState, layoutBox, heightAndMargin); > >- LOG_WITH_STREAM(FormattingContextLayout, stream << "[Height][Margin] -> inflow non-replaced -> streched to viewport -> height(" << heightAndMargin.height << "px) margin(" << heightAndMargin.margin.top << "px, " << heightAndMargin.margin.bottom << "px) -> layoutBox(" << &layoutBox << ")"); >+ LOG_WITH_STREAM(FormattingContextLayout, stream << "[Height][Margin] -> inflow non-replaced -> streched to viewport -> height(" << heightAndMargin.height << "px) margin(" << heightAndMargin.usedMarginValues().top << "px, " << heightAndMargin.usedMarginValues().bottom << "px) -> layoutBox(" << &layoutBox << ")"); > return heightAndMargin; > } > >diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContextQuirks.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContextQuirks.cpp >index 2ab92e89204..9372cbc02d1 100644 >--- a/Source/WebCore/layout/blockformatting/BlockFormattingContextQuirks.cpp >+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContextQuirks.cpp >@@ -79,9 +79,12 @@ HeightAndMargin BlockFormattingContext::Quirks::stretchedHeight(const LayoutStat > strechedHeight -= documentBoxVerticalBorders + documentBoxVerticalPaddings; > > LayoutUnit totalVerticalMargins; >- if (layoutBox.isDocumentBox()) >- totalVerticalMargins = heightAndMargin.margin.top + heightAndMargin.margin.bottom; >- else if (layoutBox.isBodyBox()) { >+ if (layoutBox.isDocumentBox()) { >+ auto verticalMargins = heightAndMargin.usedMarginValues(); >+ // Document box's margins do not collapse. >+ ASSERT(!heightAndMargin.collapsedMargin); >+ totalVerticalMargins = verticalMargins.top + verticalMargins.bottom; >+ } else if (layoutBox.isBodyBox()) { > // Here is the quirky part for body box: > // Stretch the body using the initial containing block's height and shrink it with document box's margin/border/padding. > // This looks extremely odd when html has non-auto height. >@@ -90,7 +93,7 @@ HeightAndMargin BlockFormattingContext::Quirks::stretchedHeight(const LayoutStat > > // This quirk happens when the body height is 0 which means its vertical margins collapse through (top and bottom margins are adjoining). > // However now that we stretch the body they don't collapse through anymore, so we need to use the non-collapsed values instead. >- auto bodyBoxVerticalMargins = heightAndMargin.height ? heightAndMargin.usedMarginValues() : heightAndMargin.margin; >+ auto bodyBoxVerticalMargins = heightAndMargin.height ? heightAndMargin.usedMarginValues() : heightAndMargin.nonCollapsedMargin; > totalVerticalMargins = bodyBoxVerticalMargins.top + bodyBoxVerticalMargins.bottom; > } > >diff --git a/Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp b/Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp >index 32012c3ca2a..b530ad6f4c1 100644 >--- a/Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp >+++ b/Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp >@@ -362,8 +362,8 @@ void InlineFormattingContext::computeHeightAndMargin(const Box& layoutBox) const > > auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox); > displayBox.setContentBoxHeight(heightAndMargin.height); >- displayBox.setVerticalNonCollapsedMargin(heightAndMargin.margin); >- displayBox.setVerticalMargin(heightAndMargin.collapsedMargin.value_or(heightAndMargin.margin)); >+ displayBox.setVerticalNonCollapsedMargin(heightAndMargin.nonCollapsedMargin); >+ displayBox.setVerticalMargin(heightAndMargin.usedMarginValues()); > } > > void InlineFormattingContext::layoutFormattingContextRoot(const Box& root) const
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 192345
: 356445