Skip to content

GitLab

T
tangled
Commit bbdbf9ff authored by mstensho@opera.com's avatar mstensho@opera.com
Browse files
Options

Avoid duplicated code in LayoutBlockChild::layoutBlockChild(). 

Before laying out, we try to estimate and set the logical top of the child, but
it may turn out after one layout pass that the estimate was wrong, due to
margin collapsing, float clearance or pagination.

So sometimes we need to reposition and relayout once or even twice inside
layoutBlockChild(). This was done with slightly poorly duplicated code.
Refactor into positionAndLayoutOnceIfNeeded() (and
markDescendantsWithFloatsForLayoutIfNeeded()). One instance of this duplicated
code used to sit in adjustBlockChildForPagination(). Moved it into
layoutBlockChild(), to make it easier to understand what's going on (and to
give adjustBlockChildForPagination() one less thing to worry about - one
parameter removed). Renamed |result| to |newLogicalTop| in
adjustBlockChildForPagination(), and |logicalTopAfterClear| to |newLogicalTop|
in layoutBlockChild().

No behavioral changes were actually intended, but when unifying
almost-duplicated code, some changes are inevitable. Added some tests for
something that now works, and used to fail. In the subsequent layout passes we
forgot to check if the new position changed how we were affected by floats.

R=jchaffraix@chromium.org,leviw@chromium.org

Review URL: https://codereview.chromium.org/1315353005

