⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 238735 in webkit


Ignore:
Timestamp:
Nov 30, 2018, 7:22:39 AM (8 years ago)
Author:
Alan Bujtas
Message:

[LFC][BFC][MarginCollapsing] Do not use computed display box values for border and padding
​https://bugs.webkit.org/show_bug.cgi?id=192214

Reviewed by Antti Koivisto.

Source/WebCore:

Border and padding values are not necessarily computed yet when we try to estimate the margin top value. Estimating margin top is required
to be able to place floats (vertically) sooner than we would compute the final vertical position for a regular block box.

<body><div style="float: left"></div><div><div style="margin: 10px;"></div></div>

In the above example, to estimate a final vertical position of the floating box, we need to know whether the nested div's margin is collapsed
all the way up to the body. However in order to find it out we need to check for borders and paddings (they stop margin collapsing).
At the time when the floating box is being laied out, those <div> block boxes are still far down in the layout queue.

  • layout/blockformatting/BlockFormattingContext.h:
  • layout/blockformatting/BlockFormattingContextGeometry.cpp:

(WebCore::Layout::BlockFormattingContext::Geometry::inFlowNonReplacedHeightAndMargin):

  • layout/blockformatting/BlockMarginCollapse.cpp:

(WebCore::Layout::hasBorder):
(WebCore::Layout::hasPadding):
(WebCore::Layout::hasBorderBefore):
(WebCore::Layout::hasBorderAfter):
(WebCore::Layout::hasPaddingBefore):
(WebCore::Layout::hasPaddingAfter):
(WebCore::Layout::BlockFormattingContext::Geometry::MarginCollapse::isMarginTopCollapsedWithParent):
(WebCore::Layout::isMarginBottomCollapsedThrough):
(WebCore::Layout::BlockFormattingContext::Geometry::MarginCollapse::marginTop):
(WebCore::Layout::BlockFormattingContext::Geometry::MarginCollapse::marginBottom):
(WebCore::Layout::BlockFormattingContext::Geometry::MarginCollapse::isMarginBottomCollapsedWithParent):
(WebCore::Layout::BlockFormattingContext::Geometry::MarginCollapse::collapsedMarginBottomFromLastChild):

Tools:

  • LayoutReloaded/misc/LFC-passing-tests.txt:
Location:
trunk
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r238727 r238735  
     12018-11-30  Zalan Bujtas  <zalan@apple.com>
     2
     3        [LFC][BFC][MarginCollapsing] Do not use computed display box values for border and padding
     4        https://bugs.webkit.org/show_bug.cgi?id=192214
     5
     6        Reviewed by Antti Koivisto.
     7
     8        Border and padding values are not necessarily computed yet when we try to estimate the margin top value. Estimating margin top is required
     9        to be able to place floats (vertically) sooner than we would compute the final vertical position for a regular block box.
     10
     11        <body><div style="float: left"></div><div><div style="margin: 10px;"></div></div>
     12
     13        In the above example, to estimate a final vertical position of the floating box, we need to know whether the nested div's margin is collapsed
     14        all the way up to the body. However in order to find it out we need to check for borders and paddings (they stop margin collapsing).
     15        At the time when the floating box is being laied out, those <div> block boxes are still far down in the layout queue.
     16
     17        * layout/blockformatting/BlockFormattingContext.h:
     18        * layout/blockformatting/BlockFormattingContextGeometry.cpp:
     19        (WebCore::Layout::BlockFormattingContext::Geometry::inFlowNonReplacedHeightAndMargin):
     20        * layout/blockformatting/BlockMarginCollapse.cpp:
     21        (WebCore::Layout::hasBorder):
     22        (WebCore::Layout::hasPadding):
     23        (WebCore::Layout::hasBorderBefore):
     24        (WebCore::Layout::hasBorderAfter):
     25        (WebCore::Layout::hasPaddingBefore):
     26        (WebCore::Layout::hasPaddingAfter):
     27        (WebCore::Layout::BlockFormattingContext::Geometry::MarginCollapse::isMarginTopCollapsedWithParent):
     28        (WebCore::Layout::isMarginBottomCollapsedThrough):
     29        (WebCore::Layout::BlockFormattingContext::Geometry::MarginCollapse::marginTop):
     30        (WebCore::Layout::BlockFormattingContext::Geometry::MarginCollapse::marginBottom):
     31        (WebCore::Layout::BlockFormattingContext::Geometry::MarginCollapse::isMarginBottomCollapsedWithParent):
     32        (WebCore::Layout::BlockFormattingContext::Geometry::MarginCollapse::collapsedMarginBottomFromLastChild):
     33
    1342018-11-29  Frederic Wang  <fwang@igalia.com>
    235

  • trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.h

    r238701 r238735  
    9090            static LayoutUnit marginBottom(const LayoutState&, const Box&);
    9191
    92             static bool isMarginBottomCollapsedWithParent(const LayoutState&, const Box&);
     92            static bool isMarginBottomCollapsedWithParent(const Box&);
    9393            static bool isMarginTopCollapsedWithParentMarginBottom(const Box&);
    9494

  • trunk/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp

    r238701 r238735  
    8484        auto* lastInFlowChild = downcast<Container>(layoutBox).lastInFlowChild();
    8585        ASSERT(lastInFlowChild);
    86         if (!MarginCollapse::isMarginBottomCollapsedWithParent(layoutState, *lastInFlowChild)) {
     86        if (!MarginCollapse::isMarginBottomCollapsedWithParent(*lastInFlowChild)) {
    8787            auto& lastInFlowDisplayBox = layoutState.displayBoxForLayoutBox(*lastInFlowChild);
    8888            return { lastInFlowDisplayBox.bottom() + lastInFlowDisplayBox.marginBottom() - borderAndPaddingTop, nonCollapsedMargin, collapsedMargin };

  • trunk/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp

    r238667 r238735  
    3737namespace Layout {
    3838
     39static bool hasBorder(const BorderValue& borderValue)
     40{
     41    if (borderValue.style() == BorderStyle::None || borderValue.style() == BorderStyle::Hidden)
     42        return false;
     43    return !!borderValue.width();
     44}
     45
     46static bool hasPadding(const Length& paddingValue)
     47{
     48    // FIXME: Check if percent value needs to be resolved.
     49    return !paddingValue.isZero();
     50}
     51
     52static bool hasBorderBefore(const Box& layoutBox)
     53{
     54    return hasBorder(layoutBox.style().borderBefore());
     55}
     56
     57static bool hasBorderAfter(const Box& layoutBox)
     58{
     59    return hasBorder(layoutBox.style().borderAfter());
     60}
     61
     62static bool hasPaddingBefore(const Box& layoutBox)
     63{
     64    return hasPadding(layoutBox.style().paddingBefore());
     65}
     66
     67static bool hasPaddingAfter(const Box& layoutBox)
     68{
     69    return hasPadding(layoutBox.style().paddingAfter());
     70}
     71
    3972static LayoutUnit marginValue(LayoutUnit currentMarginValue, LayoutUnit candidateMarginValue)
    4073{
    … …  
    95128        return false;
    96129
     130    // Only the first inlflow child collapses with parent.
     131    if (layoutBox.previousInFlowSibling())
     132        return false;
     133
    97134    // We never margin collapse the initial containing block.
    98135    ASSERT(layoutBox.parent());
    99136    auto& parent = *layoutBox.parent();
    100     // Only the first inlflow child collapses with parent.
    101     if (layoutBox.previousInFlowSibling())
    102         return false;
    103 
    104137    if (parent.establishesBlockFormattingContext())
    105138        return false;
    … …  
    109142        return false;
    110143
    111     auto& parentDisplayBox = layoutState.displayBoxForLayoutBox(parent);
    112     if (parentDisplayBox.borderTop())
    113         return false;
    114 
    115     if (parentDisplayBox.paddingTop().value_or(0))
     144    if (hasBorderBefore(parent))
     145        return false;
     146
     147    if (hasPaddingBefore(parent))
    116148        return false;
    117149
    … …  
    122154}
    123155
    124 static bool isMarginBottomCollapsedThrough(const LayoutState& layoutState, const Box& layoutBox)
     156static bool isMarginBottomCollapsedThrough(const Box& layoutBox)
    125157{
    126158    ASSERT(layoutBox.isBlockLevelBox());
    127159
    128160    // If the top and bottom margins of a box are adjoining, then it is possible for margins to collapse through it.
    129     auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
    130 
    131     if (displayBox.borderTop() || displayBox.borderBottom())
    132         return false;
    133 
    134     if (displayBox.paddingTop().value_or(0) || displayBox.paddingBottom().value_or(0))
     161    if (hasBorderBefore(layoutBox) || hasBorderAfter(layoutBox))
     162        return false;
     163
     164    if (hasPaddingBefore(layoutBox) || hasPaddingAfter(layoutBox))
    135165        return false;
    136166
    … …  
    220250
    221251    if (!isMarginTopCollapsedWithSibling(layoutBox)) {
    222         if (!isMarginBottomCollapsedThrough(layoutState, layoutBox))
     252        if (!isMarginBottomCollapsedThrough(layoutBox))
    223253            return nonCollapsedMarginTop(layoutState, layoutBox);
    224254        // Compute the collapsed through value.
    … …  
    247277
    248278    // TODO: take _hasAdjoiningMarginTopAndBottom() into account.
    249     if (isMarginBottomCollapsedWithParent(layoutState, layoutBox))
    250         return 0;
    251 
    252     if (isMarginBottomCollapsedThrough(layoutState, layoutBox))
     279    if (isMarginBottomCollapsedWithParent(layoutBox))
     280        return 0;
     281
     282    if (isMarginBottomCollapsedThrough(layoutBox))
    253283        return 0;
    254284
    … …  
    264294}
    265295
    266 bool BlockFormattingContext::Geometry::MarginCollapse::isMarginBottomCollapsedWithParent(const LayoutState& layoutState, const Box& layoutBox)
     296bool BlockFormattingContext::Geometry::MarginCollapse::isMarginBottomCollapsedWithParent(const Box& layoutBox)
    267297{
    268298    // last inflow box to parent.
    … …  
    276306        return false;
    277307
    278     if (isMarginBottomCollapsedThrough(layoutState, layoutBox))
     308    if (isMarginBottomCollapsedThrough(layoutBox))
    279309        return false;
    280310
    … …  
    293323        return false;
    294324
    295     auto& parentDisplayBox = layoutState.displayBoxForLayoutBox(parent);
    296     if (parentDisplayBox.borderTop())
    297         return false;
    298 
    299     if (parentDisplayBox.paddingTop().value_or(0))
     325    if (hasBorderBefore(parent))
     326        return false;
     327
     328    if (hasPaddingBefore(parent))
    300329        return false;
    301330
    … …  
    325354    // FIXME: Check for collapsed through margin.
    326355    auto& lastInFlowChild = *downcast<Container>(layoutBox).lastInFlowChild();
    327     if (!isMarginBottomCollapsedWithParent(layoutState, lastInFlowChild))
     356    if (!isMarginBottomCollapsedWithParent(lastInFlowChild))
    328357        return 0;
    329358

  • trunk/Tools/ChangeLog

    r238734 r238735  
     12018-11-30  Zalan Bujtas  <zalan@apple.com>
     2
     3        [LFC][BFC][MarginCollapsing] Do not use computed display box values for border and padding
     4        https://bugs.webkit.org/show_bug.cgi?id=192214
     5
     6        Reviewed by Antti Koivisto.
     7
     8        * LayoutReloaded/misc/LFC-passing-tests.txt:
     9
    1102018-11-30  Thibault Saunier  <tsaunier@igalia.com>
    211

  • trunk/Tools/LayoutReloaded/misc/LFC-passing-tests.txt

    r238701 r238735  
    141141fast/block/positioning/052.html
    142142fast/block/positioning/054.html
     143fast/block/inside-inlines/basic-float-intrusion.html
Note: See TracChangeset for help on using the changeset viewer.