Commit b883f432 authored by Aleks Totic's avatar Aleks Totic Committed by Commit Bot

[LayoutNG] Propagate abspos inline container

OOF elements with inline container that were not direct descendants
of container did not have their inline container set correctly.
This caused Google SERP page to display a menu incorrectly.

Test

Bug: 867307
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: Iefc78785d296c87d7fc1f34abe3980c0783d6818
Reviewed-on: https://chromium-review.googlesource.com/1165953
Commit-Queue: Aleks Totic <atotic@chromium.org>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582139}
parent 651950b2
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html lang="en" xmlns="http://www.w3.org/1999/xhtml">
<head>
<title>reference for abspos-inline-007</title>
<style type="text/css">
.block-container {
font: 20px Ahem;
height: 20px;
position: relative;
top: -1px;
}
.inline-container {
position: relative;
border: 1px solid black;
display: inline-block;
}
.parent-block {
display: inline-block;
width: 30px;
height: 10px;
}
.abspos {
position: absolute;
width: 10px;
height: 10px;
background-color: green;
display: inline-block;
vertical-align: baseline;
}
.br {
right: 0;
bottom: 0;
}
.tl {
top: 0;
left: 0;
}
.filler {
display: inline-block;
width: 30px;
height: 10px;
}
</style>
</head>
<body>
<div class="block-container">
x
<div class="inline-container">
tl
<div class="abspos tl"></div>
<div class="parent-block"></div>
<div class="filler"></div>&nbsp;
</div>x
<div class="inline-container">
br
<div class="abspos br"></div>
<div class="parent-block"></div>
<div class="filler"></div>&nbsp;
</div>x
<div class="inline-container">
static
<div class="abspos" style="position:static"></div><div class="parent-block"></div>
<div class="filler" style="width: 20px;"></div>
</div>
</div>
<p>Tests abspos positioning of an Element that 1) has an inline containing
block, and 2) is not a child of the inline containing block, but a descendant.</p>
</body>
</html>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html lang="en" xmlns="http://www.w3.org/1999/xhtml">
<head>
<title>CSS Test: Absolutely positioned descendants in inlines</title>
<link rel="author" title="Aleks Totic" href="mailto:atotic@chromium.org"/>
<link rel="help" href="http://www.w3.org/TR/CSS21/visuren.html#comp-abspos" />
<link rel="match" href="abspos-inline-007-ref.xht" />
<style type="text/css">
.block-container {
position: relative;
font: 20px Ahem;
}
.inline-container {
position: relative;
border: 1px solid black;
}
.parent-block {
display: inline-block;
width: 30px;
height: 10px;
}
.abspos {
position: absolute;
width: 10px;
height: 10px;
background-color: green;
}
.br {
right: 0;
bottom: 0;
}
.tl {
top: 0;
left: 0;
}
.filler {
display: inline-block;
width: 30px;
height: 10px;
}
</style>
</head>
<body>
<div class="block-container">
x
<span class="inline-container">
tl
<div class="parent-block">
<div class="abspos tl"></div>
</div>
<div class="filler"></div>
</span>
x
<span class="inline-container">
br
<div class="parent-block">
<div class="abspos br"></div>
</div>
<div class="filler"></div>
</span>
x
<span class="inline-container">
static
<div class="parent-block">
<div class="abspos"></div>
</div>
<div class="filler"></div>
</span>
</div>
<p>Tests abspos positioning of an Element that 1) has an inline containing
block, and 2) is not a child of the inline containing block, but a descendant.</p>
</body>
</html>
......@@ -223,12 +223,19 @@ void NGInlineLayoutStateStack::EndBoxState(
// Create box fragments for parent if the current box has properties (e.g.,
// margin) that make it tricky to compute the parent's rects.
if (box->ParentNeedsBoxFragment(parent_box))
parent_box.SetNeedsBoxFragment();
parent_box.SetNeedsBoxFragment(nullptr);
}
void NGInlineBoxState::SetNeedsBoxFragment() {
void NGInlineBoxState::SetNeedsBoxFragment(
const LayoutObject* inline_container) {
// Note: inline_container can also be incorrectly passed as null.
// when being set for parent_box. This is ok, because inline_container
// is already set correctly inside inline_layout_algorithm.
DCHECK(item);
needs_box_fragment = true;
// Assign inline_container only if it has not been set.
if (!this->inline_container)
this->inline_container = inline_container;
}
bool NGInlineBoxState::ParentNeedsBoxFragment(
......@@ -281,6 +288,7 @@ void NGInlineLayoutStateStack::AddBoxFragmentPlaceholder(
BoxData{box->fragment_start, fragment_end, box->item, size});
BoxData& box_data = box_data_list_.back();
box_data.padding = box->padding;
box_data.inline_container = box->inline_container;
if (box->has_start_edge) {
box_data.has_line_left_edge = true;
box_data.margin_line_left = box->margin_inline_start;
......@@ -525,7 +533,7 @@ NGInlineLayoutStateStack::BoxData::CreateBoxFragment(
// NGInlineLayoutAlgorithm can handle them later.
DCHECK(!child.HasInFlowFragment());
}
box.MoveOutOfFlowDescendantCandidatesToDescendants();
box.MoveOutOfFlowDescendantCandidatesToDescendants(inline_container);
return box.ToInlineBoxFragment();
}
......
......@@ -15,6 +15,7 @@
namespace blink {
class LayoutObject;
class NGInlineItem;
struct NGInlineItemResult;
class ShapeResult;
......@@ -36,6 +37,7 @@ struct NGInlineBoxState {
unsigned fragment_start = 0;
const NGInlineItem* item = nullptr;
const ComputedStyle* style = nullptr;
const LayoutObject* inline_container = nullptr;
// The united metrics for the current box. This includes all objects in this
// box, including descendants, and adjusted by placement properties such as
......@@ -83,7 +85,7 @@ struct NGInlineBoxState {
void AccumulateUsedFonts(const ShapeResult*, FontBaseline);
// Create a box fragment for this box.
void SetNeedsBoxFragment();
void SetNeedsBoxFragment(const LayoutObject* inline_container);
// In certain circumstances, the parent's rects is not a simple union of its
// children fragments' rects, e.g., when children have margin. In such cases,
......@@ -178,6 +180,7 @@ class CORE_EXPORT NGInlineLayoutStateStack {
const NGInlineItem* item;
NGLogicalSize size;
const LayoutObject* inline_container = nullptr;
bool has_line_left_edge = false;
bool has_line_right_edge = false;
NGLineBoxStrut padding;
......
......@@ -119,8 +119,10 @@ NGInlineBoxState* NGInlineLayoutAlgorithm::HandleOpenTag(
// https://drafts.csswg.org/css2/visudet.html#line-height
if (!quirks_mode_ || !item.IsEmptyItem())
box->ComputeTextMetrics(*item.Style(), baseline_type_);
if (item.ShouldCreateBoxFragment())
box->SetNeedsBoxFragment();
if (item.ShouldCreateBoxFragment()) {
box->SetNeedsBoxFragment(
box_states_->ContainingLayoutObjectForAbsolutePositionObjects());
}
return box;
}
......@@ -234,8 +236,10 @@ void NGInlineLayoutAlgorithm::CreateLine(NGLineInfo* line_info,
} else if (item.Type() == NGInlineItem::kOpenTag) {
box = HandleOpenTag(item, item_result);
} else if (item.Type() == NGInlineItem::kCloseTag) {
if (!box->needs_box_fragment && item_result.inline_size)
box->SetNeedsBoxFragment();
if (!box->needs_box_fragment && item_result.inline_size) {
box->SetNeedsBoxFragment(
box_states_->ContainingLayoutObjectForAbsolutePositionObjects());
}
if (quirks_mode_ && box->needs_box_fragment)
box->EnsureTextMetrics(*item.Style(), baseline_type_);
box = box_states_->OnCloseTag(&line_box_, box, baseline_type_,
......@@ -798,7 +802,7 @@ scoped_refptr<NGLayoutResult> NGInlineLayoutAlgorithm::Layout() {
container_builder_.SetExclusionSpace(
exclusion_space ? std::move(exclusion_space)
: std::move(initial_exclusion_space));
container_builder_.MoveOutOfFlowDescendantCandidatesToDescendants();
container_builder_.MoveOutOfFlowDescendantCandidatesToDescendants(nullptr);
return container_builder_.ToLineBoxFragment();
}
......
......@@ -197,10 +197,16 @@ void NGContainerFragmentBuilder::GetAndClearOutOfFlowDescendantCandidates(
oof_positioned_candidates_.clear();
}
void NGContainerFragmentBuilder::
MoveOutOfFlowDescendantCandidatesToDescendants() {
void NGContainerFragmentBuilder::MoveOutOfFlowDescendantCandidatesToDescendants(
const LayoutObject* inline_container) {
GetAndClearOutOfFlowDescendantCandidates(&oof_positioned_descendants_,
nullptr);
if (inline_container) {
for (auto& descendant : oof_positioned_descendants_) {
if (!descendant.inline_container)
descendant.inline_container = inline_container;
}
}
}
#ifndef NDEBUG
......
......@@ -128,8 +128,9 @@ class CORE_EXPORT NGContainerFragmentBuilder : public NGBaseFragmentBuilder {
const LayoutObject* container);
// Utility routine to move all OOF descendant candidates to descendants.
// Use if fragment cannot hold any OOF children.
void MoveOutOfFlowDescendantCandidatesToDescendants();
// Use if fragment cannot position any OOF children.
void MoveOutOfFlowDescendantCandidatesToDescendants(
const LayoutObject* inline_container);
NGContainerFragmentBuilder& SetIsPushedByFloats() {
is_pushed_by_floats_ = true;
......
......@@ -25,11 +25,11 @@ namespace blink {
struct CORE_EXPORT NGOutOfFlowPositionedDescendant {
NGBlockNode node;
NGStaticPosition static_position;
LayoutObject* inline_container;
const LayoutObject* inline_container;
NGOutOfFlowPositionedDescendant(
NGBlockNode node_param,
NGStaticPosition static_position_param,
LayoutObject* inline_container_param = nullptr)
const LayoutObject* inline_container_param = nullptr)
: node(node_param),
static_position(static_position_param),
inline_container(inline_container_param) {}
......
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