WebKit Bugzilla
Attachment 360013 Details for
Bug 193768
: [LFC][BFC][MarginCollapsing] MarginCollapse::collapsedVerticalValues should not return computed non-collapsed values.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-193768-20190124083317.patch (text/plain), 8.47 KB, created by
zalan
on 2019-01-24 08:33:25 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
zalan
Created:
2019-01-24 08:33:25 PST
Size:
8.47 KB
patch
obsolete
>Subversion Revision: 240362 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index 31aadd832dcae95fcbcbfea27fe4a7c474c2d45b..24952e9f725d51c0ea29ab97077b28c64925da8e 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,21 @@ >+2019-01-24 Zalan Bujtas <zalan@apple.com> >+ >+ [LFC][BFC][MarginCollapsing] MarginCollapse::collapsedVerticalValues should not return computed non-collapsed values. >+ https://bugs.webkit.org/show_bug.cgi?id=193768 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ When it comes to the actual used values it does not really matter, only from correctness point of view. >+ (This patch also moves some checks to their correct place.) >+ >+ * layout/blockformatting/BlockMarginCollapse.cpp: >+ (WebCore::Layout::BlockFormattingContext::MarginCollapse::marginBeforeCollapsesWithPreviousSiblingMarginAfter): >+ (WebCore::Layout::BlockFormattingContext::MarginCollapse::marginBeforeCollapsesWithFirstInFlowChildMarginBefore): >+ (WebCore::Layout::BlockFormattingContext::MarginCollapse::marginAfterCollapsesWithLastInFlowChildMarginAfter): >+ (WebCore::Layout::BlockFormattingContext::MarginCollapse::positiveNegativeMarginBefore): >+ (WebCore::Layout::BlockFormattingContext::MarginCollapse::positiveNegativeMarginAfter): >+ (WebCore::Layout::BlockFormattingContext::MarginCollapse::collapsedVerticalValues): >+ > 2019-01-23 Sihui Liu <sihui_liu@apple.com> > > Clean up IndexedDB files between tests >diff --git a/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp b/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp >index 8c893ce8e7de07256b53102cdc627c0e51b562a7..aeb12342960bc5833f19bcf0d08f648c8a55ea7d 100644 >--- a/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp >+++ b/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp >@@ -156,6 +156,9 @@ bool BlockFormattingContext::MarginCollapse::marginBeforeCollapsesWithPreviousSi > > auto& previousInFlowSibling = *layoutBox.previousInFlowSibling(); > >+ if (Quirks::shouldIgnoreMarginAfter(layoutState, previousInFlowSibling)) >+ return false; >+ > // Margins between a floated box and any other box do not collapse. > if (layoutBox.isFloatingPositioned() || previousInFlowSibling.isFloatingPositioned()) > return false; >@@ -200,6 +203,9 @@ bool BlockFormattingContext::MarginCollapse::marginBeforeCollapsesWithFirstInFlo > return false; > > auto& firstInFlowChild = *downcast<Container>(layoutBox).firstInFlowChild(); >+ if (Quirks::shouldIgnoreMarginBefore(layoutState, firstInFlowChild)) >+ return false; >+ > if (!firstInFlowChild.isBlockLevelBox()) > return false; > >@@ -320,6 +326,9 @@ bool BlockFormattingContext::MarginCollapse::marginAfterCollapsesWithLastInFlowC > return false; > > auto& lastInFlowChild = *downcast<Container>(layoutBox).lastInFlowChild(); >+ if (Quirks::shouldIgnoreMarginAfter(layoutState, lastInFlowChild)) >+ return false; >+ > if (!lastInFlowChild.isBlockLevelBox()) > return false; > >@@ -535,27 +544,15 @@ PositiveAndNegativeVerticalMargin::Values BlockFormattingContext::MarginCollapse > PositiveAndNegativeVerticalMargin::Values BlockFormattingContext::MarginCollapse::positiveNegativeMarginBefore(const LayoutState& layoutState, const Box& layoutBox, const UsedVerticalMargin::NonCollapsedValues& nonCollapsedValues) > { > auto firstChildCollapsedMarginBefore = [&]() -> PositiveAndNegativeVerticalMargin::Values { >- if (!is<Container>(layoutBox)) >- return { }; >- auto* firstInFlowChild = downcast<Container>(layoutBox).firstInFlowChild(); >- if (!firstInFlowChild) >- return { }; > if (!marginBeforeCollapsesWithFirstInFlowChildMarginBefore(layoutState, layoutBox)) > return { }; >- if (Quirks::shouldIgnoreMarginBefore(layoutState, *firstInFlowChild)) >- return { }; >- return positiveNegativeValues(layoutState, *firstInFlowChild, MarginType::Before); >+ return positiveNegativeValues(layoutState, *downcast<Container>(layoutBox).firstInFlowChild(), MarginType::Before); > }; > > auto previouSiblingCollapsedMarginAfter = [&]() -> PositiveAndNegativeVerticalMargin::Values { >- auto* previousInFlowSibling = layoutBox.previousInFlowSibling(); >- if (!previousInFlowSibling) >- return { }; > if (!marginBeforeCollapsesWithPreviousSiblingMarginAfter(layoutState, layoutBox)) > return { }; >- if (Quirks::shouldIgnoreMarginAfter(layoutState, *previousInFlowSibling)) >- return { }; >- return positiveNegativeValues(layoutState, *previousInFlowSibling, MarginType::After); >+ return positiveNegativeValues(layoutState, *downcast<Container>(layoutBox).previousInFlowSibling(), MarginType::After); > }; > > // 1. Gather positive and negative margin values from first child if margins are adjoining. >@@ -568,16 +565,9 @@ PositiveAndNegativeVerticalMargin::Values BlockFormattingContext::MarginCollapse > PositiveAndNegativeVerticalMargin::Values BlockFormattingContext::MarginCollapse::positiveNegativeMarginAfter(const LayoutState& layoutState, const Box& layoutBox, const UsedVerticalMargin::NonCollapsedValues& nonCollapsedValues) > { > auto lastChildCollapsedMarginAfter = [&]() -> PositiveAndNegativeVerticalMargin::Values { >- if (!is<Container>(layoutBox)) >- return { }; >- auto* lastInFlowChild = downcast<Container>(layoutBox).lastInFlowChild(); >- if (!lastInFlowChild) >- return { }; > if (!marginAfterCollapsesWithLastInFlowChildMarginAfter(layoutState, layoutBox)) > return { }; >- if (Quirks::shouldIgnoreMarginAfter(layoutState, *lastInFlowChild)) >- return { }; >- return positiveNegativeValues(layoutState, *lastInFlowChild, MarginType::After); >+ return positiveNegativeValues(layoutState, *downcast<Container>(layoutBox).lastInFlowChild(), MarginType::After); > }; > > // We don't know yet the margin before value of the next sibling. Let's just pretend it does not have one and >@@ -624,20 +614,28 @@ UsedVerticalMargin::CollapsedValues BlockFormattingContext::MarginCollapse::coll > // 1. Get min/max margin top values from the first in-flow child if we are collapsing margin top with it. > // 2. Get min/max margin top values from the previous in-flow sibling, if we are collapsing margin top with it. > // 3. Get this layout box's computed margin top value. >- // 4. Update the min/max value and compute the final margin. >+ // 4. Update the min/max value and compute the final margin. > auto positiveNegativeMarginBefore = MarginCollapse::positiveNegativeMarginBefore(layoutState, layoutBox, nonCollapsedValues); > auto positiveNegativeMarginAfter = MarginCollapse::positiveNegativeMarginAfter(layoutState, layoutBox, nonCollapsedValues); > >- auto marginsCollapseThrough = MarginCollapse::marginsCollapseThrough(layoutState, layoutBox); >+ auto marginsCollapseThrough = MarginCollapse::marginsCollapseThrough(layoutState, layoutBox); > if (marginsCollapseThrough) { > positiveNegativeMarginBefore = computedPositiveAndNegativeMargin(positiveNegativeMarginBefore, positiveNegativeMarginAfter); >- positiveNegativeMarginAfter = positiveNegativeMarginBefore; >+ positiveNegativeMarginAfter = positiveNegativeMarginBefore; > } > >+ // FIXME: Move state saving out of this function. > auto& blockFormattingState = downcast<BlockFormattingState>(layoutState.formattingStateForBox(layoutBox)); > blockFormattingState.setPositiveAndNegativeVerticalMargin(layoutBox, { positiveNegativeMarginBefore, positiveNegativeMarginAfter }); > >- return { marginValue(positiveNegativeMarginBefore), marginValue(positiveNegativeMarginAfter), marginsCollapseThrough }; >+ auto beforeMarginIsCollapsedValue = marginBeforeCollapsesWithFirstInFlowChildMarginBefore(layoutState, layoutBox) || marginBeforeCollapsesWithPreviousSiblingMarginAfter(layoutState, layoutBox); >+ auto afterMarginIsCollapsedValue = marginAfterCollapsesWithLastInFlowChildMarginAfter(layoutState, layoutBox); >+ >+ if ((beforeMarginIsCollapsedValue && afterMarginIsCollapsedValue) || marginsCollapseThrough) >+ return { marginValue(positiveNegativeMarginBefore), marginValue(positiveNegativeMarginAfter), marginsCollapseThrough }; >+ if (beforeMarginIsCollapsedValue) >+ return { marginValue(positiveNegativeMarginBefore), { }, false }; >+ return { { }, marginValue(positiveNegativeMarginAfter), false }; > } > > }
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
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 193768
: 360013