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.

Sign up for GitHub

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

Jump to bottom

[css-align-3][css-grid-1] Alignment edge preference of baseline sharing groups. #7645

Closed
bfgeek opened this issue Aug 24, 2022 · 3 comments
Closed

[css-align-3][css-grid-1] Alignment edge preference of baseline sharing groups. #7645

bfgeek opened this issue Aug 24, 2022 · 3 comments
Labels
Closed Accepted as Editorial css-align-3 Current Work css-grid-1

Comments

@bfgeek
Copy link

bfgeek commented Aug 24, 2022

Consider the following example:
https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=10618

<!DOCTYPE html>
<div style="display: grid; grid-template: 100px 100px / 100px; justify-items: baseline;">
  <div style="writing-mode: vertical-rl; background: lime;">
    line1<br>line2
  </div>
  <div style="writing-mode: vertical-lr; background: hotpink;">
    line1<br>line2
  </div>
</div>

Here we have two separate baseline sharing groups.
css-align-3 states that if things are first-baseline aligned their default alignment should be start aligned.
Blink/WebKit's behaviour here is likely "correct" based on this. Then consider:
https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=10619

<!DOCTYPE html>
<div style="display: grid; grid-template: 100px 100px 100px / 100px; justify-items: baseline;">
  <div style="writing-mode: vertical-rl;  background: lime;">
    line1<br>line2
  </div>
  <div style="writing-mode: vertical-lr;  background: hotpink;">
    line1<br>line2
  </div>
  <div style="writing-mode: vertical-lr; justify-self: last baseline; background: cyan;">
    line1<br>line2
  </div>
</div>

Here we can't both be start & end aligned?

Two potential solutions to this problem.

  1. Make everything start aligned by default (including last-baseline).

  2. In Blink we have the concept of a "major" baseline sharing group (the one that would be start aligned), and a "minor" (the one that is end aligned). Particular care needs to be done when defining which writing-mode+directions go into the "major"/"minor" group.

If we go with (2) the likely affected WPTs are:
external/wpt/css/css-grid/alignment/grid-alignment-style-changes-003.html
external/wpt/css/css-grid/alignment/grid-alignment-style-changes-004.html
external/wpt/css/css-grid/alignment/grid-row-axis-self-baseline-synthesized-001.html
external/wpt/css/css-grid/alignment/grid-row-axis-self-baseline-synthesized-002.html
external/wpt/css/css-grid/alignment/grid-row-axis-self-baseline-synthesized-003.html
external/wpt/css/css-grid/alignment/self-baseline/grid-self-baseline-002-b.html
external/wpt/css/css-grid/alignment/self-baseline/grid-self-baseline-002.html
external/wpt/css/css-grid/alignment/self-baseline/grid-self-baseline-horiz-002.html
external/wpt/css/css-grid/alignment/self-baseline/grid-self-baseline-horiz-003.html
external/wpt/css/css-grid/alignment/self-baseline/grid-self-baseline-horiz-005.html
external/wpt/css/css-grid/alignment/self-baseline/grid-self-baseline-horiz-006.html
external/wpt/css/css-grid/alignment/self-baseline/grid-self-baseline-horiz-007.html
external/wpt/css/css-grid/alignment/self-baseline/grid-self-baseline-vertical-lr-007.html
external/wpt/css/css-grid/alignment/self-baseline/grid-self-baseline-vertical-rl-007.html
external/wpt/css/css-grid/layout-algorithm/baseline-alignment-affects-intrinsic-size-001.html
external/wpt/css/css-grid/layout-algorithm/baseline-alignment-affects-intrinsic-size-002.html
external/wpt/css/css-grid/layout-algorithm/grid-minimum-contribution-baseline-shim-vertical-rl.html
@bfgeek
Copy link
Author

bfgeek commented Aug 24, 2022

cc/ @tabatkins @fantasai

tabatkins added a commit that referenced this issue Aug 29, 2022
@tabatkins
[css-align-3] Clarify how baseline-sharing groups determine their fal…
d896c01 
…lback alignment. #7645
@fantasai
Copy link
Collaborator

@bfgeek Clarified in d896c01 ; within a baseline-sharing group, once you resolve to physical alignment directions, the fallback preferences should be the same for all items.

We believe this resolves the issue, let us know if we missed anything.

Merged
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 30, 2022
@bfgeek @chromium-wpt-export-bot
[grid] Fix alignment of baseline sharing groups.
31ba14e 
Previously we'd make baseline sharing groups both start aligned. This
was incorrect (however correct via. the spec if you read it a certain
way).

Instead the "major" group should be start aligned, and the "minor"
group should be end aligned. See:
w3c/csswg-drafts#7645

Change-Id: Iaa04f89e84116acfb9242a1c9cad0e9219f210f1
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 30, 2022
@bfgeek @chromium-wpt-export-bot
[grid] Fix alignment of baseline sharing groups.
ff0a51e 
Previously we'd make baseline sharing groups both start aligned. This
was incorrect (however correct via. the spec if you read it a certain
way).

Instead the "major" group should be start aligned, and the "minor"
group should be end aligned. See:
w3c/csswg-drafts#7645

Change-Id: Iaa04f89e84116acfb9242a1c9cad0e9219f210f1
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 31, 2022
@bfgeek @chromium-wpt-export-bot
[grid] Fix alignment of baseline sharing groups.
0a8d454 
Previously we'd make baseline sharing groups both start aligned. This
was incorrect (however correct via. the spec if you read it a certain
way).

Instead the "major" group should be start aligned, and the "minor"
group should be end aligned. See:
w3c/csswg-drafts#7645

Change-Id: Iaa04f89e84116acfb9242a1c9cad0e9219f210f1
aarongable pushed a commit to chromium/chromium that referenced this issue Aug 31, 2022
@bfgeek
[grid] Fix alignment of baseline sharing groups.
796675a 
Previously we'd make baseline sharing groups both start aligned. This
was incorrect (however correct via. the spec if you read it a certain
way).

Instead the "major" group should be start aligned, and the "minor"
group should be end aligned. See:
w3c/csswg-drafts#7645

Change-Id: Iaa04f89e84116acfb9242a1c9cad0e9219f210f1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3858306
Reviewed-by: Kurt Catti-Schmidt <kschmi@microsoft.com>
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1041724}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 31, 2022
@bfgeek @chromium-wpt-export-bot
[grid] Fix alignment of baseline sharing groups.
72d7bd0 
Previously we'd make baseline sharing groups both start aligned. This
was incorrect (however correct via. the spec if you read it a certain
way).

Instead the "major" group should be start aligned, and the "minor"
group should be end aligned. See:
w3c/csswg-drafts#7645

Change-Id: Iaa04f89e84116acfb9242a1c9cad0e9219f210f1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3858306
Reviewed-by: Kurt Catti-Schmidt <kschmi@microsoft.com>
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1041724}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 31, 2022
@bfgeek @chromium-wpt-export-bot
[grid] Fix alignment of baseline sharing groups.
4705a80 
Previously we'd make baseline sharing groups both start aligned. This
was incorrect (however correct via. the spec if you read it a certain
way).

Instead the "major" group should be start aligned, and the "minor"
group should be end aligned. See:
w3c/csswg-drafts#7645

Change-Id: Iaa04f89e84116acfb9242a1c9cad0e9219f210f1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3858306
Reviewed-by: Kurt Catti-Schmidt <kschmi@microsoft.com>
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1041724}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Sep 5, 2022
@bfgeek @moz-wptsync-bot
Bug 1788736 [wpt PR 35715] - [grid] Fix alignment of baseline sharing…
56a533f 
… groups., a=testonly

Automatic update from web-platform-tests
[grid] Fix alignment of baseline sharing groups.

Previously we'd make baseline sharing groups both start aligned. This
was incorrect (however correct via. the spec if you read it a certain
way).

Instead the "major" group should be start aligned, and the "minor"
group should be end aligned. See:
w3c/csswg-drafts#7645

Change-Id: Iaa04f89e84116acfb9242a1c9cad0e9219f210f1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3858306
Reviewed-by: Kurt Catti-Schmidt <kschmi@microsoft.com>
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1041724}

--

wpt-commits: 4705a80ce28f07353ff682106f6548e1003e2032
wpt-pr: 35715
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
@bfgeek
[grid] Fix alignment of baseline sharing groups.
313c5b4 
Previously we'd make baseline sharing groups both start aligned. This
was incorrect (however correct via. the spec if you read it a certain
way).

Instead the "major" group should be start aligned, and the "minor"
group should be end aligned. See:
w3c/csswg-drafts#7645

Change-Id: Iaa04f89e84116acfb9242a1c9cad0e9219f210f1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3858306
Reviewed-by: Kurt Catti-Schmidt <kschmi@microsoft.com>
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1041724}
NOKEYCHECK=True
GitOrigin-RevId: 796675a5c26de3ef838c0e5104ef4f928ebb744c
@tabatkins
Copy link
Member

Ian said r+ offline

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Closed Accepted as Editorial css-align-3 Current Work css-grid-1
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@tabatkins @fantasai @bfgeek