Skip to content

GitLab

T
tangled
Commit 772559ed authored by jfernandez's avatar jfernandez Committed by Commit bot
Browse files
Options

Revert of [table] Stretching tables when needed due to self-alignment... 

Revert of [table] Stretching tables when needed due to self-alignment properties (patchset #6 id:120001 of https://codereview.chromium.org/2528253003/ )

Reason for revert:
This patch is causing several important regressions.

Original issue's description:
> [table] Stretching tables when needed due to self-alignment properties
>
> The Self-Alignment properties, align-self and justify-self, apply to
> tables when they are flex or grid items.
>
> This patch addresses only the issue of applying the stretch value, which
> effectively affects the size of the table in those cases.
>
> The justify-self applies also to tables when they are block-level
> boxes, but the alignment feature is still not implemented for that
> case, so for now the related test cases expect no stretching.
>
> Other values of alignment aren't implemented yet for tables, but those
> will be part of future CLs, once somebody files the appropriated bug.
>
> BUG=667785
>
> Review-Url: https://codereview.chromium.org/2528253003
> Cr-Commit-Position: refs/heads/master@{#454743}
> Committed: https://chromium.googlesource.com/chromium/src/+/a90dba1f2dd094e51229dd2e5e059c6b53cee93a

TBR=svillar@igalia.com,rego@igalia.com,cbiesinger@chromium.org,dgrogan@chromium.org,mstensho@opera.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=667785

Review-Url: https://codereview.chromium.org/2740063003
Cr-Commit-Position: refs/heads/master@{#455719}
parent b348620b
Hide whitespace changes
Inline Side-by-side
Showing with 45 additions and 172 deletions
third_party/WebKit/LayoutTests/css3/flexbox/flexitem-expected.txt 0 → 100644
View file @ 772559ed
This is a testharness.js-based test.
PASS .flexbox 1
PASS .flexbox 2
PASS .flexbox 3
PASS .flexbox 4
PASS .flexbox 5
PASS .flexbox 6
PASS .flexbox 7
PASS .flexbox 8
PASS .flexbox 9
PASS .flexbox 10
PASS .flexbox 11
FAIL .flexbox 12 assert_equals:
<div class="flexbox">
<!-- FIXME: This table should flex. -->
<div data-expected-display="table" data-expected-width="600" style="display: inline-table"></div>
</div>
width expected 600 but got 0
PASS .flexbox 13
PASS .flexbox 14
PASS .flexbox 15
PASS .flexbox 16
PASS .flexbox 17
PASS .flexbox 18
Harness: the test ran to completion.
third_party/WebKit/LayoutTests/fast/table/table-size-must-consider-stretch-alignment.html deleted 100644 → 0
View file @ b348620b
<!DOCTYPE HTML>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script src="../../resources/check-layout-th.js"></script>
<style>
.block {
width: 200px;
height: 200px;
background: lightgrey;
}
.flex { display: flex; }
.grid {
display: grid;
grid: 100px / 150px;
}
.itemsStart {
align-items: start;
justify-items: start;
}
.itemsFlexStart {
align-items: flex-start;
justify-items: flex-start;
}
.item {
display: table;
background: lime;
border-spacing: 0px;
font: 10px/1 Ahem;
}
td { padding: 0px; }
caption { background: grey; }
</style>
<p>This test verifies that table sizing logic considers stretch alignment when computing its width and height.</p>
<p>Regular block container of a table element and 1 implicit row and column.<br>The align-self property doesn't apply to block-level boxes.</br>The justify-self property has its initial/default value 'normal', which behaves like 'start'.</p>
<div class="block">
<div class="item" data-expected-width="100" data-expected-height="10">table cell</div>
</div>
<br><br>
<p>Regular block container of an empty table.<br>The align-self property doesn't apply to block-level boxes.</br>The justify-self property has its initial/default value 'normal', which behaves like 'start'.</p>
<div class="block">
<table class="item" data-expected-width="70" data-expected-height="10">
<caption>caption</caption>
</table>
</div>
<br><br>
<p>Regular block container of a table element and 1 explicit row and column.<br>The align-self property doesn't apply to block-level boxes.</br>The justify-self property has its initial/default value 'normal', which behaves like 'start'.</p>
<div class="block">
<table class="item" data-expected-width="100" data-expected-height="20">
<tr data-expected-width="100" data-expected-height="10">
<td>table cell</td>
</tr>
<caption>caption</caption>
</table>
</div>
<br><br>
<p>Grid container of table element and 1 implicit row and column.<br>Both the align-self and justify-self properties have their initial/default value 'normal', which behaves like 'stretch'.</p>
<div class="block grid">
<div class="item" data-expected-width="150" data-expected-height="100">table cell</div>
</div>
<br><br>
<p>Grid container of an empty table element.<br>Both the align-self and justify-self properties have their initial/default value 'normal', which behaves like 'stretch'.</p>
<div class="block grid">
<table class="item" data-expected-width="150" data-expected-height="110">
<caption>caption</caption>
</table>
</div>
<br><br>
<p>Grid container of an empty table element.<br>Both the align-self and justify-self properties have a value 'start', which should prevent the item to be stretched.</p>
<div class="block grid itemsStart">
<table class="item" data-expected-width="70" data-expected-height="10">
<caption>caption</caption>
</table>
</div>
<br><br>
<p>Grid container of table element and 1 explicit row and column.<br>Both the align-self and justify-self properties have their initial/default value 'normal', which behaves like 'stretch'.</p>
<div class="block grid">
<table class="item" data-expected-width="150" data-expected-height="110">
<tr data-expected-width="150" data-expected-height="100">
<td>table cell</td>
</tr>
<caption>caption</caption>
</table>
</div>
<br><br>
<p>Flex container of table element and 1 implicit row and column.<br>Both the align-self and justify-self properties have their initial/default value 'normal', which behaves like 'stretch'.</p>
<div class="block flex">
<div class="item" class="item" data-expected-width="200" data-expected-height="200">table cell</div>
</div>
<br><br>
<p>Flex container of an empty table element.<br>Both the align-self and justify-self properties have their initial/default value 'normal', which behaves like 'stretch'.</p>
<div class="block flex">
<table class="item" data-expected-width="200" data-expected-height="210">
<caption>caption</caption>
</table>
</div>
<br><br>
<p>Flex container of an empty table element.<br>Both the align-self and justify-self properties have a value 'start', which should prevent the item to be stretched.</p>
<div class="block flex itemsFlexStart">
<table class="item" data-expected-width="70" data-expected-height="10">
<caption>caption</caption>
</table>
</div>
<br><br>
<p>Flex container of table element and 1 explicit row and column.<br>Both the align-self and justify-self properties have their initial/default value 'normal', which behaves like 'stretch'.</p>
<div class="block flex">
<table class="item" class="item" data-expected-width="200" data-expected-height="210">
<tr data-expected-width="200" data-expected-height="200">
<td>table cell</td>
</tr>
<caption>caption</caption>
</table>
</div>
<script>
checkLayout('.block');
</script>
third_party/WebKit/Source/core/layout/LayoutBox.h
View file @ 772559ed
...@@ -1387,7 +1387,8 @@ class CORE_EXPORT LayoutBox : public LayoutBoxModelObject { ...@@ -1387,7 +1387,8 @@ class CORE_EXPORT LayoutBox : public LayoutBoxModelObject {
virtual ItemPosition selfAlignmentNormalBehavior( virtual ItemPosition selfAlignmentNormalBehavior(
const LayoutBox* child = nullptr) const { const LayoutBox* child = nullptr) const {
return ItemPositionStart; DCHECK(!child);
return ItemPositionStretch;
} }
// Returns false if it could not cheaply compute the extent (e.g. fixed // Returns false if it could not cheaply compute the extent (e.g. fixed
......
third_party/WebKit/Source/core/layout/LayoutFlexibleBox.h
View file @ 772559ed
...@@ -91,10 +91,6 @@ class CORE_EXPORT LayoutFlexibleBox : public LayoutBlock { ...@@ -91,10 +91,6 @@ class CORE_EXPORT LayoutFlexibleBox : public LayoutBlock {
void styleDidChange(StyleDifference, const ComputedStyle* oldStyle) override; void styleDidChange(StyleDifference, const ComputedStyle* oldStyle) override;
void removeChild(LayoutObject*) override; void removeChild(LayoutObject*) override;
ItemPosition selfAlignmentNormalBehavior(
const LayoutBox* child = nullptr) const override {
return ItemPositionStretch;
}
private: private:
enum FlexSign { enum FlexSign {
......
third_party/WebKit/Source/core/layout/LayoutFullScreen.h
View file @ 772559ed
...@@ -69,6 +69,7 @@ class CORE_EXPORT LayoutFullScreen final : public LayoutFlexibleBox { ...@@ -69,6 +69,7 @@ class CORE_EXPORT LayoutFullScreen final : public LayoutFlexibleBox {
LayoutBlockFlow* m_placeholder; LayoutBlockFlow* m_placeholder;
ItemPosition selfAlignmentNormalBehavior( ItemPosition selfAlignmentNormalBehavior(
const LayoutBox* child = nullptr) const override { const LayoutBox* child = nullptr) const override {
DCHECK(!child);
return ItemPositionCenter; return ItemPositionCenter;
} }
}; };
......
third_party/WebKit/Source/core/layout/LayoutTable.cpp
View file @ 772559ed
...@@ -297,21 +297,17 @@ void LayoutTable::updateLogicalWidth() { ...@@ -297,21 +297,17 @@ void LayoutTable::updateLogicalWidth() {
availableContentLogicalWidth = shrinkLogicalWidthToAvoidFloats( availableContentLogicalWidth = shrinkLogicalWidthToAvoidFloats(
marginStart, marginEnd, toLayoutBlockFlow(cb)); marginStart, marginEnd, toLayoutBlockFlow(cb));
if (hasStretchedLogicalWidth()) { // Ensure we aren't bigger than our available width.
setLogicalWidth(availableContentLogicalWidth); LayoutUnit maxWidth = maxPreferredLogicalWidth();
} else { // scaledWidthFromPercentColumns depends on m_layoutStruct in
// Ensure we aren't bigger than our available width. // TableLayoutAlgorithmAuto, which maxPreferredLogicalWidth fills in. So
LayoutUnit maxWidth = maxPreferredLogicalWidth(); // scaledWidthFromPercentColumns has to be called after
// scaledWidthFromPercentColumns depends on m_layoutStruct in // maxPreferredLogicalWidth.
// TableLayoutAlgorithmAuto, which maxPreferredLogicalWidth fills in. So LayoutUnit scaledWidth = m_tableLayout->scaledWidthFromPercentColumns() +
// scaledWidthFromPercentColumns has to be called after bordersPaddingAndSpacingInRowDirection();
// maxPreferredLogicalWidth. maxWidth = std::max(scaledWidth, maxWidth);
LayoutUnit scaledWidth = m_tableLayout->scaledWidthFromPercentColumns() + setLogicalWidth(
bordersPaddingAndSpacingInRowDirection(); LayoutUnit(std::min(availableContentLogicalWidth, maxWidth).floor()));
maxWidth = std::max(scaledWidth, maxWidth);
setLogicalWidth(
LayoutUnit(std::min(availableContentLogicalWidth, maxWidth).floor()));
}
} }
// Ensure we aren't bigger than our max-width style. // Ensure we aren't bigger than our max-width style.
...@@ -469,16 +465,11 @@ void LayoutTable::layoutSection(LayoutTableSection& section, ...@@ -469,16 +465,11 @@ void LayoutTable::layoutSection(LayoutTableSection& section,
LayoutUnit LayoutTable::logicalHeightFromStyle() const { LayoutUnit LayoutTable::logicalHeightFromStyle() const {
LayoutUnit computedLogicalHeight; LayoutUnit computedLogicalHeight;
if (hasOverrideLogicalContentHeight()) { Length logicalHeightLength = style()->logicalHeight();
computedLogicalHeight = overrideLogicalContentHeight(); if (logicalHeightLength.isIntrinsic() ||
} else { (logicalHeightLength.isSpecified() && logicalHeightLength.isPositive())) {
Length logicalHeightLength = style()->logicalHeight(); computedLogicalHeight =
if (logicalHeightLength.isIntrinsic() || convertStyleLogicalHeightToComputedHeight(logicalHeightLength);
(logicalHeightLength.isSpecified() &&
logicalHeightLength.isPositive())) {
computedLogicalHeight =
convertStyleLogicalHeightToComputedHeight(logicalHeightLength);
}
} }
Length logicalMaxHeightLength = style()->logicalMaxHeight(); Length logicalMaxHeightLength = style()->logicalMaxHeight();
......
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