Reviewers: ikilpatrick, kojii
CL:
https://codereview.chromium.org/2847823002/Description:
Make leading OOF objects to be handled by block layout in LayoutNG
This patch changes LayoutNG's LayoutObject tree for DOM examples with
OOF object
We want to use the block layout for out of flow positioned
objects when they go in front of inline blocks or if they are just
standalone objects.
Example 1:
<div id="zero"><div id="oof"></div></div>
Legacy Layout: #oof is in inline context.
LayoutNG: #oof is in block context.
Example 2:
<div id=container><oof></oof>Hello!</div>
Legacy Layout: oof is in inline context.
LayoutNG: oof is in block context.
Example 3:
<div id=container>Hello!<oof></oof></div>
Legacy Layout: oof is in inline context.
LayoutNG: oof is in inline context.
This also deprecates ShouldHandleByInlineContext in favor of
LayoutObject::ChildrenInline()
2 new failing tests:
floats-clear/floats-114.xht
floats-clear/floats-040.xht
Those are reftest, this patch fixes border/padding issue for OOF so they
started failing because we need to fix reftest as well.
BUG=712665,635619
Affected files (+32, -27 lines):
M third_party/WebKit/LayoutTests/TestExpectations
M third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp
M third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm_test.cc
M third_party/WebKit/Source/core/layout/ng/ng_block_node.cc
M third_party/WebKit/Source/core/layout/ng/ng_block_node_test.cc
Index: third_party/WebKit/LayoutTests/TestExpectations
diff --git a/third_party/WebKit/LayoutTests/TestExpectations b/third_party/WebKit/LayoutTests/TestExpectations
index 6b5f754a601d4ae5eb99a1fcde55796603d06109..5afa83ebe958b1a9202c134be51a866331391c7a 100644
--- a/third_party/WebKit/LayoutTests/TestExpectations
+++ b/third_party/WebKit/LayoutTests/TestExpectations
@@ -371,12 +371,14 @@
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/floats-031
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/floats-036.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/floats-038.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/floats-039.xht [ Skip ]
+
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/floats-040.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/floats-043.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/floats-101.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/floats-108.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/floats-109.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/floats-110.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/floats-113.xht [ Skip ]
+
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/floats-114.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/floats-117.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/floats-120.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/floats-126.xht [ Skip ]
Index: third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp b/third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp
index 9fd14255efb568cc83fcfc7da64129b8aba15919..3e9abd5d671ba8aaeedaf0b269388036334bdd4c 100644
--- a/third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp
@@ -2984,8 +2984,30 @@ void LayoutBlockFlow::AddChild(LayoutObject* new_child,
// children as blocks.
// So, if our children are currently inline and a block child has to be
// inserted, we move all our inline children into anonymous block boxes.
- bool child_is_block_level =
- !new_child->IsInline() && !new_child->IsFloatingOrOutOfFlowPositioned();
+ bool child_is_block_level = !new_child->IsInline();
+
+ // ** LayoutNG **
+ // We want to use the block layout for out of flow positioned
+ // objects when they go in front of inline blocks or if they are just
+ // standalone objects.
+ // Example 1:
+ // <div id="zero"><div id="oof"></div></div>
+ // Legacy Layout: #oof is in inline context.
+ // LayoutNG: #oof is in block context.
+ //
+ // Example 2:
+ // <div id=container><oof></oof>Hello!</div>
+ // Legacy Layout: oof is in inline context.
+ // LayoutNG: oof is in block context.
+ //
+ // Example 3:
+ // <div id=container>Hello!<oof></oof></div>
+ // Legacy Layout: oof is in inline context.
+ // LayoutNG: oof is in inline context.
+ bool layout_ng_enabled = RuntimeEnabledFeatures::layoutNGEnabled();
+ if (new_child->IsFloatingOrOutOfFlowPositioned())
+ child_is_block_level = layout_ng_enabled && !FirstChild();
+
if (ChildrenInline()) {
if (child_is_block_level) {
// Wrap the inline content in anonymous blocks, to allow for the new block
Index: third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm_test.cc
diff --git a/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm_test.cc b/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm_test.cc
index 946359d25203cced24ece0f797abba11a7724cd9..41ed97fca9fdf0a77f9e5ad17b48643015a47a83 100644
--- a/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm_test.cc
+++ b/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm_test.cc
@@ -119,8 +119,10 @@ TEST_F(NGInlineLayoutAlgorithmTest, TextFloatsAroundFloatsBefore) {
ToNGPhysicalBoxFragment(html_fragment->Children()[0].Get());
auto* container_fragment =
ToNGPhysicalBoxFragment(body_fragment->Children()[0].Get());
- auto* line_box_fragments_wrapper =
+ auto* span_box_fragments_wrapper =
ToNGPhysicalBoxFragment(container_fragment->Children()[0].Get());
+ auto* line_box_fragments_wrapper =
+ ToNGPhysicalBoxFragment(span_box_fragments_wrapper->Children()[0].Get());
Vector<NGPhysicalTextFragment*> text_fragments;
for (const auto& child : line_box_fragments_wrapper->Children()) {
auto* line_box = ToNGPhysicalLineBoxFragment(child.Get());
Index: third_party/WebKit/Source/core/layout/ng/ng_block_node.cc
diff --git a/third_party/WebKit/Source/core/layout/ng/ng_block_node.cc b/third_party/WebKit/Source/core/layout/ng/ng_block_node.cc
index 96402b5f64b35d33df7bb8377a1409ecae153d5f..36a3fa547ce57cd2b817ddbf357d9fd624f6fc6d 100644
--- a/third_party/WebKit/Source/core/layout/ng/ng_block_node.cc
+++ b/third_party/WebKit/Source/core/layout/ng/ng_block_node.cc
@@ -222,32 +222,11 @@ LayoutObject* NGBlockNode::GetLayoutObject() {
return layout_box_;
}
-static bool ShouldHandleByInlineContext(LayoutObject* child) {
- DCHECK(child);
- // The spec isn't clear about whether floats/OOF should be in inline
- // formatting context or in block formatting context.
- // Prefer inline formatting context because 1) floats/OOF at the beginning
- // and in the middle of inline should be handled in the same code, and 2)
- // it matches to the LayoutObject tree.
- for (; child; child = child->NextSibling()) {
- if (child->IsInline())
- return true;
- if (child->IsFloating() || child->IsOutOfFlowPositioned())
- continue;
- return false;
- }
- // All children are either float or OOF.
- // TODO(kojii): Should this be handled in block context or inline context?
- // If we handle in inline, we can remove all code for floats/OOF from block
- // layout, but it may change semantics and causes incorrectness?
- return false;
-}
-
NGLayoutInputNode* NGBlockNode::FirstChild() {
if (!first_child_) {
LayoutObject* child = layout_box_->SlowFirstChild();
if (child) {
- if (ShouldHandleByInlineContext(child)) {
+ if (layout_box_->ChildrenInline()) {
first_child_ = new NGInlineNode(child, ToLayoutBlockFlow(layout_box_));
} else {
first_child_ = new NGBlockNode(child);
Index: third_party/WebKit/Source/core/layout/ng/ng_block_node_test.cc
diff --git a/third_party/WebKit/Source/core/layout/ng/ng_block_node_test.cc b/third_party/WebKit/Source/core/layout/ng/ng_block_node_test.cc
index d3becbc086b9845c4ec174e69290d1588d4a7e2e..0950ab67bc028a9a15b8a92deb4f9fe66360b124 100644
--- a/third_party/WebKit/Source/core/layout/ng/ng_block_node_test.cc
+++ b/third_party/WebKit/Source/core/layout/ng/ng_block_node_test.cc
@@ -77,7 +77,7 @@ TEST_F(NGBlockNodeForTest, ChildFloatBeforeInline) {
NGBlockNode* container =
new NGBlockNode(GetLayoutObjectByElementId("container"));
NGLayoutInputNode* child1 = container->FirstChild();
- EXPECT_TRUE(child1 && child1->IsInline());
+ EXPECT_TRUE(child1 && child1->IsBlock());
NGLayoutInputNode* child2 = child1->NextSibling();
EXPECT_EQ(child2, nullptr);
}
@@ -143,7 +143,7 @@ TEST_F(NGBlockNodeForTest, ChildOofBeforeInline) {
NGBlockNode* container =
new NGBlockNode(GetLayoutObjectByElementId("container"));
NGLayoutInputNode* child1 = container->FirstChild();
- EXPECT_TRUE(child1 && child1->IsInline());
+ EXPECT_TRUE(child1 && child1->IsBlock());
NGLayoutInputNode* child2 = child1->NextSibling();
EXPECT_EQ(child2, nullptr);
}