Commit 3d2d29a9 authored by Aleks Totic's avatar Aleks Totic Committed by Commit Bot

Fix incorrect static position, and container_border_physical_offset_

I was not interpreting offset correctly if static position was kTopRight
and we were looking for RightInset.

Renamed methods RightInset (instead of RightPosition) trying to make 
it more explicit, and less error prone.

Bug: 740993
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: I97c12ef1adce31f73b01e4c20850dd18ac01c443
Reviewed-on: https://chromium-review.googlesource.com/658403
Commit-Queue: Aleks Totic <atotic@chromium.org>
Reviewed-by: default avatarIan Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501155}
parent bb10fad7
......@@ -15,6 +15,34 @@ crbug.com/714962 editing/text-iterator/findString-restarts-at-last-position.html
# LayoutNG is enabled, but not when disabled.
crbug.com/707656 editing/deleting/in-visibly-empty-root.html [ Failure ]
# New passes
crbug.com/626703 external/wpt/css/css-ui-3/text-overflow-021.html [ Pass ]
crbug.com/492664 external/wpt/css/css-writing-modes-3/inline-block-alignment-003.xht [ Pass ]
crbug.com/492664 external/wpt/css/css-writing-modes-3/inline-block-alignment-005.xht [ Pass ]
crbug.com/492664 external/wpt/css/css-writing-modes-3/inline-table-alignment-003.xht [ Pass ]
crbug.com/492664 external/wpt/css/css-writing-modes-3/inline-table-alignment-005.xht [ Pass ]
crbug.com/626703 external/wpt/css/css-writing-modes-3/sizing-orthog-htb-in-vlr-004.xht [ Pass ]
crbug.com/626703 external/wpt/css/css-writing-modes-3/sizing-orthog-htb-in-vlr-016.xht [ Pass ]
crbug.com/626703 external/wpt/css/css-writing-modes-3/sizing-orthog-htb-in-vrl-016.xht [ Pass ]
crbug.com/626703 external/wpt/css/css-writing-modes-3/sizing-orthog-prct-htb-in-vlr-001.xht [ Pass ]
crbug.com/626703 external/wpt/css/css-writing-modes-3/sizing-orthog-prct-htb-in-vlr-002.xht [ Pass ]
crbug.com/626703 external/wpt/css/css-writing-modes-3/sizing-orthog-prct-htb-in-vlr-005.xht [ Pass ]
crbug.com/626703 external/wpt/css/css-writing-modes-3/sizing-orthog-prct-htb-in-vlr-006.xht [ Pass ]
crbug.com/626703 external/wpt/css/css-writing-modes-3/sizing-orthog-prct-htb-in-vrl-001.xht [ Pass ]
crbug.com/626703 external/wpt/css/css-writing-modes-3/sizing-orthog-prct-htb-in-vrl-002.xht [ Pass ]
crbug.com/626703 external/wpt/css/css-writing-modes-3/sizing-orthog-prct-htb-in-vrl-005.xht [ Pass ]
crbug.com/626703 external/wpt/css/css-writing-modes-3/sizing-orthog-prct-htb-in-vrl-006.xht [ Pass ]
crbug.com/626703 external/wpt/css/css-writing-modes-3/sizing-orthog-prct-vlr-in-htb-001.xht [ Pass ]
crbug.com/626703 external/wpt/css/css-writing-modes-3/sizing-orthog-prct-vlr-in-htb-002.xht [ Pass ]
crbug.com/626703 external/wpt/css/css-writing-modes-3/sizing-orthog-prct-vrl-in-htb-001.xht [ Pass ]
crbug.com/626703 external/wpt/css/css-writing-modes-3/sizing-orthog-prct-vrl-in-htb-002.xht [ Pass ]
crbug.com/626703 external/wpt/css/css-writing-modes-3/sizing-orthog-vlr-in-htb-001.xht [ Pass ]
crbug.com/626703 external/wpt/css/css-writing-modes-3/sizing-orthog-vlr-in-htb-004.xht [ Pass ]
crbug.com/626703 external/wpt/css/css-writing-modes-3/sizing-orthog-vrl-in-htb-001.xht [ Pass ]
crbug.com/626703 external/wpt/css/css-writing-modes-3/sizing-orthog-vrl-in-htb-004.xht [ Pass ]
crbug.com/542660 fast/css/absolute-inline-alignment-2.html [ Pass ]
crbug.com/724697 overflow/overflow-basic-002.html [ Pass ]
# New failures are appended below by the script.
crbug.com/591099 accessibility/accessibility-hit-test-crash.html [ Failure Pass ]
crbug.com/591099 accessibility/accessibility-node-memory-management.html [ Failure Pass ]
......@@ -10098,7 +10126,6 @@ crbug.com/591099 fast/scroll-behavior/smooth-scroll/mousewheel-scroll.html [ Fai
crbug.com/591099 fast/scroll-behavior/smooth-scroll/scroll-during-selection.html [ Failure Pass ]
crbug.com/591099 fast/scroll-behavior/smooth-scroll/track-scroll.html [ Failure Pass ]
crbug.com/591099 fast/scroll-behavior/subframe-interrupted-scroll.html [ Failure Pass ]
crbug.com/591099 fast/scrolling/abspos-relayout-overflow-style-change.html [ Failure ]
crbug.com/591099 fast/scrolling/content-box-smaller-than-scrollbar.html [ Crash Failure ]
crbug.com/591099 fast/scrolling/custom-scrollbar-style-applied.html [ Failure ]
crbug.com/591099 fast/scrolling/editor-command-scroll-page-scale.html [ Failure Pass ]
......@@ -13878,7 +13905,6 @@ crbug.com/591099 netinfo/web-worker.html [ Failure Pass ]
crbug.com/591099 overflow/overflow-basic-003.html [ Failure ]
crbug.com/591099 overflow/overflow-bug-chrome-ng-001.html [ Failure ]
crbug.com/591099 overflow/overflow-position-003.html [ Failure ]
crbug.com/591099 overflow/overflow-position-004.html [ Failure ]
crbug.com/591099 overflow/overflow-transform-perspective.html [ Failure ]
crbug.com/591099 paint/background/background-and-shadow.html [ Failure ]
crbug.com/591099 paint/background/fieldset-legend-background-shadow-border-radius.html [ Failure ]
......@@ -590,7 +590,6 @@ crbug.com/728378 virtual/layout_ng/overflow/overflow-basic-003.html [ Failure ]
crbug.com/724701 virtual/layout_ng/overflow/overflow-basic-004.html [ Failure ]
crbug.com/728378 virtual/layout_ng/overflow/overflow-bug-chrome-ng-001.html [ Failure ]
crbug.com/728378 virtual/layout_ng/overflow/overflow-position-003.html [ Failure ]
crbug.com/728378 virtual/layout_ng/overflow/overflow-position-004.html [ Failure ]
# ====== LayoutNG-only failures until here ======
......
......@@ -32,4 +32,34 @@ NGStaticPosition NGStaticPosition::Create(NGWritingMode writing_mode,
return position;
}
LayoutUnit NGStaticPosition::LeftInset(LayoutUnit container_size,
LayoutUnit width,
LayoutUnit margin_left,
LayoutUnit margin_right) const {
if (HasLeft())
return offset.left;
else
return offset.left - width - margin_left - margin_right;
}
LayoutUnit NGStaticPosition::RightInset(LayoutUnit container_size,
LayoutUnit width,
LayoutUnit margin_left,
LayoutUnit margin_right) const {
if (HasLeft())
return container_size - offset.left - width - margin_left - margin_right;
else
return container_size - offset.left;
}
LayoutUnit NGStaticPosition::TopInset(LayoutUnit container_size,
LayoutUnit height,
LayoutUnit margin_top,
LayoutUnit margin_bottom) const {
if (HasTop())
return offset.top;
else
return offset.top - height - margin_bottom - margin_top;
}
} // namespace blink
......@@ -24,48 +24,27 @@ struct CORE_EXPORT NGStaticPosition {
static NGStaticPosition Create(NGWritingMode,
TextDirection,
NGPhysicalOffset);
// Left/Right/TopPosition functions map static position to
// Left/Right/TopPosition functions map static position to inset of
// left/right/top edge wrt container space.
// The function arguments are required to solve the equation:
// contaner_size = left + margin_left + width + margin_right + right
LayoutUnit LeftPosition(LayoutUnit container_size,
LayoutUnit width,
LayoutUnit margin_left,
LayoutUnit margin_right) const {
return GenericPosition(HasLeft(), offset.left, container_size, width,
margin_left, margin_right);
}
LayoutUnit RightPosition(LayoutUnit container_size,
LayoutUnit width,
LayoutUnit margin_left,
LayoutUnit margin_right) const {
return GenericPosition(!HasLeft(), offset.left, container_size, width,
margin_left, margin_right);
}
LayoutUnit TopPosition(LayoutUnit container_size,
LayoutUnit height,
LayoutUnit margin_top,
LayoutUnit margin_bottom) const {
return GenericPosition(HasTop(), offset.top, container_size, height,
margin_top, margin_bottom);
}
LayoutUnit LeftInset(LayoutUnit container_size,
LayoutUnit width,
LayoutUnit margin_left,
LayoutUnit margin_right) const;
LayoutUnit RightInset(LayoutUnit container_size,
LayoutUnit width,
LayoutUnit margin_left,
LayoutUnit margin_right) const;
LayoutUnit TopInset(LayoutUnit container_size,
LayoutUnit height,
LayoutUnit margin_top,
LayoutUnit margin_bottom) const;
private:
bool HasTop() const { return type == kTopLeft || type == kTopRight; }
bool HasLeft() const { return type == kTopLeft || type == kBottomLeft; }
LayoutUnit GenericPosition(bool position_matches,
LayoutUnit position,
LayoutUnit container_size,
LayoutUnit length,
LayoutUnit margin_start,
LayoutUnit margin_end) const {
DCHECK_GE(container_size, LayoutUnit());
DCHECK_GE(length, LayoutUnit());
if (position_matches)
return position;
else
return container_size - position - length - margin_start - margin_end;
}
};
} // namespace blink
......
......@@ -114,11 +114,11 @@ void ComputeAbsoluteHorizontal(const NGConstraintSpace& space,
DCHECK(child_minmax.has_value());
width = child_minmax->ShrinkToFit(container_size.width) + border_padding;
if (space.Direction() == TextDirection::kLtr) {
left = static_position.LeftPosition(container_size.width, *width,
*margin_left, *margin_right);
left = static_position.LeftInset(container_size.width, *width,
*margin_left, *margin_right);
} else {
right = static_position.RightPosition(container_size.width, *width,
*margin_left, *margin_right);
right = static_position.RightInset(container_size.width, *width,
*margin_left, *margin_right);
}
} else if (left && right && width) {
// Standard: "If left, right, and width are not auto:"
......@@ -172,11 +172,11 @@ void ComputeAbsoluteHorizontal(const NGConstraintSpace& space,
// Rule 2.
DCHECK(width.has_value());
if (space.Direction() == TextDirection::kLtr)
left = static_position.LeftPosition(container_size.width, *width,
*margin_left, *margin_right);
left = static_position.LeftInset(container_size.width, *width,
*margin_left, *margin_right);
else
right = static_position.RightPosition(container_size.width, *width,
*margin_left, *margin_right);
right = static_position.RightInset(container_size.width, *width,
*margin_left, *margin_right);
} else if (!width && !right) {
// Rule 3.
DCHECK(child_minmax.has_value());
......@@ -269,8 +269,8 @@ void ComputeAbsoluteVertical(const NGConstraintSpace& space,
margin_bottom = LayoutUnit();
DCHECK(child_minmax.has_value());
height = child_minmax->ShrinkToFit(container_size.height) + border_padding;
top = static_position.TopPosition(container_size.height, *height,
*margin_top, *margin_bottom);
top = static_position.TopInset(container_size.height, *height, *margin_top,
*margin_bottom);
} else if (top && bottom && height) {
// Standard: "If top, bottom, and height are not auto:"
// Compute margins.
......@@ -311,8 +311,8 @@ void ComputeAbsoluteVertical(const NGConstraintSpace& space,
} else if (!top && !bottom) {
// Rule 2.
DCHECK(height.has_value());
top = static_position.TopPosition(container_size.height, *height,
*margin_top, *margin_bottom);
top = static_position.TopInset(container_size.height, *height, *margin_top,
*margin_bottom);
} else if (!height && !bottom) {
// Rule 3.
DCHECK(child_minmax.has_value());
......
......@@ -158,7 +158,7 @@ TEST_F(NGAbsoluteUtilsTest, Horizontal) {
static_right_position,
estimated_inline, WTF::nullopt);
EXPECT_EQ(minmax_60.min_size + border_padding, p.size.width);
EXPECT_EQ(LayoutUnit(0), p.inset.right);
EXPECT_EQ(container_size_.inline_size, p.inset.right);
// All auto + RTL.
p = ComputePartialAbsoluteWithChildInlineSize(
......@@ -346,7 +346,7 @@ TEST_F(NGAbsoluteUtilsTest, Vertical) {
ComputeFullAbsoluteWithChildBlockSize(*ltr_space_, *style_,
static_position_bottom, auto_height,
WTF::nullopt, &p);
EXPECT_EQ(LayoutUnit(0), p.inset.bottom);
EXPECT_EQ(container_size_.block_size, p.inset.bottom);
// If top, bottom, and height are known, compute margins.
SetVerticalStyle(top, NGAuto, height, NGAuto, bottom);
......
......@@ -206,8 +206,8 @@ RefPtr<NGLayoutResult> NGBlockLayoutAlgorithm::Layout() {
if (NeedMinMaxSize(ConstraintSpace(), Style()))
min_max_size = ComputeMinMaxSize();
border_scrollbar_padding_ = CalculateBorderScrollbarPadding(
ConstraintSpace(), Style(), Node().GetLayoutObject());
border_scrollbar_padding_ =
CalculateBorderScrollbarPadding(ConstraintSpace(), Style(), Node());
// TODO(layout-ng): For quirks mode, should we pass blockSize instead of
// NGSizeIndefinite?
......@@ -407,7 +407,8 @@ RefPtr<NGLayoutResult> NGBlockLayoutAlgorithm::Layout() {
// Only layout absolute and fixed children if we aren't going to revisit this
// layout.
if (unpositioned_floats_.IsEmpty()) {
NGOutOfFlowLayoutPart(ConstraintSpace(), Style(), &container_builder_)
NGOutOfFlowLayoutPart(Node(), ConstraintSpace(), Style(),
&container_builder_)
.Run();
}
......
......@@ -198,6 +198,23 @@ MinMaxSize NGBlockNode::ComputeMinMaxSize() {
return sizes;
}
NGBoxStrut NGBlockNode::GetScrollbarSizes() const {
NGPhysicalBoxStrut sizes;
const ComputedStyle* style = GetLayoutObject()->Style();
if (!style->IsOverflowVisible()) {
const LayoutBox* box = ToLayoutBox(GetLayoutObject());
LayoutUnit vertical = LayoutUnit(box->VerticalScrollbarWidth());
LayoutUnit horizontal = LayoutUnit(box->HorizontalScrollbarHeight());
sizes.bottom = horizontal;
if (style->ShouldPlaceBlockDirectionScrollbarOnLogicalLeft())
sizes.left = vertical;
else
sizes.right = vertical;
}
return sizes.ConvertToLogical(
FromPlatformWritingMode(style->GetWritingMode()), style->Direction());
}
NGLayoutInputNode NGBlockNode::NextSibling() const {
LayoutObject* next_sibling = box_->NextSibling();
if (next_sibling) {
......@@ -261,7 +278,7 @@ void NGBlockNode::CopyFragmentDataToLayoutBox(
intrinsic_content_logical_height += fragment.OverflowSize().block_size;
NGBoxStrut border_scrollbar_padding =
ComputeBorders(constraint_space, Style()) +
ComputePadding(constraint_space, Style()) + GetScrollbarSizes(box_);
ComputePadding(constraint_space, Style()) + GetScrollbarSizes();
if (IsLastFragment(physical_fragment))
intrinsic_content_logical_height -= border_scrollbar_padding.BlockSum();
box_->SetLogicalHeight(logical_height);
......
......@@ -20,6 +20,7 @@ class NGPhysicalBoxFragment;
class NGPhysicalFragment;
struct MinMaxSize;
struct NGBaselineRequest;
struct NGBoxStrut;
struct NGLogicalOffset;
// Represents a node to be laid out.
......@@ -40,6 +41,8 @@ class CORE_EXPORT NGBlockNode final : public NGLayoutInputNode {
// both.
MinMaxSize ComputeMinMaxSize();
NGBoxStrut GetScrollbarSizes() const;
NGLayoutInputNode FirstChild();
// Runs layout on the underlying LayoutObject and creates a fragment for the
......
......@@ -23,8 +23,8 @@ RefPtr<NGLayoutResult> NGColumnLayoutAlgorithm::Layout() {
Optional<MinMaxSize> min_max_size;
if (NeedMinMaxSize(ConstraintSpace(), Style()))
min_max_size = ComputeMinMaxSize();
NGBoxStrut border_scrollbar_padding = CalculateBorderScrollbarPadding(
ConstraintSpace(), Style(), Node().GetLayoutObject());
NGBoxStrut border_scrollbar_padding =
CalculateBorderScrollbarPadding(ConstraintSpace(), Style(), Node());
NGLogicalSize border_box_size =
CalculateBorderBoxSize(ConstraintSpace(), Style(), min_max_size);
content_box_size_ =
......@@ -71,7 +71,8 @@ RefPtr<NGLayoutResult> NGColumnLayoutAlgorithm::Layout() {
container_builder_.SetOverflowSize(overflow);
NGOutOfFlowLayoutPart(ConstraintSpace(), Style(), &container_builder_).Run();
NGOutOfFlowLayoutPart(Node(), ConstraintSpace(), Style(), &container_builder_)
.Run();
// TODO(mstensho): Propagate baselines.
......
......@@ -511,21 +511,12 @@ LayoutUnit ConstrainByMinMax(LayoutUnit length,
return length;
}
NGBoxStrut GetScrollbarSizes(const LayoutObject* layout_object) {
NGPhysicalBoxStrut sizes;
const ComputedStyle* style = layout_object->Style();
if (!style->IsOverflowVisible()) {
const LayoutBox* box = ToLayoutBox(layout_object);
LayoutUnit vertical = LayoutUnit(box->VerticalScrollbarWidth());
LayoutUnit horizontal = LayoutUnit(box->HorizontalScrollbarHeight());
sizes.bottom = horizontal;
if (style->ShouldPlaceBlockDirectionScrollbarOnLogicalLeft())
sizes.left = vertical;
else
sizes.right = vertical;
}
return sizes.ConvertToLogical(
FromPlatformWritingMode(style->GetWritingMode()), style->Direction());
NGBoxStrut CalculateBorderScrollbarPadding(
const NGConstraintSpace& constraint_space,
const ComputedStyle& style,
const NGBlockNode node) {
return ComputeBorders(constraint_space, style) +
ComputePadding(constraint_space, style) + node.GetScrollbarSizes();
}
NGLogicalSize CalculateContentBoxSize(
......
......@@ -15,9 +15,9 @@
namespace blink {
class ComputedStyle;
class LayoutObject;
class Length;
class NGConstraintSpace;
class NGBlockNode;
class NGLayoutInputNode;
enum class LengthResolveType {
......@@ -128,17 +128,10 @@ CORE_EXPORT LayoutUnit ConstrainByMinMax(LayoutUnit length,
Optional<LayoutUnit> min,
Optional<LayoutUnit> max);
// Returns scrollbar sizes or this layout object.
NGBoxStrut GetScrollbarSizes(const LayoutObject*);
inline NGBoxStrut CalculateBorderScrollbarPadding(
NGBoxStrut CalculateBorderScrollbarPadding(
const NGConstraintSpace& constraint_space,
const ComputedStyle& style,
const LayoutObject* layout_object) {
return ComputeBorders(constraint_space, style) +
ComputePadding(constraint_space, style) +
GetScrollbarSizes(layout_object);
}
const NGBlockNode node);
inline NGLogicalSize CalculateBorderBoxSize(
const NGConstraintSpace& constraint_space,
......
......@@ -34,6 +34,7 @@ bool IsContainingBlockForAbsoluteDescendant(
} // namespace
NGOutOfFlowLayoutPart::NGOutOfFlowLayoutPart(
const NGBlockNode container,
const NGConstraintSpace& container_space,
const ComputedStyle& container_style,
NGFragmentBuilder* container_builder)
......@@ -42,17 +43,19 @@ NGOutOfFlowLayoutPart::NGOutOfFlowLayoutPart(
FromPlatformWritingMode(container_style_.GetWritingMode()));
NGBoxStrut borders = ComputeBorders(container_space, container_style_);
container_border_offset_ =
NGLogicalOffset{borders.inline_start, borders.block_start};
container_border_physical_offset_ =
container_border_offset_.ConvertToPhysical(
writing_mode, container_style_.Direction(),
container_builder_->Size().ConvertToPhysical(writing_mode),
NGPhysicalSize());
NGBoxStrut scrollers = container.GetScrollbarSizes();
NGBoxStrut borders_and_scrollers = borders + scrollers;
content_offset_ = NGLogicalOffset{borders_and_scrollers.inline_start,
borders_and_scrollers.block_start};
NGPhysicalBoxStrut physical_borders = borders_and_scrollers.ConvertToPhysical(
writing_mode, container_style_.Direction());
content_physical_offset_ =
NGPhysicalOffset(physical_borders.left, physical_borders.top);
container_size_ = container_builder_->Size();
container_size_.inline_size -= borders.InlineSum();
container_size_.block_size -= borders.BlockSum();
container_size_.inline_size -= borders_and_scrollers.InlineSum();
container_size_.block_size -= borders_and_scrollers.BlockSum();
icb_size_ = container_space.InitialContainingBlockSize();
}
......@@ -97,7 +100,7 @@ RefPtr<NGLayoutResult> NGOutOfFlowLayoutPart::LayoutDescendant(
// Adjust the static_position origin. The static_position coordinate origin is
// relative to the container's border box, ng_absolute_utils expects it to be
// relative to the container's padding box.
static_position.offset -= container_border_physical_offset_;
static_position.offset -= content_physical_offset_;
// The block estimate is in the descendant's writing mode.
RefPtr<NGConstraintSpace> descendant_constraint_space =
......@@ -151,10 +154,8 @@ RefPtr<NGLayoutResult> NGOutOfFlowLayoutPart::LayoutDescendant(
// to the padding box so add back the container's borders.
NGBoxStrut inset = node_position.inset.ConvertToLogical(
container_writing_mode, container_style_.Direction());
offset->inline_offset =
inset.inline_start + container_border_offset_.inline_offset;
offset->block_offset =
inset.block_start + container_border_offset_.block_offset;
offset->inline_offset = inset.inline_start + content_offset_.inline_offset;
offset->block_offset = inset.block_start + content_offset_.block_offset;
return layout_result;
}
......
......@@ -27,7 +27,8 @@ class CORE_EXPORT NGOutOfFlowLayoutPart {
STACK_ALLOCATED();
public:
NGOutOfFlowLayoutPart(const NGConstraintSpace& contianer_space,
NGOutOfFlowLayoutPart(const NGBlockNode container,
const NGConstraintSpace& container_space,
const ComputedStyle& container_style,
NGFragmentBuilder* container_builder);
void Run();
......@@ -45,8 +46,8 @@ class CORE_EXPORT NGOutOfFlowLayoutPart {
const ComputedStyle& container_style_;
NGFragmentBuilder* container_builder_;
NGLogicalOffset container_border_offset_;
NGPhysicalOffset container_border_physical_offset_;
NGLogicalOffset content_offset_;
NGPhysicalOffset content_physical_offset_;
NGLogicalSize container_size_;
NGPhysicalSize icb_size_;
};
......
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