Skip to content

GitLab

T
tangled
Commit 0ebf8fb7 authored by Aleks Totic's avatar Aleks Totic Committed by Commit Bot
Browse files
Options

regression fix: non-auto table sizing inside flexbox 

https://chromium-review.googlesource.com/c/chromium/src/+/2214895
introduced a change where LayoutTable started respecting
OverrideLogicalWidth for flex items.

This broke rendering of non-auto tables inside flexbox.

The fix has 2 parts:
1) LayoutTable::UpdateLogicalWidth always respects OverrideLogicalWidth
2) FlexLayoutAlgorithm::ShouldApplyMinSizeAutoForChild returns true
for tables.

Another side effect of this fix is that the TablesNG flexbox hack
is no longer needed.
https://chromium-review.googlesource.com/c/chromium/src/+/1730138/167/third_party/blink/renderer/core/layout/ng/flex/ng_flex_layout_algorithm.cc

Bug: 1123100
Change-Id: Ide428db79a3d9b26e426c6f7727349549ea40836
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2392444
Commit-Queue: Aleks Totic <atotic@chromium.org>
Reviewed-by: default avatarIan Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: default avatarDavid Grogan <dgrogan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805127}
parent 6a4770b1
Hide whitespace changes
Inline Side-by-side
Showing with 76 additions and 3 deletions
third_party/blink/renderer/core/layout/flexible_box_algorithm.cc
View file @ 0ebf8fb7
......@@ -746,6 +746,9 @@ bool FlexLayoutAlgorithm::ShouldApplyMinSizeAutoForChild(
: child.StyleRef().MinHeight();
bool main_axis_is_childs_block_axis =
IsHorizontalFlow() != child.StyleRef().IsHorizontalWritingMode();
// TODO(dgrogan) Move this closer to ng_flex_layout_algorithm.cc:616.
if (child.IsTable() && !main_axis_is_childs_block_axis)
return true;
bool intrinsic_in_childs_block_axis =
main_axis_is_childs_block_axis &&
(min.IsMinContent() || min.IsMaxContent() || min.IsMinIntrinsic() ||
......
third_party/blink/renderer/core/layout/layout_table.cc
View file @ 0ebf8fb7
......@@ -388,9 +388,6 @@ void LayoutTable::UpdateLogicalWidth() {
std::min(available_content_logical_width, max_width).Floor()));
}
if (HasOverrideLogicalWidth())
SetLogicalWidth(std::max(LogicalWidth(), OverrideLogicalWidth()));
// Ensure we aren't bigger than our max-width style.
const Length& style_max_logical_width = StyleRef().LogicalMaxWidth();
if ((style_max_logical_width.IsSpecified() &&
......@@ -436,6 +433,8 @@ void LayoutTable::UpdateLogicalWidth() {
// FIXME: When we convert to sub-pixel layout for tables we can remove the int
// conversion. http://crbug.com/241198
DCHECK_GE(LogicalWidth().Floor(), preferred_logical_widths.min_size.Floor());
if (HasOverrideLogicalWidth())
SetLogicalWidth(OverrideLogicalWidth());
}
// This method takes a ComputedStyle's logical width, min-width, or max-width
......
third_party/blink/web_tests/FlagExpectations/disable-layout-ng
View file @ 0ebf8fb7
......@@ -682,3 +682,7 @@ crbug.com/591099 external/wpt/html/links/manifest/wrong-mime-type-text-plain-man
### virtual/stable/compositing/filters/
crbug.com/591099 virtual/stable/compositing/filters/sw-nested-shadow-overlaps-hw-nested-shadow.html [ Failure ]
# broken by https://chromium-review.googlesource.com/c/chromium/src/+/2392444
crbug.com/958381 external/wpt/css/css-flexbox/table-as-item-wide-content.html [ Failure ]
crbug.com/958381 external/wpt/css/css-flexbox/table-as-item-fixed-min-width.html [ Failure ]
third_party/blink/web_tests/external/wpt/css/css-flexbox/table-with-percent-intrinsic-width.html 0 → 100644
View file @ 0ebf8fb7
<!DOCTYPE html>
<html>
<head>
<title>CSS Flexbox: table descendants</title>
<link rel="help" href="https://drafts.csswg.org/css-flexbox/#flex-containers">
<link rel="help" href="https://bugs.chromium.org/p/chromium/issues/detail?id=1123100">
<meta name="assert" content="Percentage sized flex table does not use percent for intrinsic, or layout size.">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/check-layout-th.js"></script>
<style>
.container {
display:flex;
width: 200px;
}
.left {
width:200px;
background:yellow;
}
main table {
border-spacing: 0;
background: red;
}
.spacer {
display:inline-block;
width: 10px;
height: 10px;
background: green;
}
main td {
padding: 0;
}
</style>
</head>
<body>
<main>
<div class="container">
<div class="left">
</div>
<table style="width:100%" data-expected-width=100>
<td data-expected-width=20><div class="spacer"></div><div class="spacer"></div></td>
<td data-expected-width=20><div class="spacer"></div><div class="spacer"></div></td>
<td data-expected-width=20><div class="spacer"></div><div class="spacer"></div></td>
<td data-expected-width=20><div class="spacer"></div><div class="spacer"></div></td>
<td data-expected-width=20><div class="spacer"></div><div class="spacer"></div></td>
</table>
</div>
<div class="container">
<div class="left">
</div>
<table style="width:70%; flex-basis: 200px" data-expected-width=100>
<td data-expected-width=20><div class="spacer"></div><div class="spacer"></div></td>
<td data-expected-width=20><div class="spacer"></div><div class="spacer"></div></td>
<td data-expected-width=20><div class="spacer"></div><div class="spacer"></div></td>
<td data-expected-width=20><div class="spacer"></div><div class="spacer"></div></td>
<td data-expected-width=20><div class="spacer"></div><div class="spacer"></div></td>
</table>
</div>
</main>
<div id=log></div>
<script>
checkLayout("table");
</script>
</body>
</html>
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