Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[css-flexbox][baseline-alignment] Do not synthesize first baseline wh…
…en flex item block axis is parallel to flex cross axis.

https://bugs.webkit.org/show_bug.cgi?id=256937
rdar://109489182

Reviewed by Alan Baradlay.

This patches focuses on applying the described changes to flex items
that are block/grid/flex containers or tables.

If a flex item has a writing-mode that is different from the flex
container, that does not necessarily mean it cannot provide an ascent
value to use for baseline alignment within the flexbox. The
firstLineBaseline implementation within RenderBlockFlow/Grid/FlexibleBox/Table
each had a check for isWritingModeRoot() that would return std::nullopt
if the writing mode of the item was different from the flex container.
This would result in the flex container synthesizing a baseline for
the item which is not correct in all cases. This should only be done
if the block axis is *not* parallel to the flex container's cross axis.

In the following example we cannot get the baseline of the item so we
should synthesize it:
<div style="display: flex;">
  <div style="writing-mode: vertical-lr"> no valid baseline </div>
</div>

However, in the example below we can get the baseline so we should
*not* try to synthesize one:
<div style="display: flex; flex-direction: column;">
  <div style="writing-mode: vertical-lr"> Can get baseline </div>
</div>

We can resolve this issue by doing two things:
1.) Append an extra constraint against the isWritingModeRoot() check to
make sure this logic isn't applied on flex items
2.) Make RenderFlexibleBox check the block axis of the flex item with
its cross axis

We may also need to perform a translation on the ascent value that is
applied to the flex item since it may have been computed in a coordinate
space that is different from the flex container's (e.g. vertical-lr
flex container with vertical-rl item).

* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-column-vert-lr-flexbox-item-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-column-vert-lr-flexbox-item.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-column-vert-lr-grid-item-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-column-vert-lr-grid-item.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-column-vert-lr-items-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-column-vert-lr-items.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-column-vert-lr-table-item-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-column-vert-lr-table-item.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-column-vert-rl-flexbox-item-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-column-vert-rl-flexbox-item.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-column-vert-rl-grid-item-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-column-vert-rl-grid-item.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-column-vert-rl-items-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-column-vert-rl-items.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-column-vert-rl-table-item-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-column-vert-rl-table-item.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-vert-lr-column-horz-flexbox-item-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-vert-lr-column-horz-flexbox-item.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-vert-lr-column-horz-grid-item-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-vert-lr-column-horz-grid-item.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-vert-lr-column-horz-items-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-vert-lr-column-horz-items.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-vert-lr-column-horz-table-item-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-vert-lr-column-horz-table-item.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-vert-rl-column-horz-flexbox-item-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-vert-rl-column-horz-flexbox-item.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-vert-rl-column-horz-grid-item-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-vert-rl-column-horz-grid-item.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-vert-rl-column-horz-items-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-vert-rl-column-horz-items.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-vert-rl-column-horz-table-item-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/align-items-baseline-vert-rl-column-horz-table-item.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/alignment/flex-align-baseline-001-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/alignment/flex-align-baseline-002-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/alignment/flex-align-baseline-003-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/alignment/flex-align-baseline-004-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/alignment/flex-align-baseline-006-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/alignment/flex-align-baseline-007-expected.txt:
* Source/WebCore/rendering/RenderBlock.cpp:
(WebCore::RenderBlock::firstLineBaseline const):
* Source/WebCore/rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::firstLineBaseline const):
(WebCore::RenderBlockFlow::lastLineBaseline const):
* Source/WebCore/rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::firstLineBaseline const):
(WebCore::RenderFlexibleBox::marginBoxAscentForChild):
* Source/WebCore/rendering/RenderGrid.cpp:
(WebCore::RenderGrid::firstLineBaseline const):
* Source/WebCore/rendering/RenderTable.cpp:
(WebCore::RenderTable::firstLineBaseline const):

Canonical link: https://commits.webkit.org/264423@main
  • Loading branch information
sgill26 authored and Sammy Gill committed May 23, 2023
1 parent 2b33231 commit 2c62617
Show file tree
Hide file tree
Showing 43 changed files with 833 additions and 33 deletions.
@@ -0,0 +1,7 @@
two
lines
hello

PASS #target > div 1
PASS #target > div 2

@@ -0,0 +1,42 @@
<!DOCTYPE html>
<link rel="help" href="https://drafts.csswg.org/css-align-3/#baseline-rules">
<link rel="help" href="https://drafts.csswg.org/css-flexbox-1/#cross-alignment">
<link rel="author" href="mailto:sammy.gill@apple.com" title="Sammy Gill">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/check-layout-th.js"></script>
<style>
#target {
display: flex;
flex-direction: column;
width: 200px;
align-items: baseline;
border: 3px solid black;
font-family: monospace;
font-size: 10px;
line-height: 10px;
}
#target > :nth-child(1) {
background: hotpink;
writing-mode: vertical-lr;
padding-left: 30px;
margin-left: 10px;
}
#target > :nth-child(2) {
background: papayawhip;
writing-mode: vertical-lr;
}
.inner {
display: flex;
border: 5px solid black;
padding: 5px;
}
</style>
<body onload="checkLayout('#target > div')">
<div id="target">
<div class="inner" data-offset-x="21">
<div>two<br>lines</div>
</div>
<div data-offset-x="56">hello</div>
</div>
</body>
@@ -0,0 +1,7 @@
two
lines
hello

PASS #target > div 1
PASS #target > div 2

@@ -0,0 +1,43 @@
<!DOCTYPE html>
<link rel="help" href="https://drafts.csswg.org/css-align-3/#baseline-rules">
<link rel="help" href="https://drafts.csswg.org/css-flexbox-1/#cross-alignment">
<link rel="author" href="mailto:sammy.gill@apple.com" title="Sammy Gill">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/check-layout-th.js"></script>
<style>
#target {
display: flex;
flex-direction: column;
width: 200px;
align-items: baseline;
border: 3px solid black;
font-family: monospace;
font-size: 10px;
line-height: 10px;
}
#target > :nth-child(1) {
background: hotpink;
writing-mode: vertical-lr;
padding-left: 30px;
margin-left: 10px;
}
#target > :nth-child(2) {
background: papayawhip;
writing-mode: vertical-lr;
}
.inner {
display: grid;
grid-template: auto / auto;
border: 5px solid black;
padding: 5px;
}
</style>
<body onload="checkLayout('#target > div')">
<div id="target">
<div class="inner" data-offset-x="21">
<div>two<br>lines</div>
</div>
<div data-offset-x="56">hello</div>
</div>
</body>
@@ -0,0 +1,7 @@
two
lines
hello

PASS #target > div 1
PASS #target > div 2

@@ -0,0 +1,35 @@
<!DOCTYPE html>
<link rel="help" href="https://drafts.csswg.org/css-align-3/#baseline-rules">
<link rel="help" href="https://drafts.csswg.org/css-flexbox-1/#cross-alignment">
<link rel="author" href="mailto:sammy.gill@apple.com" title="Sammy Gill">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/check-layout-th.js"></script>
<style>
#target {
display: flex;
flex-direction: column;
width: 200px;
align-items: baseline;
border: 3px solid black;
font-family: monospace;
font-size: 10px;
line-height: 10px;
}
#target > :nth-child(1) {
background: hotpink;
writing-mode: vertical-lr;
padding-left: 30px;
margin-left: 10px;
}
#target > :nth-child(2) {
background: papayawhip;
writing-mode: vertical-lr;
}
</style>
<body onload="checkLayout('#target > div')">
<div id="target">
<div data-offset-x="21">two<br>lines</div>
<div data-offset-x="51">hello</div>
</div>
</body>
@@ -0,0 +1,7 @@
two
lines
hello

PASS #target > * 1
PASS #target > * 2

@@ -0,0 +1,45 @@
<!DOCTYPE html>
<link rel="help" href="https://drafts.csswg.org/css-align-3/#baseline-rules">
<link rel="help" href="https://drafts.csswg.org/css-flexbox-1/#cross-alignment">
<link rel="author" href="mailto:sammy.gill@apple.com" title="Sammy Gill">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/check-layout-th.js"></script>
<style>
#target {
display: flex;
flex-direction: column;
width: 200px;
align-items: baseline;
border: 3px solid black;
font-family: monospace;
font-size: 10px;
line-height: 10px;
}
#target > :nth-child(1) {
background: hotpink;
writing-mode: vertical-lr;
padding-left: 30px;
margin-left: 10px;
}
#target > :nth-child(2) {
background: papayawhip;
writing-mode: vertical-lr;
}
.inner {
border: 5px solid black;
padding: 5px;
}
</style>
<body onload="checkLayout('#target > *')">
<div id="target">
<table class="inner" data-offset-x="21">
<tr>
<td style="vertical-align: baseline;">
<div>two<br>lines</div>
</td>
</tr>
</table>
<div data-offset-x="59">hello</div>
</div>
</body>
@@ -0,0 +1,13 @@
two
lines
hello

FAIL #target > div 1 assert_equals:
<div class="inner" data-offset-x="146">
<div>two<br>lines</div>
</div>
offsetLeft expected 146 but got 21
FAIL #target > div 2 assert_equals:
<div data-offset-x="191">hello</div>
offsetLeft expected 191 but got 66

@@ -0,0 +1,44 @@
<!DOCTYPE html>
<link rel="help" href="https://drafts.csswg.org/css-align-3/#baseline-rules">
<link rel="help" href="https://drafts.csswg.org/css-flexbox-1/#cross-alignment">
<link rel="author" href="mailto:sammy.gill@apple.com" title="Sammy Gill">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/check-layout-th.js"></script>
<style>
#target {
font-size: 16px;
line-height: 16px;
display: flex;
flex-direction: column;
width: 200px;
align-items: baseline;
border: 3px solid black;
font-family: monospace;
font-size: 10px;
line-height: 10px;
}
#target > :nth-child(1) {
background: hotpink;
writing-mode: vertical-rl;
padding-left: 30px;
margin-left: 10px;
}
#target > :nth-child(2) {
background: papayawhip;
writing-mode: vertical-rl;
}
.inner {
display: flex;
border: 5px solid black;
padding: 5px;
}
</style>
<body onload="checkLayout('#target > div')">
<div id="target">
<div class="inner" data-offset-x="146">
<div>two<br>lines</div>
</div>
<div data-offset-x="191">hello</div>
</div>
</body>
@@ -0,0 +1,13 @@
two
lines
hello

FAIL #target > div 1 assert_equals:
<div class="inner" data-offset-x="146">
<div>two<br>lines</div>
</div>
offsetLeft expected 146 but got 21
FAIL #target > div 2 assert_equals:
<div data-offset-x="191">hello</div>
offsetLeft expected 191 but got 66

@@ -0,0 +1,43 @@
<!DOCTYPE html>
<link rel="help" href="https://drafts.csswg.org/css-align-3/#baseline-rules">
<link rel="help" href="https://drafts.csswg.org/css-flexbox-1/#cross-alignment">
<link rel="author" href="mailto:sammy.gill@apple.com" title="Sammy Gill">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/check-layout-th.js"></script>
<style>
#target {
display: flex;
flex-direction: column;
width: 200px;
align-items: baseline;
border: 3px solid black;
font-family: monospace;
font-size: 10px;
line-height: 10px;
}
#target > :nth-child(1) {
background: hotpink;
writing-mode: vertical-rl;
padding-left: 30px;
margin-left: 10px;
}
#target > :nth-child(2) {
background: papayawhip;
writing-mode: vertical-rl;
}
.inner {
display: grid;
grid-template: auto / auto;
border: 5px solid black;
padding: 5px;
}
</style>
<body onload="checkLayout('#target > div')">
<div id="target">
<div class="inner" data-offset-x="146">
<div>two<br>lines</div>
</div>
<div data-offset-x="191">hello</div>
</div>
</body>
@@ -0,0 +1,11 @@
two
lines
hello

FAIL #target > div 1 assert_equals:
<div data-offset-x="161">two<br>lines</div>
offsetLeft expected 161 but got 21
FAIL #target > div 2 assert_equals:
<div data-offset-x="201">hello</div>
offsetLeft expected 201 but got 61

@@ -0,0 +1,35 @@
<!DOCTYPE html>
<link rel="help" href="https://drafts.csswg.org/css-align-3/#baseline-rules">
<link rel="help" href="https://drafts.csswg.org/css-flexbox-1/#cross-alignment">
<link rel="author" href="mailto:sammy.gill@apple.com" title="Sammy Gill">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/check-layout-th.js"></script>
<style>
#target {
display: flex;
flex-direction: column;
width: 200px;
align-items: baseline;
border: 3px solid black;
font-family: monospace;
font-size: 10px;
line-height: 10px;
}
#target > :nth-child(1) {
background: hotpink;
writing-mode: vertical-rl;
padding-left: 30px;
margin-left: 10px;
}
#target > :nth-child(2) {
background: papayawhip;
writing-mode: vertical-rl;
}
</style>
<body onload="checkLayout('#target > div')">
<div id="target">
<div data-offset-x="161">two<br>lines</div>
<div data-offset-x="201">hello</div>
</div>
</body>
@@ -0,0 +1,17 @@
two
lines
hello

FAIL #target > * 1 assert_equals:
<table class="inner" data-offset-x="140">
<tbody><tr>
<td style="vertical-align: baseline;">
<div>two<br>lines</div>
</td>
</tr>
</tbody></table>
offsetLeft expected 140 but got 21
FAIL #target > * 2 assert_equals:
<div data-offset-x="188">hello</div>
offsetLeft expected 188 but got 69

0 comments on commit 2c62617

Please sign in to comment.