Skip to content

GitLab

T
tangled
Commit 18637ad8 authored by Morten Stenshorne's avatar Morten Stenshorne Committed by Commit Bot
Browse files
Options

[LayoutNG] Use the containing LayoutBlock for legacy write-back. 

The LayoutBox associated with a block node will *normally* be the
containing block (on the legacy side) of its children (on the NG side),
but this isn't an invariant. Multicol and list item markers are
two examples.

Remove the kembo DCHECK, since it seems impossible to come up with a
complete list of exceptions to the rule (that the node's box should be
the containing block of the child nodes' boxes). Instead always get the
right containing block from legacy layout. Don't use that of the node.

With the DCHECK removed, a lot of tests stop crashing, and some just
start passing instead. One known effect of this change is that floats
that are direct children of multicol containers are painted correctly.
They used to be added to the float list of the multicol container,
rather than that of the flow thread, which caused two problems: paint
order (regular blocks got painted on top of them), and that they
appeared unfragmented (they just overflowed the multicol container
instead). Added a test for that.

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: I1ee926c39fbedfd52d87e9cf790996909914efb2
Reviewed-on: https://chromium-review.googlesource.com/960027
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Reviewed-by: default avatarIan Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: default avatarAleks Totic <atotic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543065}
parent a8cf1a86
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 65 additions and 97 deletions
third_party/WebKit/LayoutTests/external/wpt/css/css-multicol/float-and-block.html 0 → 100644
View file @ 18637ad8
<!DOCTYPE html>
<title>Paint order with float VS regular block is correct inside multicol</title>
<link rel="author" title="Morten Stenshorne" href="mstensho@chromium.org">
<link rel="help" href="https://www.w3.org/TR/CSS22/zindex.html#painting-order" title="E.2 Painting order">
<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div style="columns:2; column-gap:0; width:100px; background:hotpink;">
<div style="float:left; width:50px; height:200px; background:green;"></div>
<div style="height:200px; background:red;"></div>
</div>
third_party/WebKit/Source/core/layout/ng/ng_block_node.cc
View file @ 18637ad8
......@@ -445,21 +445,18 @@ void NGBlockNode::CopyChildFragmentPosition(
DCHECK(layout_box->Parent()) << "Should be called on children only.";
// We should only be positioning children which are relative to ourselves.
// The flow thread, however, is invisible to LayoutNG, so we need to make
// an exception there.
DCHECK(
box_ == layout_box->ContainingBlock() ||
(layout_box->ContainingBlock()->IsLayoutFlowThread() &&
box_ == layout_box->ContainingBlock()->ContainingBlock()) ||
(layout_box->ContainingBlock()->IsInline() && // anonymous wrapper case
box_->Parent() == layout_box->ContainingBlock()) ||
(box_->IsLayoutNGListItem() && layout_box->IsLayoutNGListMarker()));
// The containing block of |layout_box| on the legacy layout side is normally
// |box_|, but this is not an invariant. Among other things, it does not apply
// to list item markers and multicol container children. Multicol containiner
// children typically have their flow thread (not the multicol container
// itself) as their containing block, and we need to use the right containing
// block for inserting floats, flipping for writing modes, etc.
LayoutBlock* containing_block = layout_box->ContainingBlock();
// LegacyLayout flips vertical-rl horizontal coordinates before paint.
// NGLayout flips X location for LegacyLayout compatibility.
if (box_->StyleRef().IsFlippedBlocksWritingMode()) {
LayoutUnit container_width = box_->Size().Width();
if (containing_block->StyleRef().IsFlippedBlocksWritingMode()) {
LayoutUnit container_width = containing_block->Size().Width();
layout_box->SetX(container_width - fragment.Offset().left -
additional_offset.left - fragment.Size().width);
} else {
......@@ -468,9 +465,9 @@ void NGBlockNode::CopyChildFragmentPosition(
layout_box->SetY(fragment.Offset().top + additional_offset.top);
// Floats need an associated FloatingObject for painting.
if (IsFloatFragment(fragment) && box_->IsLayoutBlockFlow()) {
if (IsFloatFragment(fragment) && containing_block->IsLayoutBlockFlow()) {
FloatingObject* floating_object =
ToLayoutBlockFlow(box_)->InsertFloatingObject(*layout_box);
ToLayoutBlockFlow(containing_block)->InsertFloatingObject(*layout_box);
floating_object->SetIsInPlacedTree(false);
floating_object->SetX(fragment.Offset().left + additional_offset.left -
layout_box->MarginLeft());
......
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