git-svn-id: svn://svn.chromium.org/blink/trunk@201866 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent c35dc14d
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 217 additions and 105 deletions
third_party/WebKit/LayoutTests/fast/block/margin-collapse/bfc-beside-float-complex-margin-collapsing-expected.txt 0 → 100644
View file @ bbdbf9ff
Given a float followed by a regular block with the same height as the float, then a block B with a negative margin, followed by a block with with overflow:hidden. Check that the overflow:hidden block doesn't overlap with the float, and also that it takes up all available space (but not more) beside the float. There's an empty collapse-through block before B, so that our initial logical top estimate fails and we have to relayout. When laying out again, we have to detect that the float that we first thought didn't affect the overflow:hidden block now affects it.
Below there should be an olive square to the left of a navy square. The navy square should be slightly larger than the olive one, and there should be spacing between them. They should not overlap.
PASS
third_party/WebKit/LayoutTests/fast/block/margin-collapse/bfc-beside-float-complex-margin-collapsing.html 0 → 100644
View file @ bbdbf9ff
<!DOCTYPE html>
<script src="../../../resources/check-layout.js"></script>
<style>
body { margin:8px; font-size:16px; }
</style>
<p>Given a float followed by a regular block with the same height as the float, then a block B with
a negative margin, followed by a block with with overflow:hidden. Check that the
overflow:hidden block doesn't overlap with the float, and also that it takes up all available
space (but not more) beside the float. There's an empty collapse-through block before B, so
that our initial logical top estimate fails and we have to relayout. When laying out again, we
have to detect that the float that we first thought didn't affect the overflow:hidden block now
affects it.</p>
<p>Below there should be an olive square to the left of a navy square. The navy square should be
slightly larger than the olive one, and there should be spacing between them. They should not
overlap.</p>
<div style="width:18em;">
<div>
<div style="float:left; width:8em; height:8em; margin-right:1em; background:olive;"></div>
<div style="height:8em;"></div>
<div>
<!-- Here's an empty block that we can just collapse through, but we don't realize that
when calculating our initial top estimate on its parent, so we have to relayout
when it turns out that a negative margin has pulled us upwards (and suddenly we
have something block-level that's affected by the float). -->
<div></div>
<div style="margin-top:-8em;"></div>
<!-- Here's a block-level element that is affected by floats, because it's a table. We
could have used e.g. an overflow:hidden DIV for the same effect. -->
<div id="bfc" style="overflow:hidden; height:9em; background:navy;" data-total-x="152" data-expected-width="144"></div>
</div>
</div>
</div>
<p id="result" style="clear:both;"></p>
<script>
checkLayout("#bfc", document.getElementById("result"));
</script>
third_party/WebKit/LayoutTests/fast/block/margin-collapse/line-beside-float-complex-margin-collapsing-expected.txt 0 → 100644
View file @ bbdbf9ff
Given a float followed by a regular block with the same height as the float, then a block B with a negative margin: Check that a line inside B doesn't overlap with the float. There's an empty collapse-through block before B, so that our initial logical top estimate fails and we have to relayout. When laying out again, we have to detect that the float that we first thought didn't affect the line now affects it.
The word "BINGO" should be seen below, to the right of a blue block.
BINGO
PASS
PASS
third_party/WebKit/LayoutTests/fast/block/margin-collapse/line-beside-float-complex-margin-collapsing.html 0 → 100644
View file @ bbdbf9ff
<!DOCTYPE html>
<style>
body { margin:8px; font-size:16px; }
</style>
<script src="../../../resources/check-layout.js"></script>
<p>Given a float followed by a regular block with the same height as the float, then a block B with
a negative margin: Check that a line inside B doesn't overlap with the float. There's an empty
collapse-through block before B, so that our initial logical top estimate fails and we have to
relayout. When laying out again, we have to detect that the float that we first thought didn't
affect the line now affects it.</p>
<p>The word "BINGO" should be seen below, to the right of a blue block.</p>
<div style="width:20em; color:blue;">
<div>
<div style="float:left; width:8em; height:8em; margin-right:1em; background:blue;"></div>
<div style="height:8em;"></div>
<div>
<!-- Here's an empty block that we can just collapse through, but we don't realize that
when calculating our initial top estimate on its parent, so we have to relayout
when it turns out that a negative margin has pulled us upwards (and suddenly we
have a line that's affected by the float). -->
<div></div>
<div class="testee" data-total-x="8" style="margin-top:-4em;">
<span class="testee" data-total-x="152">BINGO</span>
</div>
</div>
</div>
</div>
<p id="result" style="clear:both;"></p>
<script>
checkLayout(".testee", document.getElementById("result"));
</script>
third_party/WebKit/LayoutTests/fast/block/margin-collapse/table-beside-float-complex-margin-collapsing-expected.txt 0 → 100644
View file @ bbdbf9ff
Given a float followed by a regular block with the same height as the float, then a block B with a negative margin, followed by a table. Check that the table doesn't overlap with the float. There's an empty collapse-through block before B, so that our initial logical top estimate fails and we have to relayout. When laying out again, we have to detect that the float that we first thought didn't affect the table now affects it.
The word "BINGO" should be seen below, to the right of a blue block.
BINGO
PASS
third_party/WebKit/LayoutTests/fast/block/margin-collapse/table-beside-float-complex-margin-collapsing.html 0 → 100644
View file @ bbdbf9ff
<!DOCTYPE html>
<style>
body { margin:8px; font-size:16px; }
</style>
<script src="../../../resources/check-layout.js"></script>
<p>Given a float followed by a regular block with the same height as the float, then a block B with
a negative margin, followed by a table. Check that the table doesn't overlap with the
float. There's an empty collapse-through block before B, so that our initial logical top
estimate fails and we have to relayout. When laying out again, we have to detect that the float
that we first thought didn't affect the table now affects it.</p>
<p>The word "BINGO" should be seen below, to the right of a blue block.</p>
<div style="width:20em; color:blue;">
<div>
<div style="float:left; width:8em; height:8em; margin-right:1em; background:blue;"></div>
<div style="height:8em;"></div>
<div>
<!-- Here's an empty block that we can just collapse through, but we don't realize that
when calculating our initial top estimate on its parent, so we have to relayout
when it turns out that a negative margin has pulled us upwards (and suddenly we
have something block-level that's affected by the float). -->
<div></div>
<div style="margin-top:-4em;"></div>
<!-- Here's a block-level element that is affected by floats, because it's a table. We
could have used e.g. an overflow:hidden DIV for the same effect. -->
<div id="table" style="display:table;" data-total-x="152">BINGO</div>
</div>
</div>
</div>
<p id="result" style="clear:both;"></p>
<script>
checkLayout("#table", document.getElementById("result"));
</script>
third_party/WebKit/Source/core/layout/LayoutBlockFlow.h
View file @ bbdbf9ff
...@@ -288,6 +288,8 @@ private: ...@@ -288,6 +288,8 @@ private:
bool layoutBlockFlow(bool relayoutChildren, LayoutUnit& pageLogicalHeight, SubtreeLayoutScope&); bool layoutBlockFlow(bool relayoutChildren, LayoutUnit& pageLogicalHeight, SubtreeLayoutScope&);
void layoutBlockChildren(bool relayoutChildren, SubtreeLayoutScope&, LayoutUnit beforeEdge, LayoutUnit afterEdge); void layoutBlockChildren(bool relayoutChildren, SubtreeLayoutScope&, LayoutUnit beforeEdge, LayoutUnit afterEdge);
void markDescendantsWithFloatsForLayoutIfNeeded(LayoutBlockFlow& child, LayoutUnit newLogicalTop, LayoutUnit previousFloatLogicalBottom);
bool positionAndLayoutOnceIfNeeded(LayoutBox& child, LayoutUnit newLogicalTop, LayoutUnit& previousFloatLogicalBottom);
void layoutBlockChild(LayoutBox& child, MarginInfo&, LayoutUnit& previousFloatLogicalBottom); void layoutBlockChild(LayoutBox& child, MarginInfo&, LayoutUnit& previousFloatLogicalBottom);
void adjustPositionedBlock(LayoutBox& child, const MarginInfo&); void adjustPositionedBlock(LayoutBox& child, const MarginInfo&);
void adjustFloatingBlock(const MarginInfo&); void adjustFloatingBlock(const MarginInfo&);
...@@ -487,7 +489,7 @@ private: ...@@ -487,7 +489,7 @@ private:
LayoutUnit applyBeforeBreak(LayoutBox& child, LayoutUnit logicalOffset); // If the child has a before break, then return a new yPos that shifts to the top of the next page/column. LayoutUnit applyBeforeBreak(LayoutBox& child, LayoutUnit logicalOffset); // If the child has a before break, then return a new yPos that shifts to the top of the next page/column.
LayoutUnit applyAfterBreak(LayoutBox& child, LayoutUnit logicalOffset, MarginInfo&); // If the child has an after break, then return a new offset that shifts to the top of the next page/column. LayoutUnit applyAfterBreak(LayoutBox& child, LayoutUnit logicalOffset, MarginInfo&); // If the child has an after break, then return a new offset that shifts to the top of the next page/column.
LayoutUnit adjustBlockChildForPagination(LayoutUnit logicalTopAfterClear, LayoutUnit estimateWithoutPagination, LayoutBox& child, bool atBeforeSideOfBlock); LayoutUnit adjustBlockChildForPagination(LayoutUnit logicalTop, LayoutBox& child, bool atBeforeSideOfBlock);
// Computes a deltaOffset value that put a line at the top of the next page if it doesn't fit on the current page. // Computes a deltaOffset value that put a line at the top of the next page if it doesn't fit on the current page.
void adjustLinePositionForPagination(RootInlineBox&, LayoutUnit& deltaOffset); void adjustLinePositionForPagination(RootInlineBox&, LayoutUnit& deltaOffset);
// If the child is unsplittable and can't fit on the current page, return the top of the next page/column. // If the child is unsplittable and can't fit on the current page, return the top of the next page/column.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment