Skip to content
This repository has been archived by the owner on Jul 18, 2018. It is now read-only.

Commit

Permalink
Reland of [css-align] Don't resolve 'auto' values for computed style.
Browse files Browse the repository at this point in the history
Fixed the regression caused by the previous patch and provided a proper
regression test.

Reason for revert:
This patch breaks DevTools toolbars (Console gear icon, checkbox labels are centered), it also seems to break the Welcome to Chrome page layout.

Original issue's description:
> [css-align] Don't resolve 'auto' values for computed style.
>
> The CSS Box Alignment specification has been changed recently so that
> now all the propeties have the specificed value as computed value. The
> rationale of this change are at the associated W3C github issue [1].
>
> This change implies that we don't need to execute the StyleAdjuter
> logic we implemented specifically for supporting 'auto' values
> resolution for computed style. We can live now with resolution at
> layout time only.
>
> [1] w3c/csswg-drafts#440
>
> BUG=725489
>
> Review-Url: https://codereview.chromium.org/2455093002
> Cr-Commit-Position: refs/heads/master@{#475400}
> Committed: https://chromium.googlesource.com/chromium/src/+/5389373c6dec30d783eb46b4c8190720f411a8a7

TBR=cbiesinger@chromium.org,cbiesinger@google.com,meade@chromium.org,rego@igalia.com,svillar@igalia.com,jfernandez@igalia.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=725489
Review-Url: https://codereview.chromium.org/2913093002
Cr-Commit-Position: refs/heads/master@{#475689}
Committed: https://chromium.googlesource.com/chromium/src/+/8e4d09ab6b864897d8399ccda555a61b030ceb84

BUG=725489

Review-Url: https://codereview.chromium.org/2915773002
Cr-Commit-Position: refs/heads/master@{#476024}
  • Loading branch information
javifernandez authored and Commit Bot committed May 31, 2017
1 parent 30333e4 commit d10fc6a
Show file tree
Hide file tree
Showing 30 changed files with 330 additions and 283 deletions.
4 changes: 0 additions & 4 deletions third_party/WebKit/LayoutTests/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -2677,10 +2677,6 @@ crbug.com/707359 [ Mac ] fast/css-grid-layout/grid-self-baseline-vertical-rl-03.
crbug.com/707359 [ Mac ] fast/css-grid-layout/grid-self-baseline-vertical-rl-04.html [ Failure ]
crbug.com/707359 [ Mac ] fast/css-grid-layout/grid-self-baseline-vertical-rl-05.html [ Failure ]

# [css-align]
crbug.com/668639 external/wpt/css/css-align-3/default-alignment/place-items-shorthand-006.html [ Failure ]
crbug.com/668639 external/wpt/css/css-align-3/self-alignment/place-self-shorthand-006.html [ Failure ]

# [selectors-4]
crbug.com/706118 external/wpt/css/selectors4/hover-001-manual.html [ Skip ]
crbug.com/576815 external/wpt/css/selectors4/selectors-dir-selector-ltr-001.html [ Failure ]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ PASS window.getComputedStyle(flexbox, null).justifyContent is "space-evenly"
PASS flexbox.style.justifyContent is ""
PASS window.getComputedStyle(flexbox, null).justifyContent is "normal"
PASS flexbox.style.alignSelf is ""
PASS window.getComputedStyle(flexbox, null).alignSelf is "normal"
PASS window.getComputedStyle(document.documentElement, null).alignSelf is "normal"
PASS window.getComputedStyle(flexbox, null).alignSelf is "auto"
PASS window.getComputedStyle(document.documentElement, null).alignSelf is "auto"
PASS flexbox.style.alignSelf is ""
PASS window.getComputedStyle(flexbox, null).alignSelf is "normal"
PASS window.getComputedStyle(flexbox, null).alignSelf is "auto"
PASS flexbox.style.alignSelf is "auto"
PASS window.getComputedStyle(flexbox, null).alignSelf is "normal"
PASS window.getComputedStyle(flexbox, null).alignSelf is "auto"
PASS flexbox.style.alignSelf is "flex-start"
PASS window.getComputedStyle(flexbox, null).alignSelf is "flex-start"
PASS flexbox.style.alignSelf is "flex-end"
Expand All @@ -48,41 +48,41 @@ PASS window.getComputedStyle(flexbox, null).alignSelf is "stretch"
PASS flexbox.style.alignSelf is "baseline"
PASS window.getComputedStyle(flexbox, null).alignSelf is "baseline"
PASS flexbox.style.alignSelf is ""
PASS window.getComputedStyle(flexbox, null).alignSelf is "normal"
PASS window.getComputedStyle(flexbox, null).alignSelf is "auto"
PASS flexbox.style.alignItems is ""
PASS flexitem.style.alignSelf is ""
PASS window.getComputedStyle(flexbox, null).alignItems is "normal"
PASS window.getComputedStyle(flexitem, null).alignSelf is "normal"
PASS window.getComputedStyle(flexitem, null).alignSelf is "auto"
PASS flexbox.style.alignItems is ""
PASS flexitem.style.alignSelf is ""
PASS window.getComputedStyle(flexbox, null).alignItems is "normal"
PASS window.getComputedStyle(flexitem, null).alignSelf is "normal"
PASS window.getComputedStyle(flexitem, null).alignSelf is "auto"
PASS flexbox.style.alignItems is ""
PASS flexitem.style.alignSelf is ""
PASS window.getComputedStyle(flexbox, null).alignItems is "normal"
PASS window.getComputedStyle(flexitem, null).alignSelf is "normal"
PASS window.getComputedStyle(flexitem, null).alignSelf is "auto"
PASS flexbox.style.alignItems is "flex-start"
PASS flexitem.style.alignSelf is ""
PASS window.getComputedStyle(flexbox, null).alignItems is "flex-start"
PASS window.getComputedStyle(flexitem, null).alignSelf is "flex-start"
FAIL window.getComputedStyle(flexitem, null).alignSelf should be flex-start. Was auto.
PASS flexbox.style.alignItems is "flex-end"
PASS window.getComputedStyle(flexbox, null).alignItems is "flex-end"
PASS window.getComputedStyle(flexitem, null).alignSelf is "flex-end"
FAIL window.getComputedStyle(flexitem, null).alignSelf should be flex-end. Was auto.
PASS flexbox.style.alignItems is "center"
PASS window.getComputedStyle(flexbox, null).alignItems is "center"
PASS window.getComputedStyle(flexitem, null).alignSelf is "center"
FAIL window.getComputedStyle(flexitem, null).alignSelf should be center. Was auto.
PASS flexbox.style.alignItems is "stretch"
PASS window.getComputedStyle(flexbox, null).alignItems is "stretch"
PASS window.getComputedStyle(flexitem, null).alignSelf is "stretch"
FAIL window.getComputedStyle(flexitem, null).alignSelf should be stretch. Was auto.
PASS flexbox.style.alignItems is "baseline"
PASS window.getComputedStyle(flexbox, null).alignItems is "baseline"
PASS window.getComputedStyle(flexitem, null).alignSelf is "baseline"
FAIL window.getComputedStyle(flexitem, null).alignSelf should be baseline. Was auto.
PASS flexbox.style.alignItems is ""
PASS window.getComputedStyle(flexbox, null).alignItems is "normal"
PASS window.getComputedStyle(flexitem, null).alignSelf is "normal"
PASS window.getComputedStyle(flexitem, null).alignSelf is "auto"
PASS flexbox.style.alignItems is ""
PASS window.getComputedStyle(flexbox, null).alignItems is "normal"
PASS window.getComputedStyle(flexitem, null).alignSelf is "normal"
PASS window.getComputedStyle(flexitem, null).alignSelf is "auto"
PASS window.getComputedStyle(detachedFlexbox, null).alignSelf is ""
PASS window.getComputedStyle(detachedFlexItem, null).alignSelf is ""
PASS flexbox.style.flexDirection is ""
Expand Down
24 changes: 12 additions & 12 deletions third_party/WebKit/LayoutTests/css3/flexbox/css-properties.html
Original file line number Diff line number Diff line change
Expand Up @@ -84,17 +84,17 @@
shouldBeEqualToString('window.getComputedStyle(flexbox, null).justifyContent', 'normal');

shouldBeEqualToString('flexbox.style.alignSelf', '');
// The initial value is 'auto', which will be resolved depending on parent's style (except for the 'document' element).
shouldBeEqualToString('window.getComputedStyle(flexbox, null).alignSelf', 'normal');
shouldBeEqualToString('window.getComputedStyle(document.documentElement, null).alignSelf', 'normal');
// The initial value is 'auto'.
shouldBeEqualToString('window.getComputedStyle(flexbox, null).alignSelf', 'auto');
shouldBeEqualToString('window.getComputedStyle(document.documentElement, null).alignSelf', 'auto');

flexbox.style.alignSelf = 'foo';
shouldBeEqualToString('flexbox.style.alignSelf', '');
shouldBeEqualToString('window.getComputedStyle(flexbox, null).alignSelf', 'normal');
shouldBeEqualToString('window.getComputedStyle(flexbox, null).alignSelf', 'auto');

flexbox.style.alignSelf = 'auto';
shouldBeEqualToString('flexbox.style.alignSelf', 'auto');
shouldBeEqualToString('window.getComputedStyle(flexbox, null).alignSelf', 'normal');
shouldBeEqualToString('window.getComputedStyle(flexbox, null).alignSelf', 'auto');

flexbox.style.alignSelf = 'flex-start';
shouldBeEqualToString('flexbox.style.alignSelf', 'flex-start');
Expand All @@ -118,26 +118,26 @@

flexbox.style.alignSelf = '';
shouldBeEqualToString('flexbox.style.alignSelf', '');
shouldBeEqualToString('window.getComputedStyle(flexbox, null).alignSelf', 'normal');
shouldBeEqualToString('window.getComputedStyle(flexbox, null).alignSelf', 'auto');

shouldBeEqualToString('flexbox.style.alignItems', '');
shouldBeEqualToString('flexitem.style.alignSelf', '');
// The initial value is 'auto', which will be resolved to 'normal' in case of flexbox containers.
// The initial value is 'auto'.
shouldBeEqualToString('window.getComputedStyle(flexbox, null).alignItems', 'normal');
shouldBeEqualToString('window.getComputedStyle(flexitem, null).alignSelf', 'normal');
shouldBeEqualToString('window.getComputedStyle(flexitem, null).alignSelf', 'auto');

flexbox.style.alignItems = 'foo';
shouldBeEqualToString('flexbox.style.alignItems', '');
shouldBeEqualToString('flexitem.style.alignSelf', '');
shouldBeEqualToString('window.getComputedStyle(flexbox, null).alignItems', 'normal');
shouldBeEqualToString('window.getComputedStyle(flexitem, null).alignSelf', 'normal');
shouldBeEqualToString('window.getComputedStyle(flexitem, null).alignSelf', 'auto');

// The 'auto' value is not valid for the align-items property.
flexbox.style.alignItems = 'auto';
shouldBeEqualToString('flexbox.style.alignItems', '');
shouldBeEqualToString('flexitem.style.alignSelf', '');
shouldBeEqualToString('window.getComputedStyle(flexbox, null).alignItems', 'normal');
shouldBeEqualToString('window.getComputedStyle(flexitem, null).alignSelf', 'normal');
shouldBeEqualToString('window.getComputedStyle(flexitem, null).alignSelf', 'auto');

flexbox.style.alignItems = 'flex-start';
shouldBeEqualToString('flexbox.style.alignItems', 'flex-start');
Expand Down Expand Up @@ -168,12 +168,12 @@
flexbox.style.alignItems = '';
shouldBeEqualToString('flexbox.style.alignItems', '');
shouldBeEqualToString('window.getComputedStyle(flexbox, null).alignItems', 'normal');
shouldBeEqualToString('window.getComputedStyle(flexitem, null).alignSelf', 'normal');
shouldBeEqualToString('window.getComputedStyle(flexitem, null).alignSelf', 'auto');

flexbox.style.display = 'none';
shouldBeEqualToString('flexbox.style.alignItems', '');
shouldBeEqualToString('window.getComputedStyle(flexbox, null).alignItems', 'normal');
shouldBeEqualToString('window.getComputedStyle(flexitem, null).alignSelf', 'normal');
shouldBeEqualToString('window.getComputedStyle(flexitem, null).alignSelf', 'auto');
flexbox.style.display = 'flex';


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<script>
var values = ["normal", "stretch"].concat(selfPositionValues, baselineValues);
values.forEach(function(alignValue) {
["", "auto"].concat(values).forEach(function(justifyValue) {
[""].concat(values).forEach(function(justifyValue) {
var value = (alignValue + " " + justifyValue).trim();
test(function() {
checkPlaceShorhand("place-items", alignValue, justifyValue)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<!DOCTYPE html PUBLIC "-//W3C//DTD//XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<title>CSS Test: A flex container with 'column' direction and 'align-items' property set to 'flex-start'</title>
<link rel="author" title="Javier Fernandez Garcia-Boente" href="mailto:jfernandez@igalia.com">
<link rel="help" href="http://www.w3.org/TR/css-flexbox-1/#align-items-property" />
<link rel="help" href="http://www.w3.org/TR/css-flexbox-1/#flex-direction-property" />
<link rel="match" href="reference/align-content-001-ref.html" />
<meta name="assert" content="This test checks that 'align-items: flex-start' implies the flex item's 'auto' width to be resolved as 'fit-content'." />
<style type="text/css">
.block {
position: absolute;
width: 300px;
height: 100px;
}
#flexbox
{
font: 50px/1 Ahem;
background: green;
flex-direction: column;
align-items: flex-start;
display: flex;
width: 300px;
height: 100px;
}
</style>
</head>
<body>
<p>Test passes if there is no red visible on the page.</p>
<div class="block">
<div style="width: 200px; height: 50px; background: green"></div>
</div>
<div id="flexbox">
<div style="background: red; color: red">XXXX</div>
</div>
</body>
</html>
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,18 @@
}
.grid { display: grid; }
.flex { display: flex; }
.itemsCenter {
align-items: center;
justify-items: center;
.selfCenter {
align-self: center;
justify-self: center;
}
</style>

<p>Grid item created as anonymous box to contain the text element. The text element should be centered in both axis due to the Default Alignment 'center' value on its container.</p>
<div class="container grid itemsCenter">
<div>foobar</div>
<div class="container grid">
<div class="selfCenter">foobar</div>
</div>

<p>Flex item created as anonymous box to contain the text element. The text element should be centered in both axis due to the Default Alignment 'center' value on its container.</p>
<div class="container flex itemsCenter">
<div>foobar</div>
<p>Flex item created as anonymous box to contain the text element. The text element should be centered in cross axis due to the Default Alignment 'center' value on its container.</p>
<div class="container flex">
<div class="selfCenter">foobar</div>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
foobar
</div>

<p>Flex item created as anonymous box to contain the text element. The text element should be centered in both axis due to the Default Alignment 'center' value on its container.</p>
<p>Flex item created as anonymous box to contain the text element. The text element should be centered in cross axis due to the Default Alignment 'center' value on its container.</p>
<div class="container flex itemsCenter">
foobar
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -6,37 +6,37 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE

Verifying initial values are supported when grid is ENABLED.
PASS CSS.supports('align-items', 'normal') is true
PASS CSS.supports('align-self', 'normal') is true
PASS CSS.supports('align-self', 'auto') is true
PASS CSS.supports('align-content', 'normal') is true
PASS CSS.supports('justify-content', 'normal') is true
PASS CSS.supports('align-items', 'normal') is true
PASS CSS.supports('align-self', 'normal') is true
PASS CSS.supports('align-self', 'auto') is true
PASS CSS.supports('align-content', 'normal') is true
PASS CSS.supports('justify-content', 'normal') is true
PASS CSS.supports('align-items', 'normal') is true
PASS CSS.supports('align-self', 'normal') is true
PASS CSS.supports('align-self', 'auto') is true
PASS CSS.supports('align-content', 'normal') is true
PASS CSS.supports('justify-content', 'normal') is true
PASS CSS.supports('align-items', 'normal') is true
PASS CSS.supports('align-self', 'normal') is true
PASS CSS.supports('align-self', 'auto') is true
PASS CSS.supports('align-content', 'normal') is true
PASS CSS.supports('justify-content', 'normal') is true

Verifying initial values are supported when grid is DISABLED.
PASS CSS.supports('align-items', 'stretch') is true
PASS CSS.supports('align-self', 'stretch') is true
PASS CSS.supports('align-self', 'auto') is true
PASS CSS.supports('align-content', 'stretch') is true
PASS CSS.supports('justify-content', 'flex-start') is true
PASS CSS.supports('align-items', 'stretch') is true
PASS CSS.supports('align-self', 'stretch') is true
PASS CSS.supports('align-self', 'auto') is true
PASS CSS.supports('align-content', 'stretch') is true
PASS CSS.supports('justify-content', 'flex-start') is true
PASS CSS.supports('align-items', 'stretch') is true
PASS CSS.supports('align-self', 'stretch') is true
PASS CSS.supports('align-self', 'auto') is true
PASS CSS.supports('align-content', 'stretch') is true
PASS CSS.supports('justify-content', 'flex-start') is true
PASS CSS.supports('align-items', 'stretch') is true
PASS CSS.supports('align-self', 'stretch') is true
PASS CSS.supports('align-self', 'auto') is true
PASS CSS.supports('align-content', 'stretch') is true
PASS CSS.supports('justify-content', 'flex-start') is true
PASS successfullyParsed is true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@
container.style.webkitAlignItems = value;
if (gridEnabled) {
checkValues(container, "alignItems", "align-items", value, computedValue);
checkValues(item, "alignSelf", "align-self", "auto", computedValue);
checkValues(item, "alignSelf", "align-self", "auto", "auto");
} else {
checkValues(container, "alignItems", "align-items", "flex-end", "flex-end");
checkValues(item, "alignSelf", "align-self", "auto", "flex-end");
checkValues(item, "alignSelf", "align-self", "auto", "auto");
}
}

Expand Down
Loading

0 comments on commit d10fc6a

Please sign in to comment.