Commit e2339885 authored by Ian Kilpatrick's avatar Ian Kilpatrick Committed by Commit Bot

[layout] Fix OOF replaced elements with display: table.

Previously we had special logic within ng_absolute_utils.cc which simply
checked if we had an element with display:table for some table specific
logic.

This was incorrect, and should have been checking for the LayoutObject
type instead.
This caused some incorrect layout of replaced elements when OOF.

This patch uses the NGBlockNode::IsTable instead to apply this logic.

Bug: 1138851
Change-Id: Id428df4f48c48ad949bd3d8ffce70827e8809081
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2483962Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Reviewed-by: default avatarChristian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818448}
parent d5c55e9a
......@@ -9,6 +9,7 @@
#include "third_party/blink/renderer/core/html/html_dialog_element.h"
#include "third_party/blink/renderer/core/layout/layout_object.h"
#include "third_party/blink/renderer/core/layout/ng/geometry/ng_static_position.h"
#include "third_party/blink/renderer/core/layout/ng/ng_block_node.h"
#include "third_party/blink/renderer/core/layout/ng/ng_constraint_space.h"
#include "third_party/blink/renderer/core/layout/ng/ng_length_utils.h"
#include "third_party/blink/renderer/core/paint/paint_layer_scrollable_area.h"
......@@ -327,9 +328,10 @@ void ComputeAbsoluteSize(const LayoutUnit border_padding_size,
// exceed the available-size of the containing-block (e.g. with insets
// similar to: "left: -100px; right: -100px").
bool AbsoluteNeedsChildInlineSize(const ComputedStyle& style) {
if (style.IsDisplayTableBox())
bool AbsoluteNeedsChildInlineSize(const NGBlockNode& node) {
if (node.IsTable())
return true;
const auto& style = node.Style();
return style.LogicalWidth().IsIntrinsic() ||
style.LogicalMinWidth().IsIntrinsic() ||
style.LogicalMaxWidth().IsIntrinsic() ||
......@@ -337,9 +339,10 @@ bool AbsoluteNeedsChildInlineSize(const ComputedStyle& style) {
(style.LogicalLeft().IsAuto() || style.LogicalRight().IsAuto()));
}
bool AbsoluteNeedsChildBlockSize(const ComputedStyle& style) {
if (style.IsDisplayTableBox())
bool AbsoluteNeedsChildBlockSize(const NGBlockNode& node) {
if (node.IsTable())
return true;
const auto& style = node.Style();
return style.LogicalHeight().IsIntrinsic() ||
style.LogicalMinHeight().IsIntrinsic() ||
style.LogicalMaxHeight().IsIntrinsic() ||
......@@ -347,7 +350,8 @@ bool AbsoluteNeedsChildBlockSize(const ComputedStyle& style) {
(style.LogicalTop().IsAuto() || style.LogicalBottom().IsAuto()));
}
bool IsInlineSizeComputableFromBlockSize(const ComputedStyle& style) {
bool IsInlineSizeComputableFromBlockSize(const NGBlockNode& node) {
const auto& style = node.Style();
DCHECK(style.HasOutOfFlowPosition());
if (style.AspectRatio().IsAuto())
return false;
......@@ -362,8 +366,8 @@ bool IsInlineSizeComputableFromBlockSize(const ComputedStyle& style) {
return true;
// If we have block insets but no inline insets, we compute based on the
// insets.
return !AbsoluteNeedsChildBlockSize(style) &&
AbsoluteNeedsChildInlineSize(style);
return !AbsoluteNeedsChildBlockSize(node) &&
AbsoluteNeedsChildInlineSize(node);
}
base::Optional<LayoutUnit> ComputeAbsoluteDialogYPosition(
......@@ -417,8 +421,8 @@ base::Optional<LayoutUnit> ComputeAbsoluteDialogYPosition(
}
void ComputeOutOfFlowInlineDimensions(
const NGBlockNode& node,
const NGConstraintSpace& space,
const ComputedStyle& style,
const NGBoxStrut& border_padding,
const NGLogicalStaticPosition& static_position,
const base::Optional<MinMaxSizes>& minmax_content_sizes,
......@@ -428,6 +432,7 @@ void ComputeOutOfFlowInlineDimensions(
NGLogicalOutOfFlowDimensions* dimensions) {
DCHECK(dimensions);
const auto& style = node.Style();
Length min_inline_length = style.LogicalMinWidth();
base::Optional<MinMaxSizes> min_size_minmax = minmax_content_sizes;
// We don't need to check for IsInlineSizeComputableFromBlockSize; this is
......@@ -454,7 +459,7 @@ void ComputeOutOfFlowInlineDimensions(
}
// Tables are never allowed to go below their min-content size.
const bool is_table = style.IsDisplayTableBox();
const bool is_table = node.IsTable();
if (is_table)
min_inline_size = std::max(min_inline_size, minmax_content_sizes->min_size);
......@@ -465,7 +470,7 @@ void ComputeOutOfFlowInlineDimensions(
minmax_content_sizes, style.LogicalWidth());
} else if (replaced_size.has_value()) {
inline_size = replaced_size->inline_size;
} else if (IsInlineSizeComputableFromBlockSize(style)) {
} else if (IsInlineSizeComputableFromBlockSize(node)) {
DCHECK(minmax_content_sizes.has_value());
inline_size = minmax_content_sizes->min_size;
}
......@@ -494,21 +499,21 @@ void ComputeOutOfFlowInlineDimensions(
}
void ComputeOutOfFlowBlockDimensions(
const NGBlockNode& node,
const NGConstraintSpace& space,
const ComputedStyle& style,
const NGBoxStrut& border_padding,
const NGLogicalStaticPosition& static_position,
const base::Optional<LayoutUnit>& child_block_size,
const base::Optional<LogicalSize>& replaced_size,
const WritingDirectionMode container_writing_direction,
NGLogicalOutOfFlowDimensions* dimensions) {
const auto& style = node.Style();
// After partial size has been computed, child block size is either unknown,
// or fully computed, there is no minmax. To express this, a 'fixed' minmax
// is created where min and max are the same.
base::Optional<MinMaxSizes> min_max_sizes;
if (child_block_size.has_value()) {
if (child_block_size.has_value())
min_max_sizes = MinMaxSizes{*child_block_size, *child_block_size};
}
LayoutUnit child_block_size_or_indefinite =
child_block_size.value_or(kIndefiniteSize);
......@@ -521,7 +526,7 @@ void ComputeOutOfFlowBlockDimensions(
LengthResolvePhase::kLayout);
// Tables are never allowed to go below their "auto" block-size.
const bool is_table = style.IsDisplayTableBox();
const bool is_table = node.IsTable();
if (is_table)
min_block_size = std::max(min_block_size, min_max_sizes->min_size);
......
......@@ -14,8 +14,8 @@
namespace blink {
class ComputedStyle;
class LayoutObject;
class NGBlockNode;
class NGConstraintSpace;
struct NGLogicalStaticPosition;
......@@ -49,15 +49,15 @@ CORE_EXPORT base::Optional<LayoutUnit> ComputeAbsoluteDialogYPosition(
// Returns true if |ComputeOutOfFlowInlineDimensions| will need an estimated
// inline-size.
CORE_EXPORT bool AbsoluteNeedsChildInlineSize(const ComputedStyle&);
CORE_EXPORT bool AbsoluteNeedsChildInlineSize(const NGBlockNode&);
// Returns true if |ComputeOutOfFlowBlockDimensions| will need an estimated
// block-size.
CORE_EXPORT bool AbsoluteNeedsChildBlockSize(const ComputedStyle&);
CORE_EXPORT bool AbsoluteNeedsChildBlockSize(const NGBlockNode&);
// Returns true if the inline size can be computed from an aspect ratio and
// the block size.
bool IsInlineSizeComputableFromBlockSize(const ComputedStyle& style);
bool IsInlineSizeComputableFromBlockSize(const NGBlockNode&);
// Computes part of the absolute position which depends on the child's
// inline-size.
......@@ -66,8 +66,8 @@ bool IsInlineSizeComputableFromBlockSize(const ComputedStyle& style);
// |replaced_size| should be set if and only if element is replaced element.
// Returns the partially filled position.
CORE_EXPORT void ComputeOutOfFlowInlineDimensions(
const NGBlockNode&,
const NGConstraintSpace&,
const ComputedStyle&,
const NGBoxStrut& border_padding,
const NGLogicalStaticPosition&,
const base::Optional<MinMaxSizes>& minmax_content_sizes,
......@@ -79,8 +79,8 @@ CORE_EXPORT void ComputeOutOfFlowInlineDimensions(
// Computes the rest of the absolute position which depends on child's
// block-size.
CORE_EXPORT void ComputeOutOfFlowBlockDimensions(
const NGBlockNode&,
const NGConstraintSpace&,
const ComputedStyle&,
const NGBoxStrut& border_padding,
const NGLogicalStaticPosition&,
const base::Optional<LayoutUnit>& child_block_size,
......
......@@ -617,7 +617,7 @@ scoped_refptr<const NGLayoutResult> NGOutOfFlowLayoutPart::LayoutCandidate(
// have the same logic in legacy layout in
// |LayoutBlockFlow::UpdateBlockLayout()|.
if (node.GetLayoutBox()->IntrinsicLogicalWidthsDirty() &&
AbsoluteNeedsChildInlineSize(candidate_style)) {
AbsoluteNeedsChildInlineSize(node)) {
// Freeze the scrollbars for this layout pass. We don't want them to
// change *again*.
freeze_scrollbars.emplace();
......@@ -753,18 +753,15 @@ scoped_refptr<const NGLayoutResult> NGOutOfFlowLayoutPart::Layout(
bool has_computed_block_dimensions = false;
bool is_replaced = node.IsReplaced();
bool should_be_considered_as_replaced = node.ShouldBeConsideredAsReplaced();
bool absolute_needs_child_block_size =
AbsoluteNeedsChildBlockSize(candidate_style);
bool absolute_needs_child_block_size = AbsoluteNeedsChildBlockSize(node);
base::Optional<MinMaxSizes> minmax_intrinsic_sizes_for_ar;
// We also include items with aspect ratio here, because if the inline size
// is auto and we have a definite block size, we want to use that for the
// inline size calculation.
bool compute_inline_from_ar =
IsInlineSizeComputableFromBlockSize(candidate_style);
if (AbsoluteNeedsChildInlineSize(candidate_style) ||
NeedMinMaxSize(candidate_style) || should_be_considered_as_replaced ||
compute_inline_from_ar) {
bool compute_inline_from_ar = IsInlineSizeComputableFromBlockSize(node);
if (AbsoluteNeedsChildInlineSize(node) || NeedMinMaxSize(candidate_style) ||
should_be_considered_as_replaced || compute_inline_from_ar) {
MinMaxSizesInput input(kIndefiniteSize, MinMaxSizesType::kContent);
if (is_replaced) {
input.percentage_resolution_block_size =
......@@ -773,7 +770,7 @@ scoped_refptr<const NGLayoutResult> NGOutOfFlowLayoutPart::Layout(
// If we can determine our block-size ahead of time (it doesn't depend on
// our content), we use this for our %-block-size.
ComputeOutOfFlowBlockDimensions(
candidate_constraint_space, candidate_style, border_padding,
node, candidate_constraint_space, border_padding,
candidate_static_position, base::nullopt, base::nullopt,
container_writing_direction, &node_dimensions);
has_computed_block_dimensions = true;
......@@ -821,7 +818,7 @@ scoped_refptr<const NGLayoutResult> NGOutOfFlowLayoutPart::Layout(
}
ComputeOutOfFlowInlineDimensions(
candidate_constraint_space, candidate_style, border_padding,
node, candidate_constraint_space, border_padding,
candidate_static_position, min_max_sizes, minmax_intrinsic_sizes_for_ar,
replaced_size, container_writing_direction, &node_dimensions);
......@@ -871,7 +868,7 @@ scoped_refptr<const NGLayoutResult> NGOutOfFlowLayoutPart::Layout(
// our |min_max_sizes|, only run if needed.
if (!has_computed_block_dimensions) {
ComputeOutOfFlowBlockDimensions(
candidate_constraint_space, candidate_style, border_padding,
node, candidate_constraint_space, border_padding,
candidate_static_position, block_estimate, replaced_size,
container_writing_direction, &node_dimensions);
has_computed_block_dimensions = true;
......
<!DOCTYPE html>
<link rel="match" href="../reference/ref-filled-green-100px-square-only.html">
<link rel="help" href="https://bugs.chromium.org/p/chromium/issues/detail?id=1138851">
<p>Test passes if there is a filled green square.</p>
<div style="position: relative;">
<canvas width="200" style="background: green; width: 100px; height: 100px; position: absolute; display: table;"></canvas>
</div>
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