- Revision
- 233350
- Author
- za...@apple.com
- Date
- 2018-06-29 00:55:53 -0700 (Fri, 29 Jun 2018)
Log Message
[LFC] The static position for an out-of-flow box should include the previous sibling's collapsed margin
https://bugs.webkit.org/show_bug.cgi?id=187169
Reviewed by Antti Koivisto.
When computing the static position of an absolutely positioned box, we need to look at the previous sibling's bottom margin.
If the previous sibling happens to collapse its bottom margin with the parent's bottom margin, we still need to account for it
and compute the static vertical position as if the bottom margin was not collapsed.
* layout/FormattingContext.cpp:
(WebCore::Layout::FormattingContext::computeFloatingHeightAndMargin const):
(WebCore::Layout::FormattingContext::computeOutOfFlowVerticalGeometry const):
* layout/FormattingContextGeometry.cpp:
(WebCore::Layout::staticVerticalPositionForOutOfFlowPositioned):
* layout/LayoutContext.cpp:
(WebCore::Layout::LayoutContext::initializeRoot):
* layout/Verification.cpp:
(WebCore::Layout::outputMismatchingBoxInformationIfNeeded):
* layout/blockformatting/BlockFormattingContext.cpp:
(WebCore::Layout::BlockFormattingContext::computeInFlowHeightAndMargin const):
* layout/displaytree/DisplayBox.cpp:
(WebCore::Display::Box::nonCollapsedMarginBox const):
* layout/displaytree/DisplayBox.h:
(WebCore::Display::Box::setHasValidVerticalNonCollapsedMargin):
(WebCore::Display::Box::setVerticalMargin):
(WebCore::Display::Box::setVerticalNonCollapsedMargin):
(WebCore::Display::Box::nonCollapsedMarginTop const):
(WebCore::Display::Box::nonCollapsedMarginBottom const):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (233349 => 233350)
--- trunk/Source/WebCore/ChangeLog 2018-06-29 05:45:34 UTC (rev 233349)
+++ trunk/Source/WebCore/ChangeLog 2018-06-29 07:55:53 UTC (rev 233350)
@@ -1,3 +1,34 @@
+2018-06-29 Zalan Bujtas <za...@apple.com>
+
+ [LFC] The static position for an out-of-flow box should include the previous sibling's collapsed margin
+ https://bugs.webkit.org/show_bug.cgi?id=187169
+
+ Reviewed by Antti Koivisto.
+
+ When computing the static position of an absolutely positioned box, we need to look at the previous sibling's bottom margin.
+ If the previous sibling happens to collapse its bottom margin with the parent's bottom margin, we still need to account for it
+ and compute the static vertical position as if the bottom margin was not collapsed.
+
+ * layout/FormattingContext.cpp:
+ (WebCore::Layout::FormattingContext::computeFloatingHeightAndMargin const):
+ (WebCore::Layout::FormattingContext::computeOutOfFlowVerticalGeometry const):
+ * layout/FormattingContextGeometry.cpp:
+ (WebCore::Layout::staticVerticalPositionForOutOfFlowPositioned):
+ * layout/LayoutContext.cpp:
+ (WebCore::Layout::LayoutContext::initializeRoot):
+ * layout/Verification.cpp:
+ (WebCore::Layout::outputMismatchingBoxInformationIfNeeded):
+ * layout/blockformatting/BlockFormattingContext.cpp:
+ (WebCore::Layout::BlockFormattingContext::computeInFlowHeightAndMargin const):
+ * layout/displaytree/DisplayBox.cpp:
+ (WebCore::Display::Box::nonCollapsedMarginBox const):
+ * layout/displaytree/DisplayBox.h:
+ (WebCore::Display::Box::setHasValidVerticalNonCollapsedMargin):
+ (WebCore::Display::Box::setVerticalMargin):
+ (WebCore::Display::Box::setVerticalNonCollapsedMargin):
+ (WebCore::Display::Box::nonCollapsedMarginTop const):
+ (WebCore::Display::Box::nonCollapsedMarginBottom const):
+
2018-06-27 Antoine Quint <grao...@apple.com>
[Web Animations] Using a Web Animation leaks the Document
Modified: trunk/Source/WebCore/layout/FormattingContext.cpp (233349 => 233350)
--- trunk/Source/WebCore/layout/FormattingContext.cpp 2018-06-29 05:45:34 UTC (rev 233349)
+++ trunk/Source/WebCore/layout/FormattingContext.cpp 2018-06-29 07:55:53 UTC (rev 233350)
@@ -58,6 +58,7 @@
displayBox.moveVertically(heightAndMargin.margin.top);
ASSERT(!heightAndMargin.collapsedMargin);
displayBox.setVerticalMargin(heightAndMargin.margin);
+ displayBox.setVerticalNonCollapsedMargin(heightAndMargin.margin);
}
void FormattingContext::computeFloatingWidthAndMargin(LayoutContext& layoutContext, const Box& layoutBox, Display::Box& displayBox) const
@@ -83,6 +84,7 @@
displayBox.setContentBoxHeight(verticalGeometry.heightAndMargin.height);
ASSERT(!verticalGeometry.heightAndMargin.collapsedMargin);
displayBox.setVerticalMargin(verticalGeometry.heightAndMargin.margin);
+ displayBox.setVerticalNonCollapsedMargin(verticalGeometry.heightAndMargin.margin);
}
void FormattingContext::computeBorderAndPadding(LayoutContext& layoutContext, const Box& layoutBox, Display::Box& displayBox) const
Modified: trunk/Source/WebCore/layout/FormattingContextGeometry.cpp (233349 => 233350)
--- trunk/Source/WebCore/layout/FormattingContextGeometry.cpp 2018-06-29 05:45:34 UTC (rev 233349)
+++ trunk/Source/WebCore/layout/FormattingContextGeometry.cpp 2018-06-29 07:55:53 UTC (rev 233350)
@@ -80,6 +80,11 @@
ASSERT(layoutBox.isOutOfFlowPositioned());
LayoutUnit top;
+ // Add sibling offset
+ if (auto* previousInFlowSibling = layoutBox.previousInFlowSibling()) {
+ auto& previousInFlowDisplayBox = *layoutContext.displayBoxForLayoutBox(*previousInFlowSibling);
+ top += previousInFlowDisplayBox.bottom() + previousInFlowDisplayBox.nonCollapsedMarginBottom();
+ }
// Resolve top all the way up to the containing block.
auto* containingBlock = layoutBox.containingBlock();
for (auto* parent = layoutBox.parent(); parent; parent = parent->parent()) {
@@ -88,11 +93,6 @@
if (parent == containingBlock)
break;
}
- // Add sibling offset
- if (auto* previousInFlowSibling = layoutBox.previousInFlowSibling()) {
- auto& previousInFlowDisplayBox = *layoutContext.displayBoxForLayoutBox(*previousInFlowSibling);
- top += previousInFlowDisplayBox.bottom() + previousInFlowDisplayBox.marginBottom();
- }
// FIXME: floatings need to be taken into account.
return top;
}
Modified: trunk/Source/WebCore/layout/LayoutContext.cpp (233349 => 233350)
--- trunk/Source/WebCore/layout/LayoutContext.cpp 2018-06-29 05:45:34 UTC (rev 233349)
+++ trunk/Source/WebCore/layout/LayoutContext.cpp 2018-06-29 07:55:53 UTC (rev 233350)
@@ -59,6 +59,7 @@
// FIXME: m_root could very well be a formatting context root with ancestors and resolvable border and padding (as opposed to the topmost root)
displayBox.setHorizontalMargin({ });
displayBox.setVerticalMargin({ });
+ displayBox.setVerticalNonCollapsedMargin({ });
displayBox.setBorder({ });
displayBox.setPadding({ });
displayBox.setContentBoxHeight(containerSize.height());
Modified: trunk/Source/WebCore/layout/Verification.cpp (233349 => 233350)
--- trunk/Source/WebCore/layout/Verification.cpp 2018-06-29 05:45:34 UTC (rev 233349)
+++ trunk/Source/WebCore/layout/Verification.cpp 2018-06-29 07:55:53 UTC (rev 233350)
@@ -67,19 +67,10 @@
return true;
}
-#ifndef NDEBUG
if (renderer.marginBoxRect() != displayBox->nonCollapsedMarginBox()) {
outputRect("marginBox", renderer.marginBoxRect(), displayBox->nonCollapsedMarginBox());
return true;
}
-#else
- // For now in non-debug builds, verify the horizontal margin only
- if (renderer.marginBoxRect().left() != displayBox->marginBox().left()
- || renderer.marginBoxRect().right() != displayBox->marginBox().right() ) {
- outputRect("marginBox", renderer.marginBoxRect(), displayBox->marginBox());
- return true;
- }
-#endif
if (renderer.borderBoxRect() != displayBox->borderBox()) {
outputRect("borderBox", renderer.borderBoxRect(), displayBox->borderBox());
Modified: trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp (233349 => 233350)
--- trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp 2018-06-29 05:45:34 UTC (rev 233349)
+++ trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp 2018-06-29 07:55:53 UTC (rev 233350)
@@ -194,9 +194,7 @@
displayBox.setContentBoxHeight(heightAndMargin.height);
displayBox.moveVertically(heightAndMargin.collapsedMargin.value_or(heightAndMargin.margin).top);
displayBox.setVerticalMargin(heightAndMargin.collapsedMargin.value_or(heightAndMargin.margin));
-#ifndef NDEBUG
displayBox.setVerticalNonCollapsedMargin(heightAndMargin.margin);
-#endif
}
void BlockFormattingContext::computeInFlowWidthAndMargin(LayoutContext& layoutContext, const Box& layoutBox, Display::Box& displayBox) const
Modified: trunk/Source/WebCore/layout/displaytree/DisplayBox.cpp (233349 => 233350)
--- trunk/Source/WebCore/layout/displaytree/DisplayBox.cpp 2018-06-29 05:45:34 UTC (rev 233349)
+++ trunk/Source/WebCore/layout/displaytree/DisplayBox.cpp 2018-06-29 07:55:53 UTC (rev 233350)
@@ -62,19 +62,17 @@
return marginBox;
}
-#ifndef NDEBUG
Box::Rect Box::nonCollapsedMarginBox() const
{
auto borderBox = this->borderBox();
Rect marginBox;
- marginBox.setTop(borderBox.top() - m_nonCollapsedVertivalMargin.top);
+ marginBox.setTop(borderBox.top() - nonCollapsedMarginTop());
marginBox.setLeft(borderBox.left() - marginLeft());
- marginBox.setHeight(borderBox.height() + m_nonCollapsedVertivalMargin.top + m_nonCollapsedVertivalMargin.bottom);
+ marginBox.setHeight(borderBox.height() + nonCollapsedMarginTop() + nonCollapsedMarginBottom());
marginBox.setWidth(borderBox.width() + marginLeft() + marginRight());
return marginBox;
}
-#endif
Box::Rect Box::borderBox() const
{
Modified: trunk/Source/WebCore/layout/displaytree/DisplayBox.h (233349 => 233350)
--- trunk/Source/WebCore/layout/displaytree/DisplayBox.h 2018-06-29 05:45:34 UTC (rev 233349)
+++ trunk/Source/WebCore/layout/displaytree/DisplayBox.h 2018-06-29 07:55:53 UTC (rev 233350)
@@ -131,6 +131,9 @@
LayoutUnit marginBottom() const;
LayoutUnit marginRight() const;
+ LayoutUnit nonCollapsedMarginTop() const;
+ LayoutUnit nonCollapsedMarginBottom() const;
+
LayoutUnit borderTop() const;
LayoutUnit borderLeft() const;
LayoutUnit borderBottom() const;
@@ -147,9 +150,8 @@
LayoutUnit contentBoxWidth() const;
Rect marginBox() const;
-#ifndef NDEBUG
Rect nonCollapsedMarginBox() const;
-#endif
+
Rect borderBox() const;
Rect paddingBox() const;
Rect contentBox() const;
@@ -189,9 +191,8 @@
void setHorizontalMargin(HorizontalEdges);
void setVerticalMargin(VerticalEdges);
-#ifndef NDEBUG
- void setVerticalNonCollapsedMargin(VerticalEdges margin) { m_nonCollapsedVertivalMargin = margin; }
-#endif
+ void setVerticalNonCollapsedMargin(VerticalEdges);
+
void setBorder(Edges);
void setPadding(Edges);
@@ -201,6 +202,7 @@
void invalidatePadding() { m_hasValidPadding = false; }
void setHasValidVerticalMargin() { m_hasValidVerticalMargin = true; }
+ void setHasValidVerticalNonCollapsedMargin() { m_hasValidVerticalNonCollapsedMargin = true; }
void setHasValidHorizontalMargin() { m_hasValidHorizontalMargin = true; }
void setHasValidBorder() { m_hasValidBorder = true; }
@@ -217,9 +219,8 @@
LayoutUnit m_contentHeight;
Edges m_margin;
-#ifndef NDEBUG
- VerticalEdges m_nonCollapsedVertivalMargin;
-#endif
+ VerticalEdges m_verticalNonCollapsedMargin;
+
Edges m_border;
Edges m_padding;
@@ -226,6 +227,7 @@
#if !ASSERT_DISABLED
bool m_hasValidHorizontalMargin { false };
bool m_hasValidVerticalMargin { false };
+ bool m_hasValidVerticalNonCollapsedMargin { false };
bool m_hasValidBorder { false };
bool m_hasValidPadding { false };
bool m_hasValidContentHeight { false };
@@ -466,6 +468,14 @@
m_margin.vertical = margin;
}
+inline void Box::setVerticalNonCollapsedMargin(VerticalEdges margin)
+{
+#if !ASSERT_DISABLED
+ setHasValidVerticalNonCollapsedMargin();
+#endif
+ m_verticalNonCollapsedMargin = margin;
+}
+
inline void Box::setBorder(Edges border)
{
#if !ASSERT_DISABLED
@@ -506,6 +516,18 @@
return m_margin.horizontal.right;
}
+inline LayoutUnit Box::nonCollapsedMarginTop() const
+{
+ ASSERT(m_hasValidVerticalNonCollapsedMargin);
+ return m_verticalNonCollapsedMargin.top;
+}
+
+inline LayoutUnit Box::nonCollapsedMarginBottom() const
+{
+ ASSERT(m_hasValidVerticalNonCollapsedMargin);
+ return m_verticalNonCollapsedMargin.bottom;
+}
+
inline LayoutUnit Box::paddingTop() const
{
ASSERT(m_hasValidPadding);
