Commit 886aed05 authored by Koji Ishii's avatar Koji Ishii Committed by Commit Bot

[LayoutNG] Change static position of inline abpos to flow relative

This patch:
1. Fixes out-of-flow static position to be flow relative,
   from line-relative.
2. Fixes bidi-reordering orders out-of-flow objects
   incorrectly.

The first fix has been a bug since the beginning, but most
RTL out-of-flow tests pass due to the second problem.

The second problem was introduced when we stopped inserting
an Object Replacement Character for out-of-flow, because it
creates a break opportunity but all 4 impls do not create
a break opportunity for out-of-flow. It wasn't a good idea
because bidi algorithm cannot resolve embedding levels
without a character that represents.

This fix will change break opportunities around out-of-flow
objects. We can fix it in NGLineBreaker if this turns out
to be a problem.

Bug: 848496
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I3f3c7b5a8ddf93cb4b9d6fce21ccc5e2b7623901
Reviewed-on: https://chromium-review.googlesource.com/1133106Reviewed-by: default avatarAleks Totic <atotic@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576799}
parent 64315a95
...@@ -531,7 +531,6 @@ crbug.com/807708 fast/css-intrinsic-dimensions/width-avoid-floats.html [ Failure ...@@ -531,7 +531,6 @@ crbug.com/807708 fast/css-intrinsic-dimensions/width-avoid-floats.html [ Failure
crbug.com/591099 fast/css/absolute-inline-alignment-2.html [ Pass ] crbug.com/591099 fast/css/absolute-inline-alignment-2.html [ Pass ]
crbug.com/591099 fast/css/bug4860-absolute-block-child-does-not-inherit-alignment.html [ Failure ] crbug.com/591099 fast/css/bug4860-absolute-block-child-does-not-inherit-alignment.html [ Failure ]
crbug.com/591099 fast/css/case-transform.html [ Failure ] crbug.com/591099 fast/css/case-transform.html [ Failure ]
crbug.com/591099 fast/css/css-properties-position-relative-as-parent-fixed.html [ Failure ]
crbug.com/591099 fast/css/focus-ring-continuations.html [ Failure ] crbug.com/591099 fast/css/focus-ring-continuations.html [ Failure ]
crbug.com/714962 fast/css/focus-ring-recursive-continuations.html [ Failure ] crbug.com/714962 fast/css/focus-ring-recursive-continuations.html [ Failure ]
crbug.com/714962 fast/css/focus-ring-recursive-inlines.html [ Failure ] crbug.com/714962 fast/css/focus-ring-recursive-inlines.html [ Failure ]
...@@ -572,7 +571,6 @@ crbug.com/591099 fast/inline/inline-with-empty-inline-children.html [ Failure ] ...@@ -572,7 +571,6 @@ crbug.com/591099 fast/inline/inline-with-empty-inline-children.html [ Failure ]
crbug.com/591099 fast/inline/nested-text-descendants.html [ Failure ] crbug.com/591099 fast/inline/nested-text-descendants.html [ Failure ]
crbug.com/591099 fast/inline/outline-continuations.html [ Failure ] crbug.com/591099 fast/inline/outline-continuations.html [ Failure ]
crbug.com/714962 fast/inline/outline-offset.html [ Failure ] crbug.com/714962 fast/inline/outline-offset.html [ Failure ]
crbug.com/860415 fast/multicol/out-of-flow/abspos-auto-position-on-line-rtl.html [ Failure ]
crbug.com/591099 fast/overflow/overflow-update-transform.html [ Failure ] crbug.com/591099 fast/overflow/overflow-update-transform.html [ Failure ]
crbug.com/591099 fast/overflow/recompute-overflow-of-layout-root-container.html [ Failure ] crbug.com/591099 fast/overflow/recompute-overflow-of-layout-root-container.html [ Failure ]
crbug.com/591099 fast/parser/entities-in-html.html [ Failure ] crbug.com/591099 fast/parser/entities-in-html.html [ Failure ]
......
...@@ -467,9 +467,18 @@ void NGInlineLayoutAlgorithm::PlaceOutOfFlowObjects( ...@@ -467,9 +467,18 @@ void NGInlineLayoutAlgorithm::PlaceOutOfFlowObjects(
static_offset.block_offset = line_box_metrics.LineHeight(); static_offset.block_offset = line_box_metrics.LineHeight();
} }
// Our child offset is line-relative, but the static offset is
// flow-relative, using the direction we give to
// |AddInlineOutOfFlowChildCandidate|.
TextDirection line_direction = line_info.BaseDirection();
if (IsRtl(line_direction)) {
static_offset.inline_offset =
line_info.Width() - static_offset.inline_offset;
}
container_builder_.AddInlineOutOfFlowChildCandidate( container_builder_.AddInlineOutOfFlowChildCandidate(
NGBlockNode(ToLayoutBox(box)), static_offset, NGBlockNode(ToLayoutBox(box)), static_offset, line_direction,
line_info.BaseDirection(), child.out_of_flow_containing_box); child.out_of_flow_containing_box);
child.out_of_flow_positioned_box = child.out_of_flow_containing_box = child.out_of_flow_positioned_box = child.out_of_flow_containing_box =
nullptr; nullptr;
......
...@@ -108,7 +108,8 @@ void CollectInlinesInternal( ...@@ -108,7 +108,8 @@ void CollectInlinesInternal(
kObjectReplacementCharacter, nullptr, node); kObjectReplacementCharacter, nullptr, node);
} else if (node->IsOutOfFlowPositioned()) { } else if (node->IsOutOfFlowPositioned()) {
builder->AppendOpaque(NGInlineItem::kOutOfFlowPositioned, nullptr, node); builder->AppendOpaque(NGInlineItem::kOutOfFlowPositioned,
kObjectReplacementCharacter, nullptr, node);
} else if (node->IsAtomicInlineLevel()) { } else if (node->IsAtomicInlineLevel()) {
if (node->IsLayoutNGListMarker()) { if (node->IsLayoutNGListMarker()) {
...@@ -477,8 +478,7 @@ void NGInlineNode::ShapeText(const String& text_content, ...@@ -477,8 +478,7 @@ void NGInlineNode::ShapeText(const String& text_content,
break; break;
end_offset = item.EndOffset(); end_offset = item.EndOffset();
} else if (item.Type() == NGInlineItem::kOpenTag || } else if (item.Type() == NGInlineItem::kOpenTag ||
item.Type() == NGInlineItem::kCloseTag || item.Type() == NGInlineItem::kCloseTag) {
item.Type() == NGInlineItem::kOutOfFlowPositioned) {
// These items are opaque to shaping. // These items are opaque to shaping.
// Opaque items cannot have text, such as Object Replacement Characters, // Opaque items cannot have text, such as Object Replacement Characters,
// since such characters can affect shaping. // since such characters can affect shaping.
......
...@@ -258,7 +258,6 @@ void NGLineBreaker::BreakLine() { ...@@ -258,7 +258,6 @@ void NGLineBreaker::BreakLine() {
} else if (item.Type() == NGInlineItem::kOpenTag) { } else if (item.Type() == NGInlineItem::kOpenTag) {
HandleOpenTag(item); HandleOpenTag(item);
} else if (item.Type() == NGInlineItem::kOutOfFlowPositioned) { } else if (item.Type() == NGInlineItem::kOutOfFlowPositioned) {
DCHECK_EQ(item.Length(), 0u);
AddItem(item); AddItem(item);
MoveToNextOf(item); MoveToNextOf(item);
} else if (item.Length()) { } else if (item.Length()) {
......
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