Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[css-flexbox][css-align][css-grid] abspos flex/grid item "align-self: auto behaves as start" spec-text is too vague #440

Closed
dholbert opened this issue Sep 3, 2016 · 19 comments
Labels
Closed Accepted by CSSWG Resolution Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. css-align-3 Current Work

Comments

@dholbert
Copy link
Member

dholbert commented Sep 3, 2016

The css-align spec (the canonical definition for align-self) is pretty unequivocal about how to handle align-self:auto. Quoting:

The auto keyword computes to the computed value of align-items on the parent or normal if the box has no parent.
https://drafts.csswg.org/css-align/#propdef-align-self

But, the flexbox & grid specs attempt to define some special-case behavior for this auto value on absolutely-positioned elements -- and IMO they're too vague about how/where that special case behavior is supposed to be triggered, and how broadly its effects spread.

The language I'm concerned about is here in the flexbox spec:

For this purpose, a value of align-self: auto is treated identically to start.
https://drafts.csswg.org/css-flexbox-1/#abspos-items

...and here in the grid spec:

For the purpose of calculating this static position, a value of auto for align-self/justify-self behaves as start.
https://drafts.csswg.org/css-grid-1/#static-position

Superficially, the phrase "a value of" is too vague. (Is that "a specified value"? "a computed value"? something in between?) And of course, "behaves as" is vague too.

I have these specific questions/concerns:

  1. Are the flex/grid specs are trying to redefine the computed value of align-self:auto for these abspos elements? Or is align-self:auto still intended to compute the way that css-align defines it (to the parent's computed align-items) and then its computed value (e.g. center if the parent has align-items:center) should somehow produce different layout behavior than it normally would?

  2. If the intent for (1) is the latter (i.e. if auto computes away but then ends up somehow producing different results in layout), how is the different behavior intended to be triggered? Layout uses computed/used values of css properties -- so if auto gets computed away, then I don't see how layout would be able to realize that it needs to apply this special-case behavior. Maybe e.g. when auto computes to center, it's expected to really compute to a special center-which-really-came-from-auto value, which is expected to behave just like center except in this special abspos case?

  3. It's not clear to me what should happen here when an align-self:auto value is inherited to an abspos element. For example: suppose an abspos element inside of a flex container has align-self:inherit, and its parent has align-self:auto, and its grandparent has align-items:center. I'm pretty sure this abspos element should end up with a computed align-self of center, BUT: is that really center, or is it the center-which-came-from-auto scenario discussed above in (2)? (This matters for layout -- if we inherit the pure center value, the static position would be center-aligned. But if we inherit some magically tainted center-which-came-from-auto value, then the static position would be start-aligned.)

@fantasai
Copy link
Collaborator

fantasai commented Sep 5, 2016

On 09/03/2016 01:49 PM, Daniel Holbert wrote:

The css-align spec (the canonical definition for |align-self|)
is pretty unequivocal about how to handle |align-self:auto|.
Quoting:

The |auto| keyword computes to the computed value of
|align-items| on the parent or |normal| if the box has no parent.
https://drafts.csswg.org/css-align/#propdef-align-self

But, the flexbox & grid specs attempt to define some special-case
behavior for this |auto| value on absolutely-positioned elements --
and IMO they're too vague about how/where that special case behavior
is supposed to be triggered, and how broadly its effects spread.

The language I'm concerned about is here in the flexbox spec:

For this purpose, a value of |align-self: auto| is treated
identically to |start|.
https://drafts.csswg.org/css-flexbox-1/#abspos-items

...and here in the grid spec:

For the purpose of calculating this static position, a value
of |auto| for |align-self|/|justify-self| behaves as |start|.
https://drafts.csswg.org/css-grid-1/#static-position

Superficially, the phrase "a value of" is too vague. (Is that
"a /specified/ value"? "a /computed/ value"? something in
between?) And of course, "behaves as" is vague too.

IIRC we had backported this behavior in Flex and Grid from Align,
but maybe we screwed up somewhere afterwards. :/

The intention is that absolutely-positioned items never take their
parent's align/justify-items value because they are out-of-flow
and are positioned, often to an indirect ancestor. Honoring the
parent's *-items value is likely to cause confusion.

There's two ways to deal with this:
1.) 'auto' computes to 'normal' on the root and on abspos items,
and to the parent's *-items value on everything else
2.) 'auto' computes to itself always and we do the logic at
used-value time.

I don't think we have a strong opinion on this matter. Computing
through means getComputedStyle() returns a more accurate value;
otherwise it probably doesn't matter much.

~fantasai

@MatsPalmgren
Copy link

Regarding fantasai's suggested solutions.

I think 1) has the same performance problems as we reported here:
https://lists.w3.org/Archives/Public/www-style/2015Sep/0099.html
That is, the computed value for 'align-self' would depend on
its own computed values of 'position', 'display' etc.

  1. would change the meaning of 'inherit', e.g.:
<A style="justify-items:left">
  <B style="justify-items:right">
    <C style="justify-self:inherit">

Currently, this is equivalent to:

<A style="justify-items:left">
  <B style="justify-items:right; justify-self:left">
    <C style="justify-self:left">

With 2) we would instead get the computed values:

<A style="justify-items:left">
  <B style="justify-items:right; justify-self:auto">
    <C style="justify-self:auto">

B would have the used value 'left'.
C would have the used value 'right'.

It seems wrong that B and C ends up having different used values
when the author explicitly specified that C should have the same
justify-self value as B.

I think this solution might work better:
Store an "original-value-was-auto" flag to remember if the value
was 'auto' before it was replaced with its parent's justify-items.

Grid/Flexbox can then use this flag to indicate 'start' for the static
position (and it's only used for that purpose).

I think this would be easy to implement in Gecko (it's similar to
how we retain the original 'display' value for the purposes of
calculating the static position [1]).

[1]
https://drafts.csswg.org/css21/visudet.html#abs-non-replaced-width
"... this hypothetical calculation might require also assuming
a different computed value for 'display'."

@fantasai
Copy link
Collaborator

I have no opinion on this issue... up to the council of implementers and @dbaron . :)

@dholbert
Copy link
Member Author

Looks like this was discussed at https://lists.w3.org/Archives/Public/www-style/2016Sep/0041.html

It was resolved as "RESOLVED: Don't have the computed value dependency", pending any objections.

I think I'm on-board with that, but @fantasai, could you clarify what that means? Is the plan for auto to just always compute to itself now? (and get treated in a context-specific way, depending on flex vs. grid vs. abspos etc.)

@dholbert dholbert reopened this Sep 16, 2016
@fantasai
Copy link
Collaborator

Yes, I believe that is the intent. :)

@dholbert
Copy link
Member Author

dholbert commented Sep 20, 2016

OK -- that sounds good to me, I think. Thanks!

@yisibl
Copy link
Contributor

yisibl commented Nov 25, 2016

I don't understand why align-items:center doesn't work.This is very strange.

qq20161125-2

qq20161125-3

@cbiesinger
Copy link

Was there a pressing need to change this in a way that's incompatible with all shipping implementations (at the time)?

@fantasai
Copy link
Collaborator

fantasai commented Feb 1, 2017

@cbiesinger Yes. When we only had Flexbox, it didn't matter what it computed to on abspos elements, unless they also happened to be children of a flex container. But when we extended align-self/justify-self to all absolutely-positioned elements, we realized that honoring align-items/justify-items on the parent would be a confusing form of action-at-a-distance in the (very common) case of an absolutely-positioned box's containing block being an ancestor that is not its parent. So we needed to break the parent-dependence on absposes.

Lemme know if that makes sense.

@yisibl I am not sure what's going on, given you didn't post any source code, but I'm pretty sure it's unrelated to this bug report...

@yisibl
Copy link
Contributor

yisibl commented Feb 4, 2017

@fantasai Demo: http://jsbin.com/cenanahovu/edit?html

I figured it out, .absolute must be used align-self: center.

Firefox is right.

@cbiesinger
Copy link

@fantasai -- why is it an action at a distance? the static position is always influenced by the parent. But more importantly, this may break existing content -- I am hesitant to change the abspos behavior again.

@tabatkins
Copy link
Member

@cbiesinger This isn't about just static position, tho - align/justify-* affect the abspos even when t/r/b/l are all specified. (The old "t+b means you're stretched" behavior is now explicitly keyed to "justify-self: stretch"; otherwise you just align within the box defined by t+b; same for r+l and align-self.) It is quite weird if an abspos isn't relying on static position at all, and is attached to a containing block somewhere high in the tree, but it still gets aligned by its parent element using "*-items". This is the action-at-a-distance @fantasai is talking about.

However! The behavior change for static position is, I think, unintentional. We can deploy a more targeted fix to preserve that, so that "auto" continues to look at the parent's *-items value. Would that work for you?

@tabatkins tabatkins reopened this Feb 16, 2017
@cbiesinger
Copy link

Yes, that sounds reasonable.

emilio added a commit to emilio/csswg-test that referenced this issue Feb 22, 2017
The flex spec was updated a while ago to use the following language for
align-items:

> align-items sets the default alignment for all of the flex container’s items,
> including anonymous flex items. align-self allows this default alignment to
> be overridden for individual flex items.

After that, the CSSWG resolved that align-self: auto computes to itself[1][2].

Thus, this test is invalid.

[1]: w3c/csswg-drafts#440 (comment)
[2]: https://lists.w3.org/Archives/Public/www-style/2016Sep/0041.html
@emilio
Copy link
Collaborator

emilio commented Feb 22, 2017

The language introduced in f5d5826 seems problematic to me:

The auto keyword is interpreted as normal if the box is absolutely positioned or has no parent, and as the computed value of justify-items on the parent (minus any legacy keywords) otherwise.

What is the parent? The DOM parent? Or the parent box? How should the <span>s align-items act as?

<style>
#t4 { display: flex; align-items: center }
#t4 .contents { display: contents; align-items: baseline }
#t4 span { align-self: auto }
</style>
<div id="t4"><div class="contents"><span></span></div></div>

It seems to me that the intention is just to use the parent box align-items (not the parent element's), right? If so, the spec should be changed to clarify that, given that in the current state it could conflict with the description of align-items:

This property specifies the default align-self for all of the boxes (including anonymous boxes) participating in this box’s formatting context. Values have the following meanings:

@fantasai
Copy link
Collaborator

Fixed @emilio‘s issue. Agenda+ to discuss #440 (comment)

gsnedders pushed a commit to jgraham/css-test-built that referenced this issue Mar 2, 2017
The flex spec was updated a while ago to use the following language for
align-items:

> align-items sets the default alignment for all of the flex container’s items,
> including anonymous flex items. align-self allows this default alignment to
> be overridden for individual flex items.

After that, the CSSWG resolved that align-self: auto computes to itself[1][2].

Thus, this test is invalid.

[1]: w3c/csswg-drafts#440 (comment)
[2]: https://lists.w3.org/Archives/Public/www-style/2016Sep/0041.html

Build from revision af7d6f4901eab45cdeaee531daac8c4d360d4a5d
tabatkins added a commit that referenced this issue Mar 14, 2017
…to and abspos static position; Align now defines this correctly. #440.
@fantasai
Copy link
Collaborator

fantasai commented May 2, 2017

@cbiesinger @dholbert Does the updated text look OK to you? 9d16e5f

@cbiesinger
Copy link

Looks good to me, thanks!

jeffcarp pushed a commit to web-platform-tests/wpt that referenced this issue May 30, 2017
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}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue May 30, 2017
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}
scheib pushed a commit to scheib/chromium that referenced this issue May 30, 2017
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}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue May 30, 2017
… (patchset #12 id:210001 of https://codereview.chromium.org/2455093002/ )

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
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=725489

Review-Url: https://codereview.chromium.org/2913093002
Cr-Commit-Position: refs/heads/master@{#475689}
MXEBot pushed a commit to mirror/chromium that referenced this issue May 31, 2017
… (patchset #12 id:210001 of https://codereview.chromium.org/2455093002/ )

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
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=725489

Review-Url: https://codereview.chromium.org/2913093002
Cr-Commit-Position: refs/heads/master@{#475689}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue May 31, 2017
… (patchset #12 id:210001 of https://codereview.chromium.org/2455093002/ )

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
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=725489

Review-Url: https://codereview.chromium.org/2913093002
Cr-Commit-Position: refs/heads/master@{#475689}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue May 31, 2017
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}
jeffcarp pushed a commit to web-platform-tests/wpt that referenced this issue May 31, 2017
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}
MXEBot pushed a commit to mirror/chromium that referenced this issue Jun 1, 2017
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}
@dholbert
Copy link
Member Author

dholbert commented Jun 2, 2017

Looks good to me too - thanks!

hubot pushed a commit to WebKit/WebKit-http that referenced this issue Jul 11, 2017
…fy-self must not be resolved

https://bugs.webkit.org/show_bug.cgi?id=172707

Reviewed by Antti Koivisto.

LayoutTests/imported/w3c:

This change makes all the cases of the test below to pass now, hence updated expectations accordingly.

* web-platform-tests/css/css-align-3/self-alignment/place-self-shorthand-006-expected.txt:

Source/WebCore:

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

No new tests, just updating the already defined tests.

* css/CSSComputedStyleDeclaration.cpp:
(WebCore::ComputedStyleExtractor::propertyValue):
* css/StyleResolver.cpp:
(WebCore::StyleResolver::adjustRenderStyle): Removed
* css/StyleResolver.h:
* html/shadow/TextControlInnerElements.cpp:
(WebCore::TextControlInnerElement::resolveCustomStyle):
* rendering/RenderBox.cpp:
(WebCore::RenderBox::columnFlexItemHasStretchAlignment):
(WebCore::RenderBox::hasStretchedLogicalWidth):
* rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::styleDidChange): Added
(WebCore::RenderFlexibleBox::alignmentForChild):
* rendering/RenderFlexibleBox.h:

LayoutTests:

Updated layout tests so that resolved value is as specified, even for 'auto' values.

* TestExpectations:
* css3/flexbox/css-properties-expected.txt:
* css3/flexbox/css-properties.html:
* css3/flexbox/new-alignment-values-invalid-if-grid-not-enabled-expected.txt:
* css3/flexbox/new-alignment-values-invalid-if-grid-not-enabled.html:
* css3/parse-align-self.html:
* css3/parse-alignment-of-root-elements-expected.txt:
* css3/parse-alignment-of-root-elements.html:
* css3/parse-place-items.html:
* css3/parse-place-self.html:
* fast/css/ensure-flexbox-compatibility-with-initial-values-expected.txt:
* fast/css/parse-justify-self.html:


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@219315 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@fantasai fantasai added Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. and removed Commenter Response Pending css-flexbox-1 Current Work css-grid-1 labels Oct 17, 2017
ryanhaddad pushed a commit to WebKit/WebKit that referenced this issue Dec 22, 2020
…fy-self must not be resolved

https://bugs.webkit.org/show_bug.cgi?id=172707

Reviewed by Antti Koivisto.

LayoutTests/imported/w3c:

This change makes all the cases of the test below to pass now, hence updated expectations accordingly.

* web-platform-tests/css/css-align-3/self-alignment/place-self-shorthand-006-expected.txt:

Source/WebCore:

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

No new tests, just updating the already defined tests.

* css/CSSComputedStyleDeclaration.cpp:
(WebCore::ComputedStyleExtractor::propertyValue):
* css/StyleResolver.cpp:
(WebCore::StyleResolver::adjustRenderStyle): Removed
* css/StyleResolver.h:
* html/shadow/TextControlInnerElements.cpp:
(WebCore::TextControlInnerElement::resolveCustomStyle):
* rendering/RenderBox.cpp:
(WebCore::RenderBox::columnFlexItemHasStretchAlignment):
(WebCore::RenderBox::hasStretchedLogicalWidth):
* rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::styleDidChange): Added
(WebCore::RenderFlexibleBox::alignmentForChild):
* rendering/RenderFlexibleBox.h:

LayoutTests:

Updated layout tests so that resolved value is as specified, even for 'auto' values.

* TestExpectations:
* css3/flexbox/css-properties-expected.txt:
* css3/flexbox/css-properties.html:
* css3/flexbox/new-alignment-values-invalid-if-grid-not-enabled-expected.txt:
* css3/flexbox/new-alignment-values-invalid-if-grid-not-enabled.html:
* css3/parse-align-self.html:
* css3/parse-alignment-of-root-elements-expected.txt:
* css3/parse-alignment-of-root-elements.html:
* css3/parse-place-items.html:
* css3/parse-place-self.html:
* fast/css/ensure-flexbox-compatibility-with-initial-values-expected.txt:
* fast/css/parse-justify-self.html:


Canonical link: https://commits.webkit.org/191158@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@219315 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Closed Accepted by CSSWG Resolution Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. css-align-3 Current Work
8 participants