Modified: trunk/Source/WebCore/ChangeLog (144343 => 144344)
--- trunk/Source/WebCore/ChangeLog 2013-02-28 19:55:23 UTC (rev 144343)
+++ trunk/Source/WebCore/ChangeLog 2013-02-28 19:59:23 UTC (rev 144344)
@@ -1,3 +1,46 @@
+2013-02-28 David Hyatt <[email protected]>
+
+ Remove the quirk margin bits from RenderObject and put them back in RenderBlock.
+ https://bugs.webkit.org/show_bug.cgi?id=111089
+
+ Reviewed by Dan Bernstein.
+
+ This patch removes the marginBeforeQuirk and marginAfterQuirk bits from RenderObject
+ and puts them into RenderBlock instead. I also did some renaming and clean-up after
+ moving them, e.g., to hasMarginBeforeQuirk and hasMarginAfterQuirk.
+
+ Even though it's pretty irrelevant, I also made the code writing-mode-correct so that
+ the correct child margin quirk is propagated across differing writing mode
+ boundaries.
+
+ * rendering/RenderBlock.cpp:
+ (WebCore::RenderBlock::MarginInfo::MarginInfo):
+ (WebCore::RenderBlock::RenderBlock):
+ (WebCore::RenderBlock::layoutBlock):
+ (WebCore::RenderBlock::collapseMargins):
+ (WebCore::RenderBlock::marginBeforeEstimateForChild):
+ (WebCore::RenderBlock::setCollapsedBottomMargin):
+ (WebCore::RenderBlock::handleAfterSideOfBlock):
+ (WebCore::RenderBlock::hasMarginBeforeQuirk):
+ (WebCore):
+ (WebCore::RenderBlock::hasMarginAfterQuirk):
+ * rendering/RenderBlock.h:
+ (WebCore::RenderBlock::setHasMarginBeforeQuirk):
+ (WebCore::RenderBlock::setHasMarginAfterQuirk):
+ (RenderBlock):
+ (WebCore::RenderBlock::hasMarginBeforeQuirk):
+ (WebCore::RenderBlock::hasMarginAfterQuirk):
+ (MarginInfo):
+ (WebCore::RenderBlock::MarginInfo::setHasMarginBeforeQuirk):
+ (WebCore::RenderBlock::MarginInfo::setHasMarginAfterQuirk):
+ (WebCore::RenderBlock::MarginInfo::hasMarginBeforeQuirk):
+ (WebCore::RenderBlock::MarginInfo::hasMarginAfterQuirk):
+ * rendering/RenderObject.h:
+ (RenderObject):
+ (WebCore::RenderObject::RenderObjectBitfields::RenderObjectBitfields):
+ (RenderObjectBitfields):
+ * rendering/style/RenderStyle.h:
+
2013-02-28 Sheriff Bot <[email protected]>
Unreviewed, rolling out r144126 and r144176.
Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (144343 => 144344)
--- trunk/Source/WebCore/rendering/RenderBlock.cpp 2013-02-28 19:55:23 UTC (rev 144343)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp 2013-02-28 19:59:23 UTC (rev 144344)
@@ -167,8 +167,8 @@
RenderBlock::MarginInfo::MarginInfo(RenderBlock* block, LayoutUnit beforeBorderPadding, LayoutUnit afterBorderPadding)
: m_atBeforeSideOfBlock(true)
, m_atAfterSideOfBlock(false)
- , m_marginBeforeQuirk(false)
- , m_marginAfterQuirk(false)
+ , m_hasMarginBeforeQuirk(false)
+ , m_hasMarginAfterQuirk(false)
, m_determinedMarginBeforeQuirk(false)
, m_discardMargin(false)
{
@@ -201,6 +201,8 @@
RenderBlock::RenderBlock(ContainerNode* node)
: RenderBox(node)
, m_lineHeight(-1)
+ , m_hasMarginBeforeQuirk(false)
+ , m_hasMarginAfterQuirk(false)
, m_beingDestroyed(false)
, m_hasMarkupTruncation(false)
, m_hasBorderOrPaddingLogicalWidthChanged(false)
@@ -1553,8 +1555,8 @@
if (!isCell) {
initMaxMarginValues();
- setMarginBeforeQuirk(styleToUse->isMarginBeforeQuirk());
- setMarginAfterQuirk(styleToUse->isMarginAfterQuirk());
+ setHasMarginBeforeQuirk(styleToUse->hasMarginBeforeQuirk());
+ setHasMarginAfterQuirk(styleToUse->hasMarginAfterQuirk());
setPaginationStrut(0);
}
@@ -1989,7 +1991,7 @@
// See if the top margin is quirky. We only care if this child has
// margins that will collapse with us.
- bool topQuirk = child->isMarginBeforeQuirk();
+ bool topQuirk = hasMarginBeforeQuirk(child);
if (marginInfo.canCollapseWithMarginBefore()) {
if (!childDiscardMarginBefore && !marginInfo.discardMargin()) {
@@ -2004,7 +2006,7 @@
// has an example of this, a <dt> with 0.8em author-specified inside
// a <dl> inside a <td>.
if (!marginInfo.determinedMarginBeforeQuirk() && !topQuirk && (posTop - negTop)) {
- setMarginBeforeQuirk(false);
+ setHasMarginBeforeQuirk(false);
marginInfo.setDeterminedMarginBeforeQuirk(true);
}
@@ -2014,7 +2016,7 @@
// This deals with the <td><div><p> case.
// Don't do this for a block that split two inlines though. You do
// still apply margins in this case.
- setMarginBeforeQuirk(true);
+ setHasMarginBeforeQuirk(true);
} else
// The before margin of the container will also discard all the margins it is collapsing with.
setMustDiscardMarginBefore();
@@ -2027,7 +2029,7 @@
}
if (marginInfo.quirkContainer() && marginInfo.atBeforeSideOfBlock() && (posTop - negTop))
- marginInfo.setMarginBeforeQuirk(topQuirk);
+ marginInfo.setHasMarginBeforeQuirk(topQuirk);
LayoutUnit beforeCollapseLogicalTop = logicalHeight();
LayoutUnit logicalTop = beforeCollapseLogicalTop;
@@ -2065,7 +2067,7 @@
logicalTop = logicalHeight();
} else if (!marginInfo.discardMargin() && (!marginInfo.atBeforeSideOfBlock()
|| (!marginInfo.canCollapseMarginBeforeWithChildren()
- && (!document()->inQuirksMode() || !marginInfo.quirkContainer() || !marginInfo.marginBeforeQuirk())))) {
+ && (!document()->inQuirksMode() || !marginInfo.quirkContainer() || !marginInfo.hasMarginBeforeQuirk())))) {
// We're collapsing with a previous sibling's margins and not
// with the top of the block.
setLogicalHeight(logicalHeight() + max(marginInfo.positiveMargin(), posTop) - max(marginInfo.negativeMargin(), negTop));
@@ -2081,7 +2083,7 @@
marginInfo.clearMargin();
if (marginInfo.margin())
- marginInfo.setMarginAfterQuirk(child->isMarginAfterQuirk());
+ marginInfo.setHasMarginAfterQuirk(hasMarginAfterQuirk(child));
}
// If margins would pull us past the top of the next page, then we need to pull back and pretend like the margins
@@ -2181,7 +2183,7 @@
// Give up if in quirks mode and we're a body/table cell and the top margin of the child box is quirky.
// Give up if the child specified -webkit-margin-collapse: separate that prevents collapsing.
// FIXME: Use writing mode independent accessor for marginBeforeCollapse.
- if ((document()->inQuirksMode() && child->isMarginBeforeQuirk() && (isTableCell() || isBody())) || child->style()->marginBeforeCollapse() == MSEPARATE)
+ if ((document()->inQuirksMode() && hasMarginAfterQuirk(child) && (isTableCell() || isBody())) || child->style()->marginBeforeCollapse() == MSEPARATE)
return;
// The margins are discarded by a child that specified -webkit-margin-collapse: discard.
@@ -2221,8 +2223,11 @@
// Make sure to update the block margins now for the grandchild box so that we're looking at current values.
if (grandchildBox->needsLayout()) {
grandchildBox->computeAndSetBlockDirectionMargins(this);
- grandchildBox->setMarginBeforeQuirk(grandchildBox->style()->isMarginBeforeQuirk());
- grandchildBox->setMarginAfterQuirk(grandchildBox->style()->isMarginAfterQuirk());
+ if (grandchildBox->isRenderBlock()) {
+ RenderBlock* grandchildBlock = toRenderBlock(grandchildBox);
+ grandchildBlock->setHasMarginBeforeQuirk(grandchildBox->style()->hasMarginBeforeQuirk());
+ grandchildBlock->setHasMarginAfterQuirk(grandchildBox->style()->hasMarginAfterQuirk());
+ }
}
// Collapse the margin of the grandchild box with our own to produce an estimate.
@@ -2338,14 +2343,14 @@
// with our children.
setMaxMarginAfterValues(max(maxPositiveMarginAfter(), marginInfo.positiveMargin()), max(maxNegativeMarginAfter(), marginInfo.negativeMargin()));
- if (!marginInfo.marginAfterQuirk())
- setMarginAfterQuirk(false);
+ if (!marginInfo.hasMarginAfterQuirk())
+ setHasMarginAfterQuirk(false);
- if (marginInfo.marginAfterQuirk() && marginAfter() == 0)
+ if (marginInfo.hasMarginAfterQuirk() && !marginAfter())
// We have no bottom margin and our last child has a quirky margin.
// We will pick up this quirky margin and pass it through.
// This deals with the <td><div><p> case.
- setMarginAfterQuirk(true);
+ setHasMarginAfterQuirk(true);
}
}
@@ -2358,7 +2363,7 @@
// its margin.
if (!marginInfo.discardMargin() && (!marginInfo.canCollapseWithMarginAfter() && !marginInfo.canCollapseWithMarginBefore()
&& (!isAnonymousBlock() || isAnonymousColumnsBlock() || isAnonymousColumnSpanBlock())
- && (!document()->inQuirksMode() || !marginInfo.quirkContainer() || !marginInfo.marginAfterQuirk())))
+ && (!document()->inQuirksMode() || !marginInfo.quirkContainer() || !marginInfo.hasMarginAfterQuirk())))
setLogicalHeight(logicalHeight() + marginInfo.margin());
// Now add in our bottom border/padding.
@@ -7639,6 +7644,40 @@
return marginAfterForChild(child);
}
+bool RenderBlock::hasMarginBeforeQuirk(const RenderBox* child) const
+{
+ // If the child has the same directionality as we do, then we can just return its
+ // margin quirk.
+ if (!child->isWritingModeRoot())
+ return child->isRenderBlock() ? toRenderBlock(child)->hasMarginBeforeQuirk() : child->style()->hasMarginBeforeQuirk();
+
+ // The child has a different directionality. If the child is parallel, then it's just
+ // flipped relative to us. We can use the opposite edge.
+ if (child->isHorizontalWritingMode() == isHorizontalWritingMode())
+ return child->isRenderBlock() ? toRenderBlock(child)->hasMarginAfterQuirk() : child->style()->hasMarginAfterQuirk();
+
+ // The child is perpendicular to us and box sides are never quirky in html.css, and we don't really care about
+ // whether or not authors specified quirky ems, since they're an implementation detail.
+ return false;
+}
+
+bool RenderBlock::hasMarginAfterQuirk(const RenderBox* child) const
+{
+ // If the child has the same directionality as we do, then we can just return its
+ // margin quirk.
+ if (!child->isWritingModeRoot())
+ return child->isRenderBlock() ? toRenderBlock(child)->hasMarginAfterQuirk() : child->style()->hasMarginAfterQuirk();
+
+ // The child has a different directionality. If the child is parallel, then it's just
+ // flipped relative to us. We can use the opposite edge.
+ if (child->isHorizontalWritingMode() == isHorizontalWritingMode())
+ return child->isRenderBlock() ? toRenderBlock(child)->hasMarginBeforeQuirk() : child->style()->hasMarginBeforeQuirk();
+
+ // The child is perpendicular to us and box sides are never quirky in html.css, and we don't really care about
+ // whether or not authors specified quirky ems, since they're an implementation detail.
+ return false;
+}
+
RenderBlock::MarginValues RenderBlock::marginValuesForChild(RenderBox* child) const
{
LayoutUnit childBeforePositive = 0;
Modified: trunk/Source/WebCore/rendering/RenderBlock.h (144343 => 144344)
--- trunk/Source/WebCore/rendering/RenderBlock.h 2013-02-28 19:55:23 UTC (rev 144343)
+++ trunk/Source/WebCore/rendering/RenderBlock.h 2013-02-28 19:59:23 UTC (rev 144344)
@@ -142,6 +142,15 @@
void setHasMarkupTruncation(bool b) { m_hasMarkupTruncation = b; }
bool hasMarkupTruncation() const { return m_hasMarkupTruncation; }
+ void setHasMarginBeforeQuirk(bool b) { m_hasMarginBeforeQuirk = b; }
+ void setHasMarginAfterQuirk(bool b) { m_hasMarginAfterQuirk = b; }
+
+ bool hasMarginBeforeQuirk() const { return m_hasMarginBeforeQuirk; }
+ bool hasMarginAfterQuirk() const { return m_hasMarginAfterQuirk; }
+
+ bool hasMarginBeforeQuirk(const RenderBox* child) const;
+ bool hasMarginAfterQuirk(const RenderBox* child) const;
+
RootInlineBox* createAndAppendRootInlineBox();
bool generatesLineBoxesForInlineChild(RenderObject*);
@@ -962,8 +971,8 @@
// These variables are used to detect quirky margins that we need to collapse away (in table cells
// and in the body element).
- bool m_marginBeforeQuirk : 1;
- bool m_marginAfterQuirk : 1;
+ bool m_hasMarginBeforeQuirk : 1;
+ bool m_hasMarginAfterQuirk : 1;
bool m_determinedMarginBeforeQuirk : 1;
bool m_discardMargin : 1;
@@ -982,8 +991,8 @@
m_positiveMargin = 0;
m_negativeMargin = 0;
}
- void setMarginBeforeQuirk(bool b) { m_marginBeforeQuirk = b; }
- void setMarginAfterQuirk(bool b) { m_marginAfterQuirk = b; }
+ void setHasMarginBeforeQuirk(bool b) { m_hasMarginBeforeQuirk = b; }
+ void setHasMarginAfterQuirk(bool b) { m_hasMarginAfterQuirk = b; }
void setDeterminedMarginBeforeQuirk(bool b) { m_determinedMarginBeforeQuirk = b; }
void setPositiveMargin(LayoutUnit p) { ASSERT(!m_discardMargin); m_positiveMargin = p; }
void setNegativeMargin(LayoutUnit n) { ASSERT(!m_discardMargin); m_negativeMargin = n; }
@@ -1011,8 +1020,8 @@
bool canCollapseMarginAfterWithChildren() const { return m_canCollapseMarginAfterWithChildren; }
bool quirkContainer() const { return m_quirkContainer; }
bool determinedMarginBeforeQuirk() const { return m_determinedMarginBeforeQuirk; }
- bool marginBeforeQuirk() const { return m_marginBeforeQuirk; }
- bool marginAfterQuirk() const { return m_marginAfterQuirk; }
+ bool hasMarginBeforeQuirk() const { return m_hasMarginBeforeQuirk; }
+ bool hasMarginAfterQuirk() const { return m_hasMarginAfterQuirk; }
LayoutUnit positiveMargin() const { return m_positiveMargin; }
LayoutUnit negativeMargin() const { return m_negativeMargin; }
bool discardMargin() const { return m_discardMargin; }
@@ -1235,7 +1244,9 @@
RenderObjectChildList m_children;
RenderLineBoxList m_lineBoxes; // All of the root line boxes created for this block flow. For example, <div>Hello<br>world.</div> will have two total lines for the <div>.
- mutable signed m_lineHeight : 29;
+ mutable signed m_lineHeight : 27;
+ bool m_hasMarginBeforeQuirk : 1; // Note these quirk values can't be put in RenderBlockRareData since they are set too frequently.
+ bool m_hasMarginAfterQuirk : 1;
unsigned m_beingDestroyed : 1;
unsigned m_hasMarkupTruncation : 1;
unsigned m_hasBorderOrPaddingLogicalWidthChanged : 1;
Modified: trunk/Source/WebCore/rendering/RenderObject.h (144343 => 144344)
--- trunk/Source/WebCore/rendering/RenderObject.h 2013-02-28 19:55:23 UTC (rev 144343)
+++ trunk/Source/WebCore/rendering/RenderObject.h 2013-02-28 19:59:23 UTC (rev 144344)
@@ -903,11 +903,6 @@
*/
virtual LayoutRect localCaretRect(InlineBox*, int caretOffset, LayoutUnit* extraWidthToEndOfLine = 0);
- bool isMarginBeforeQuirk() const { return m_bitfields.marginBeforeQuirk(); }
- bool isMarginAfterQuirk() const { return m_bitfields.marginAfterQuirk(); }
- void setMarginBeforeQuirk(bool b = true) { m_bitfields.setMarginBeforeQuirk(b); }
- void setMarginAfterQuirk(bool b = true) { m_bitfields.setMarginAfterQuirk(b); }
-
// When performing a global document tear-down, the renderer of the document is cleared. We use this
// as a hook to detect the case of document destruction and don't waste time doing unnecessary work.
bool documentBeingDestroyed() const;
@@ -1073,15 +1068,13 @@
, m_everHadLayout(false)
, m_inRenderFlowThread(false)
, m_childrenInline(false)
- , m_marginBeforeQuirk(false)
- , m_marginAfterQuirk(false)
, m_hasColumns(false)
, m_positionedState(IsStaticlyPositioned)
, m_selectionState(SelectionNone)
{
}
- // 32 bits have been used here. THERE ARE NO FREE BITS AVAILABLE.
+ // 29 bits have been used here. There are three bits available.
ADD_BOOLEAN_BITFIELD(needsLayout, NeedsLayout);
ADD_BOOLEAN_BITFIELD(needsPositionedMovementLayout, NeedsPositionedMovementLayout);
ADD_BOOLEAN_BITFIELD(normalChildNeedsLayout, NormalChildNeedsLayout);
@@ -1115,8 +1108,6 @@
// from RenderBlock
ADD_BOOLEAN_BITFIELD(childrenInline, ChildrenInline);
- ADD_BOOLEAN_BITFIELD(marginBeforeQuirk, MarginBeforeQuirk);
- ADD_BOOLEAN_BITFIELD(marginAfterQuirk, MarginAfterQuirk);
ADD_BOOLEAN_BITFIELD(hasColumns, HasColumns);
private: