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-align][css-flex] Doubt about baseline alignment #3416

Closed
mrego opened this Issue Dec 10, 2018 · 5 comments

Comments

Projects
None yet
3 participants
@mrego
Copy link
Member

mrego commented Dec 10, 2018

Some links to the specs:

I've some doubts about how baseline alignment should work in the case of flex vs inline-flex.
Let's compare the behavior of block vs inline-block with flex vs inline-flex in the following examples.

<h2>Case 1)</h2>
foo
<div style="display: inline-block; margin-bottom: 50px;">
  <div style="display: inline-block; width: 50px; height: 50px; background: magenta;">
  </div>
</div>
bar
<div style="display: inline-block; margin-bottom: 50px;">
  <div style="display: block; width: 50px; height: 50px; background: cyan;">
  </div>
</div>
baz

Output of case 1)

For this example all browsers behave the same.
In this case the magenta box is an inline-block which determines the baseline of the outer box, so the 50px margin are not taking into account to compute the baseline.
However the cyan box is a block, in that case it's not used to compute the baseline, and the 50px margin on the outer box are taken into account for the baseline.

<h2>Case 2)</h2>
foo
<div style="display: inline-block; margin-bottom: 50px;">
  <div style="display: inline-flex; width: 50px; height: 50px; background: magenta;">
  </div>
</div>
bar
<div style="display: inline-block; margin-bottom: 50px;">
  <div style="display: flex; width: 50px; height: 50px; background: cyan;">
  </div>
</div>
baz

Output of case 2)

Here in Chromium/WebKit the flex case (cyan) behaves the same than the flex-inline (magenta). However in Firefox/Edge it works the same than the inline-block vs block case.
I believe Firefox/Edge are the ones right in this case.

But let's see what happens when we add one level more:

<h2>Case 3)</h2>
foo
<div style="display: inline-block; margin-bottom: 50px;">
  <div style="display: inline-block;">
    <div style="width: 50px; height: 50px; background: magenta;">
    </div>
  </div>
</div>
bar
<div style="display: inline-block; margin-bottom: 50px;">
  <div style="display: block;">
    <div style="width: 50px; height: 50px; background: cyan;">
    </div>
  </div>
</div>
baz

Output of case 3)

This still works the same than the case 1) and behavior is the same in all browsers.

But what happens with flexbox:

<h2>Case 4)</h2>
foo
<div style="display: inline-block; margin-bottom: 50px;">
  <div style="display: inline-flex;">
    <div style="width: 50px; height: 50px; background: magenta;">
    </div>
  </div>
</div>
bar
<div style="display: inline-block; margin-bottom: 50px;">
  <div style="display: flex;">
    <div style="width: 50px; height: 50px; background: cyan;">
    </div>
  </div>
</div>
baz

Output of case 4)

In this case all browsers mach again, and cyan box works the same than magenta box.
The 50px margin is not used to compute the baseline.

TBH, I'm not really sure why case 3) is different from 4). Or if behavior in 4) is right or not.
You can play with this examples live at: https://jsbin.com/tetatemiqo/1/edit?html,output

@mrego

This comment has been minimized.

Copy link
Member Author

mrego commented Dec 17, 2018

I want to fix the interop issue between Firefox and Blink/WebKit for case 2) where Firefox is right (see Chromium bug #671132).
However I'd love to clarify the case 4) just in case it needs or not some behavior change.

@javifernandez

This comment has been minimized.

Copy link
Contributor

javifernandez commented Dec 19, 2018

Just a comment regarding Case 2; I think Chromium/WebKit are correct in this case, due to this statement of the Flex spec:

https://drafts.csswg.org/css-flexbox-1/#flex-baselines

Otherwise, if the flex container has at least one flex item, the flex container’s first/last main-axis baseline set is generated from the alignment baseline of the startmost/endmost flex item. (If that item has no alignment baseline parallel to the flex container’s main axis, then one is first synthesized from its border edges.)

So, the baseline of the inline-flex container is determined by its only flex item, which has no natural baseline, so it's synthesized using its border edge.

@mrego

This comment has been minimized.

Copy link
Member Author

mrego commented Dec 19, 2018

In case 2) there is no flex item, so I don't think that @javifernandez's explanation is right.

My reasoning regarding case 2) is that the inline-block lacks any inline children, so it uses its synthetized baseline, which in this case is the margin box, thus the 50px bottom margin is considered in order to align it.
For the same reason I think that case 4) could be similar, the inline-block has any inline children so it should use its margin box (but maybe in that case it applies that the flex container uses the baseline from its item).

BTW all these cases are the same using grid layout instead of flexbox.

@mrego

This comment has been minimized.

Copy link
Member Author

mrego commented Mar 21, 2019

After talking to @dholbert during the last CSWG F2F meeting in San Francisco, I agree with him that Firefox behavior is right in all the cases.

The relevant spec text is:

  • Old version from CSS2:

    The baseline of an 'inline-block' is the baseline of its last line box in the normal flow, unless it has either no in-flow line boxes or if its 'overflow' property has a computed value other than 'visible', in which case the baseline is the bottom margin edge.

  • New text from css-align:

    However, for legacy reasons, if an inline-block is a scroll container or contains no in-flow line boxes, its first and last baseline sets are synthesized from its margin box.

Case 1) All browsers do the same. When we have an inline-block wrapping an empty block, it's considered it doesn't have in-flow line boxes, and that's the reason why the margin box of the inline-blockis used to determine the baseline.

Case 2) The same kind of reasoning applies in this case. The wrapping inline-block considers that it has no in-flow line boxes, because its content is just an empty flexbox container. So it should use the margin box to determine the baseline (like Firefox does).

Case 3) Here all browsers agree again. The inner block has no baseline due to the following text from the spec:

If there is no such line box or child, then the block container has no baseline set.

As it has no baseline, all browsers follow the same bahavior than Case 1).

Case 4) Here Flexbox spec enters:

Otherwise, if the flex container has at least one flex item, the flex container’s first/last main-axis baseline set is generated from the alignment baseline of the startmost/endmost flex item.

As the flexbox container has one children, it's baseline is generated from that children. For that reason the margin box of the inline-block is not used to determine the baseline.

Please @dholbert comment or clarify antyhing here, as articulating all this with words is kind of complicated.

I have a patch to make Chromium match Firefox in all these cases, so I hope the explanation above is right and this is the expected behavior.

So, I'm closing this unless someone doesn't agree and wants to reopen it and provide some extra feedback/arguments on this topic.

@mrego mrego closed this Mar 21, 2019

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Mar 22, 2019

[css-flex][css-grid] Fix synthesized baseline
When a flex or grid container has no baseline,
its baseline should be synthesized from the border edges.
The same happens for flex and grid items.

Right now we were using the content box in some cases
and even using the margin box in a particular scenario.
The patch fixes this and update the existent tests
to the new behavior. Three new tests are added to WPT too.

At the same time this is also fixing the baseline for
inline flex/grid containers to make it interoperable with Firefox.
Inline blocks have a special behavior per legacy reasons,
which applies to inline flex/grid containers when they have no items;
otherwise the items should be used to compute its baseline.
See more at: w3c/csswg-drafts#3416

BUG=659610,904562
TEST=external/wpt/css/css-align/baseline-rules/synthesized-baseline-flexbox-001.html
TEST=external/wpt/css/css-align/baseline-rules/synthesized-baseline-grid-001.html
TEST=external/wpt/css/css-align/baseline-rules/synthesized-baseline-inline-block-001.html

Change-Id: Ic11fbfc0a6ab9252568ea1734dcbbcbc3dbeb68c

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Mar 22, 2019

[css-flex][css-grid] Fix synthesized baseline
When a flex or grid container has no baseline,
its baseline should be synthesized from the border edges.
The same happens for flex and grid items.

Right now we were using the content box in some cases
and even using the margin box in a particular scenario.
The patch fixes this and update the existent tests
to the new behavior. Three new tests are added to WPT too.

At the same time this is also fixing the baseline for
inline flex/grid containers to make it interoperable with Firefox.
Inline blocks have a special behavior per legacy reasons,
which applies to inline flex/grid containers when they have no items;
otherwise the items should be used to compute its baseline.
See more at: w3c/csswg-drafts#3416

BUG=659610,671132
TEST=external/wpt/css/css-align/baseline-rules/synthesized-baseline-flexbox-001.html
TEST=external/wpt/css/css-align/baseline-rules/synthesized-baseline-grid-001.html
TEST=external/wpt/css/css-align/baseline-rules/synthesized-baseline-inline-block-001.html

Change-Id: Ic11fbfc0a6ab9252568ea1734dcbbcbc3dbeb68c
@dholbert

This comment has been minimized.

Copy link
Member

dholbert commented Mar 22, 2019

@mrego, your summary makes sense to me, and matches my recollection of our discussion. Thanks!

aarongable pushed a commit to chromium/chromium that referenced this issue Mar 25, 2019

[css-flex][css-grid] Fix synthesized baseline
When a flex or grid container has no baseline,
its baseline should be synthesized from the border edges.
The same happens for flex and grid items.

Right now we were using the content box in some cases
and even using the margin box in a particular scenario.
The patch fixes this and update the existent tests
to the new behavior. Three new tests are added to WPT too.

At the same time this is also fixing the baseline for
inline flex/grid containers to make it interoperable with Firefox.
Inline blocks have a special behavior per legacy reasons,
which applies to inline flex/grid containers when they have no items;
otherwise the items should be used to compute its baseline.
See more at: w3c/csswg-drafts#3416

BUG=659610,671132
TEST=external/wpt/css/css-align/baseline-rules/synthesized-baseline-flexbox-001.html
TEST=external/wpt/css/css-align/baseline-rules/synthesized-baseline-grid-001.html
TEST=external/wpt/css/css-align/baseline-rules/synthesized-baseline-inline-block-001.html

Change-Id: Ic11fbfc0a6ab9252568ea1734dcbbcbc3dbeb68c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1533952
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Manuel Rego <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#643817}

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Mar 25, 2019

[css-flex][css-grid] Fix synthesized baseline
When a flex or grid container has no baseline,
its baseline should be synthesized from the border edges.
The same happens for flex and grid items.

Right now we were using the content box in some cases
and even using the margin box in a particular scenario.
The patch fixes this and update the existent tests
to the new behavior. Three new tests are added to WPT too.

At the same time this is also fixing the baseline for
inline flex/grid containers to make it interoperable with Firefox.
Inline blocks have a special behavior per legacy reasons,
which applies to inline flex/grid containers when they have no items;
otherwise the items should be used to compute its baseline.
See more at: w3c/csswg-drafts#3416

BUG=659610,671132
TEST=external/wpt/css/css-align/baseline-rules/synthesized-baseline-flexbox-001.html
TEST=external/wpt/css/css-align/baseline-rules/synthesized-baseline-grid-001.html
TEST=external/wpt/css/css-align/baseline-rules/synthesized-baseline-inline-block-001.html

Change-Id: Ic11fbfc0a6ab9252568ea1734dcbbcbc3dbeb68c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1533952
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Manuel Rego <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#643817}

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Mar 25, 2019

[css-flex][css-grid] Fix synthesized baseline
When a flex or grid container has no baseline,
its baseline should be synthesized from the border edges.
The same happens for flex and grid items.

Right now we were using the content box in some cases
and even using the margin box in a particular scenario.
The patch fixes this and update the existent tests
to the new behavior. Three new tests are added to WPT too.

At the same time this is also fixing the baseline for
inline flex/grid containers to make it interoperable with Firefox.
Inline blocks have a special behavior per legacy reasons,
which applies to inline flex/grid containers when they have no items;
otherwise the items should be used to compute its baseline.
See more at: w3c/csswg-drafts#3416

BUG=659610,671132
TEST=external/wpt/css/css-align/baseline-rules/synthesized-baseline-flexbox-001.html
TEST=external/wpt/css/css-align/baseline-rules/synthesized-baseline-grid-001.html
TEST=external/wpt/css/css-align/baseline-rules/synthesized-baseline-inline-block-001.html

Change-Id: Ic11fbfc0a6ab9252568ea1734dcbbcbc3dbeb68c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1533952
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Manuel Rego <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#643817}

xanlpz pushed a commit to xanlpz/webkit that referenced this issue Apr 12, 2019

rego@igalia.com
[css-flex][css-grid] Fix synthesized baseline
https://bugs.webkit.org/show_bug.cgi?id=196312

Reviewed by Javier Fernandez.

LayoutTests/imported/w3c:

Imported some tests from WPT css-align test suite that are fixed with this patch.

* resources/import-expectations.json:
* web-platform-tests/css/css-align/baseline-rules/synthesized-baseline-flexbox-001-expected.txt: Added.
* web-platform-tests/css/css-align/baseline-rules/synthesized-baseline-flexbox-001.html: Added.
* web-platform-tests/css/css-align/baseline-rules/synthesized-baseline-grid-001-expected.txt: Added.
* web-platform-tests/css/css-align/baseline-rules/synthesized-baseline-grid-001.html: Added.
* web-platform-tests/css/css-align/baseline-rules/synthesized-baseline-inline-block-001-expected.txt: Added.
* web-platform-tests/css/css-align/baseline-rules/synthesized-baseline-inline-block-001.html: Added.
* web-platform-tests/css/css-align/baseline-rules/w3c-import.log: Added.

Source/WebCore:

When a flex or grid container has no baseline,
its baseline should be synthesized from the border edges.
The same happens for flex and grid items.

Right now we were using the content box in some cases
and even using the margin box in a particular scenario.
The patch fixes this.

At the same time this is also fixing the baseline for
inline flex/grid containers to make it interoperable with Firefox.
Inline blocks have a special behavior per legacy reasons,
which applies to inline flex/grid containers when they have no items;
otherwise the items should be used to compute its baseline.
See more at: w3c/csswg-drafts#3416

Note that we need to keep current behavior for buttons,
as the flexbox spec doesn't apply to them.

Tests: css3/flexbox/flexbox-baseline-margins.html
       fast/css-grid-layout/grid-baseline-margins-1.html
       fast/css-grid-layout/grid-baseline-margins-2.html
       imported/w3c/web-platform-tests/css/css-align/baseline-rules/synthesized-baseline-flexbox-001.html
       imported/w3c/web-platform-tests/css/css-align/baseline-rules/synthesized-baseline-grid-001.html
       imported/w3c/web-platform-tests/css/css-align/baseline-rules/synthesized-baseline-inline-block-001.html

* rendering/RenderButton.cpp:
(WebCore::synthesizedBaselineFromContentBox):
(WebCore::RenderButton::baselinePosition const):
* rendering/RenderButton.h:
* rendering/RenderFlexibleBox.cpp:
(WebCore::synthesizedBaselineFromBorderBox):
(WebCore::RenderFlexibleBox::baselinePosition const):
(WebCore::RenderFlexibleBox::firstLineBaseline const):
(WebCore::RenderFlexibleBox::inlineBlockBaseline const):
* rendering/RenderGrid.cpp:
(WebCore::RenderGrid::baselinePosition const):
(WebCore::RenderGrid::inlineBlockBaseline const):

LayoutTests:

Some of the tests were not checking the right behavior changed them to test the expected one.
We need new rebaselines for some tests.

* TestExpectations:
* css3/flexbox/flexbox-baseline-margins.html:
* fast/css-grid-layout/grid-baseline-expected.html:
* fast/css-grid-layout/grid-baseline-margins-1-expected.html: Renamed from LayoutTests/fast/css-grid-layout/grid-baseline-margins-expected.html.
* fast/css-grid-layout/grid-baseline-margins-1.html: Renamed from LayoutTests/fast/css-grid-layout/grid-baseline-margins.html.
* fast/css-grid-layout/grid-baseline-margins-2-expected.html: Added.
* fast/css-grid-layout/grid-baseline-margins-2.html: Added.
* fast/css-grid-layout/grid-baseline.html: This test is modified and split in two parts as it doesn't fit in the viewport.
* platform/gtk/css3/flexbox/flexbox-baseline-margins-expected.png:
* platform/gtk/css3/flexbox/flexbox-baseline-margins-expected.txt:
* platform/ios/css3/flexbox/flexbox-baseline-margins-expected.png: Added.
* platform/ios/css3/flexbox/flexbox-baseline-margins-expected.txt:
* platform/mac/css3/flexbox/flexbox-baseline-margins-expected.png: Added.
* platform/mac/css3/flexbox/flexbox-baseline-margins-expected.txt:
* platform/win/css3/flexbox/flexbox-baseline-margins-expected.png: Added.
* platform/win/css3/flexbox/flexbox-baseline-margins-expected.txt:


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@244213 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.