Commit 086f7759 authored by David Grogan's avatar David Grogan Committed by Commit Bot

[LayoutNG] Improve border handling in flex items' min/max sizes

We were including border+padding in a few places where we shouldn't.

I still need to figure out whether the max-cross size should include
borders when it is influencing content size suggestion in min main size
calculations when the item has an aspect ratio.

Bug: 845235
Change-Id: Ic25f44f6dbf4da5750acc6b51b2f08f43bb0d983
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1881774
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: default avatarChristian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709805}
parent 6559f2a6
...@@ -378,9 +378,14 @@ void NGFlexLayoutAlgorithm::ConstructAndAppendFlexItems() { ...@@ -378,9 +378,14 @@ void NGFlexLayoutAlgorithm::ConstructAndAppendFlexItems() {
content_size_suggestion = content_size_suggestion =
std::min(content_size_suggestion, std::min(content_size_suggestion,
min_max_sizes_in_main_axis_direction.max_size); min_max_sizes_in_main_axis_direction.max_size);
content_size_suggestion -= main_axis_border_scrollbar_padding;
if (child.MayHaveAspectRatio()) { if (child.MayHaveAspectRatio()) {
// TODO(dgrogan): We're including borders/padding in both
// content_size_suggestion and min_max_sizes_in_cross_axis_direction.
// Maybe we need to multiply the content size by the aspect ratio and
// then apply the border/padding from the other axis inside the
// Adjust* function. Test legacy/firefox. Start with
// https://jsfiddle.net/dgrogan/9uyg3aro/
content_size_suggestion = content_size_suggestion =
AdjustChildSizeForAspectRatioCrossAxisMinAndMax( AdjustChildSizeForAspectRatioCrossAxisMinAndMax(
child, content_size_suggestion, child, content_size_suggestion,
...@@ -413,31 +418,22 @@ void NGFlexLayoutAlgorithm::ConstructAndAppendFlexItems() { ...@@ -413,31 +418,22 @@ void NGFlexLayoutAlgorithm::ConstructAndAppendFlexItems() {
// content_size_suggestion already is, and we take the min of those // content_size_suggestion already is, and we take the min of those
// two, we don't need to clamp specified_size_suggestion. // two, we don't need to clamp specified_size_suggestion.
// https://github.com/w3c/csswg-drafts/issues/3669 // https://github.com/w3c/csswg-drafts/issues/3669
if (specified_size_suggestion != LayoutUnit::Max())
specified_size_suggestion -= main_axis_border_scrollbar_padding;
min_max_sizes_in_main_axis_direction.min_size = min_max_sizes_in_main_axis_direction.min_size =
std::min(specified_size_suggestion, content_size_suggestion); std::min(specified_size_suggestion, content_size_suggestion);
} }
} else if (MainAxisIsInlineAxis(child)) { } else if (MainAxisIsInlineAxis(child)) {
min_max_sizes_in_main_axis_direction.min_size = min_max_sizes_in_main_axis_direction.min_size = ResolveMinInlineLength(
ResolveMinInlineLength( child_space, child_style, border_padding_in_child_writing_mode,
child_space, child_style, border_padding_in_child_writing_mode, intrinsic_sizes_border_box, min, LengthResolvePhase::kLayout);
intrinsic_sizes_border_box, min, LengthResolvePhase::kLayout) -
main_axis_border_scrollbar_padding;
// TODO(dgrogan): No tests changed status as result of subtracting
// main_axis_border_scrollbar_padding. It may be untested.
} else { } else {
min_max_sizes_in_main_axis_direction.min_size = min_max_sizes_in_main_axis_direction.min_size = ResolveMinBlockLength(
ResolveMinBlockLength(child_space, child_style, child_space, child_style, border_padding_in_child_writing_mode, min,
border_padding_in_child_writing_mode, min, fragment_in_child_writing_mode.BlockSize(),
fragment_in_child_writing_mode.BlockSize(), LengthResolvePhase::kLayout);
LengthResolvePhase::kLayout) -
main_axis_border_scrollbar_padding;
// TODO(dgrogan): Same as above WRT subtracting
// main_axis_border_scrollbar_padding. It may be untested.
} }
// TODO(dgrogan): Should this include scrollbar?
min_max_sizes_in_main_axis_direction -= main_axis_border_scrollbar_padding;
algorithm_ algorithm_
->emplace_back(child.GetLayoutBox(), flex_base_content_size, ->emplace_back(child.GetLayoutBox(), flex_base_content_size,
min_max_sizes_in_main_axis_direction, min_max_sizes_in_main_axis_direction,
......
...@@ -1397,7 +1397,6 @@ crbug.com/591099 external/wpt/css/css-flexbox/flexbox_align-items-stretch-3.html ...@@ -1397,7 +1397,6 @@ crbug.com/591099 external/wpt/css/css-flexbox/flexbox_align-items-stretch-3.html
### virtual/layout_ng_flex_box/css3/flexbox/ ### virtual/layout_ng_flex_box/css3/flexbox/
crbug.com/591099 virtual/layout_ng_flex_box/css3/flexbox/auto-height-column-with-border-and-padding.html [ Failure ] crbug.com/591099 virtual/layout_ng_flex_box/css3/flexbox/auto-height-column-with-border-and-padding.html [ Failure ]
crbug.com/591099 virtual/layout_ng_flex_box/css3/flexbox/box-sizing-min-max-sizes.html [ Failure ]
crbug.com/591099 virtual/layout_ng_flex_box/css3/flexbox/flex-align-end.html [ Failure ] crbug.com/591099 virtual/layout_ng_flex_box/css3/flexbox/flex-align-end.html [ Failure ]
crbug.com/591099 virtual/layout_ng_flex_box/css3/flexbox/flex-align-vertical-writing-mode.html [ Failure ] crbug.com/591099 virtual/layout_ng_flex_box/css3/flexbox/flex-align-vertical-writing-mode.html [ Failure ]
crbug.com/591099 virtual/layout_ng_flex_box/css3/flexbox/flex-align.html [ Failure ] crbug.com/591099 virtual/layout_ng_flex_box/css3/flexbox/flex-align.html [ Failure ]
...@@ -1418,7 +1417,6 @@ crbug.com/591099 virtual/layout_ng_flex_box/css3/flexbox/inline-flexbox-wrap-ver ...@@ -1418,7 +1417,6 @@ crbug.com/591099 virtual/layout_ng_flex_box/css3/flexbox/inline-flexbox-wrap-ver
crbug.com/591099 virtual/layout_ng_flex_box/css3/flexbox/minimum-size-image.html [ Failure ] crbug.com/591099 virtual/layout_ng_flex_box/css3/flexbox/minimum-size-image.html [ Failure ]
crbug.com/591099 virtual/layout_ng_flex_box/css3/flexbox/multiline-align-self.html [ Failure ] crbug.com/591099 virtual/layout_ng_flex_box/css3/flexbox/multiline-align-self.html [ Failure ]
crbug.com/591099 virtual/layout_ng_flex_box/css3/flexbox/multiline-justify-content.html [ Failure ] crbug.com/591099 virtual/layout_ng_flex_box/css3/flexbox/multiline-justify-content.html [ Failure ]
crbug.com/591099 virtual/layout_ng_flex_box/css3/flexbox/multiline-min-max.html [ Failure ]
crbug.com/591099 virtual/layout_ng_flex_box/css3/flexbox/multiline-reverse-wrap-baseline.html [ Failure ] crbug.com/591099 virtual/layout_ng_flex_box/css3/flexbox/multiline-reverse-wrap-baseline.html [ Failure ]
crbug.com/591099 virtual/layout_ng_flex_box/css3/flexbox/multiline.html [ Failure ] crbug.com/591099 virtual/layout_ng_flex_box/css3/flexbox/multiline.html [ Failure ]
crbug.com/591099 virtual/layout_ng_flex_box/css3/flexbox/negative-overflow.html [ Failure ] crbug.com/591099 virtual/layout_ng_flex_box/css3/flexbox/negative-overflow.html [ Failure ]
......
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