1609403 - grid items with align-self:baseline should synthesize a baseline instead of falling back to start/end alignment

Status: RESOLVED FIXED

(Core :: Layout: Grid, defect, P3)

Description

[rmonteriso]

Attachment: grid-align-self-baseline.html

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:72.0) Gecko/20100101 Firefox/72.0

Steps to reproduce:

Open Firefox.
Open firefox-grid-align-self-baseline.html (see attached file) in the browser.
Observe rectangle's position.

For a useful comparison:
open Chrome and Safari.
Open firefox-grid-align-self-baseline.html in the browsers.
Observe rectangle's position.

Actual results:

Firefox:
Rectangles in div 1, 2 and 3 with property align-self: baseline do not share a common baseline alignment.

Chrome, Safari:
Rectangles in div 1 and 3 with property align-self: baseline participate in a common baseline alignment and are aligned to the dominant baseline (green rectangle, div 2).

Expected results:

Rectangles in div 1 and 3 with property align-self: baseline should participate in a common baseline alignment and should be aligned to the dominant baseline (green rectangle, div 2) in Firefox as well.

Comment 1

[rmonteriso]

Attachment: Firefox-baseline-screenshot.png

Comment 2

[rmonteriso]

Attachment: Safari-baseline-screenshot.png

Comment 3

[rmonteriso]

Attachment: Chrome-baseline-screenshot.png

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Hmm, stuff aligns to the baseline if they're non-empty... What's the baseline of an empty block?

Mats, do you have any thoughts on this?

Comment 5

[Javier Fernandez]

As far as I understand the spec, an empty box should synthesize [1] its baseline and this process may depend on the nature of the element and its formatting context. Specifically, for grid items, it should use their 'border edges'. I consider that empty boxes should follow what it's specified for Atomic inline baselines[2] , so that's why empty grid items should use the 'under' border edge. Finally, it's worth mentioning that there was a resolution [3] of the CSS WG regarding this issue.

[1] https://drafts.csswg.org/css-align/#synthesize-baseline
[2] https://drafts.csswg.org/css-writing-modes-4/#replaced-baselines
[3] https://github.com/w3c/csswg-drafts/issues/439#issuecomment-246571933

Comment 6

[Mats Palmgren (inactive)]

The grid items in the example are empty block containers. The css-align spec says:
"If there is no such line box or child, then the block container has no baseline set."
https://drafts.csswg.org/css-align/#baseline-export

So it's clear that these grid items have no baseline set. So should we synthesize a baseline and participate in the baseline alignment, or should they not participate in baseline alignment and instead use their fallback alignment?

Looking at the code, it appears we do the latter. Presumably because the css-grid/css-align specs said so at one point. But yeah, Grid baseline alignment has been pretty heavily rewritten to match Chrome's implementation since that point (all the stuff about "baseline shims" in CSS Grid was added after we implemented the original spec for example). So our implementation might be out-of-date with these changes.

What's the baseline of an empty block?

The css-align spec is clear that if we were to synthesize a baseline for an empty block container grid item, then the border-box should be used. But that's not the relevant question. The question is whether the item should participate in the first place. If the item doesn't participate in baseline alignment then its synthesized baseline is irrelevant.

Is there any spec text that says that grid items without a baseline set should participate in baseline alignment using a synthesized baseline?

Interestingly, the section on Grid container baselines, says:
"If the item has no alignment baseline..."
https://drafts.csswg.org/css-grid/#grid-baselines
Which states that some items don't have an alignment baseline, which seems to contradict any assumption that grid items always synthesize a baseline when they don't have a natural baseline set.

Comment 7

[Javier Fernandez]

The Alignment spec states the following regarding the participation of a grid item in Baseline Alignment:
"A grid item participates in first/last baseline self-alignment in its startmost/endmost row or column if its align-self or justify-self property (respectively) computes to first baseline/last baseline. "

So, it seems that the value of align-self/justify-self is enough to determine the participation. Then, I guess we can conclude that if the box has n baseline, it should synthesize it.

The sentence that you mention from the Grid Spec actually implies that the baseline must be synthesized if it has no baseline:
"If the item has no alignment baseline in the grid’s inline axis, then one is first synthesized from its border edges"

And the CSS WG resolution regarding how to synthesize baselines of grid items that are empty boxes is form 2016, so I think it's been there for quite long already.

Comment 8

[Mats Palmgren (inactive)]

So, it seems that the value of align-self/justify-self is enough to determine the participation. Then, I guess we can conclude that if the box has n baseline, it should synthesize it.

That's a pretty vague assumption though... I'd prefer if the spec would be clearer on this if that's the intention.

The sentence that you mention from the Grid Spec actually implies that the baseline must be synthesized if it has no baseline:
"If the item has no alignment baseline in the grid’s inline axis, then one is first synthesized from its border edges"

In this case, you are misreading the spec. The above text most certainly does not imply that items always synthesize a baseline. In fact, it implies the opposite since if they always synthesize a baseline then there would be no need to point it out explicitly here. (Note that this section is about Grid container baselines, so it does not attempt to define how item baselines work.)

And the CSS WG resolution regarding how to synthesize baselines of grid items that are empty boxes is form 2016, so I think it's been there for quite long already.

Again, I agree with you how to synthesize baselines for grid items. But that's not the issue here.

Comment 9

[Javier Fernandez]

That's a pretty vague assumption though... I'd prefer if the spec would be clearer on this if that's the intention.

I'm fine with asking the spec editors to clarify, explicitly, when an items participates in Baseline Alignment and maybe it should also mention explicitly when it doesn't , like it happens when there is a cyclic sizing dependency.

I think that what I'm trying to say is, where in the spec is stated that an item with no baseline doesn't participate in Baseline Alignment ? I think there are several places where it's assumed that it does, indeed. I agree, as I said above, that we should avoid the need of assumptions here.

Anyway, maybe we should move the discussion to the CSS WG then.

Comment 10

[Javier Fernandez]

Here there is another statement to be considered for this issue:

https://drafts.csswg.org/css-align/#baseline-terms

"A baseline-sharing group is composed of boxes that participate in baseline alignment together. This is possible only if they both:

• Share an alignment context along an axis perpendicular to the axis they’re being baseline-aligned in. (For example, grid items with align-self: baseline are baseline-aligning along the grid’s block axis, and therefore participate with other items in their row.)
• Have compatible baseline alignment preferences (i.e., the baselines that want to align are on the same side of the alignment context). "

This stress on the fact that a box that has 'baseline" as align-self/justify-self value does participates in Baseline Alignment.

Comment 11

[Mats Palmgren (inactive)]

Well, that whole quote hinges on "boxes that participate in baseline alignment". Thus, if the box doesn't participate in baseline alignment in the first place, that whole quote doesn't apply to it. What the above quote is trying to define is which baseline-participating boxes goes into a baseline-sharing group by adding further conditions that defines which ones are "together" and thus are in the same group.

Anyway, it's a pretty simple question for the spec editors to answer so I filed an issue here:
https://github.com/w3c/csswg-drafts/issues/4675

Comment 12

[Jonathan Kew [:jfkthame]]

Marking this P4 for now, pending spec clarification; bump to P3 if/when we have confirmation of what should happen here.

Comment 13

[Javier Fernandez]

I think Chrome doesn't pass the following tests for this same reason:

css/css-align/baseline-rules/grid-item-input-type-number.html
css/css-align/baseline-rules/grid-item-input-type-text.html

The issue in this case is that a vertical-lr grid container has a single grid item, which is an 'input' element, hence orthogonal to the grid container. Since an orthogonal grid item has no natural baseline, I think the mentioned tests assume that the grid has no natural baseline either. However, from the Grid Layout spec, I understand that the grid should generate its baseline from the item's synthesized baseline.

https://drafts.csswg.org/css-grid/#grid-baselines

2. Otherwise, if the grid container has at least one grid item, the grid container’s first (last) baseline set is generated from the alignment baseline of the first (last) grid item in row-major grid order (according to the writing mode of the grid container). If the item has no alignment baseline in the grid’s inline axis, then one is first synthesized from its border edges.

I think this interoperability issue with Chrome has this bug as common root cause, but I'll file a new bug if anybody thinks is more appropriated.

If we agree is the same root cause, I guess we need to wait for the CSS WG to resolve the corresponding issue:

https://github.com/w3c/csswg-drafts/issues/4675

Comment 14

[Javier Fernandez]

The CSS WG has resolved the issue, so that any box can participate in baseline alignment, synthesizing its baselines if it has no natural one.

So I guess this bug can be marked as conformed.

Comment 16

[Daniel Holbert [:dholbert]]

Indeed. Also: looks like https://wpt.live/css/css-grid/alignment/grid-container-baseline-001.html fails due to this issue (per bug 1623963 which I've just duped here).

Comment 17

[Emilio Cobos Álvarez (:emilio)]

Attachment: WIP: Bug 1609403 - WIP: Synthesize a baseline if there's none.

This makes the test-case look almost correct.

Comment 18

[Emilio Cobos Álvarez (:emilio)]

So I took a look at why comment 17 still doesn't get the desired output, and I found out why, which is that the MeasuringReflow call we use during baseline alignment (https://searchfox.org/mozilla-central/rev/4a10f85ebca1c22c803d0253a82fe39514ba0b53/layout/generic/nsGridContainerFrame.cpp#5805) uses the UseAutoBSize flag (https://searchfox.org/mozilla-central/rev/4a10f85ebca1c22c803d0253a82fe39514ba0b53/layout/generic/nsGridContainerFrame.cpp#5006) which causes us to ignore even explicit (non-percentage, non-keyword) block sizes.

Daniel, when would we want grid items to actually ignore even explicit pixel-sizes? That seems weird to me...

Comment 19

[Daniel Holbert [:dholbert]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #18)

Daniel, when would we want grid items to actually ignore even explicit pixel-sizes? That seems weird to me...

I think we need to measure the content size of grid items in order to properly perform align-content:baseline alignment, which is a somewhat odd action-at-a-distance feature that moves the content within an item. See https://drafts.csswg.org/css-align-3/#baseline-align-content -- we have that implemented in grid (for grid items) but nowhere else.

I don't know why we would need to ignore pixel-sizes for align-self:baseline-associated measuring-reflows, though. It might be that we're just sharing code amongst our various baseline alignment functionality and improperly applying the same flags when we really shouldn't.

Comment 20

[Emilio Cobos Álvarez (:emilio)]

Attachment: WIP: Bug 1609403 - Further changes that make the test-case work.

So, these changes make the test-case work as expected (on top of the
previous patch) by making the explicit size have an effect. It causes a
bunch progressions, but also a couple regressions:

https://treeherder.mozilla.org/jobs?repo=try&revision=2e827309e3306792998cceb710eb7e73b3384c30

It seems that intrinsically sized items that on intrinsically sized
tracks shouldn't really participate in baseline alignment, per
https://github.com/w3c/csswg-drafts/issues/1365 so, tl;dr this probably
needs more work, but uploading just for reference...

It seems we should try to look into how other browsers deal with these
to see if we can avoid extra layout passes. This seems like the relevant
chromium code for the record:

https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/layout/ng/grid/ng_grid_layout_algorithm.cc;l=1350-1372;drc=312b74e385e6aba98ab31fd911238c0dc16b396c

Comment 21

[Tiaan Louw]

Attachment: Bug 1609403 - Grid items with align/justify-self should participate in baseline alginment r=emilio,#layout-reviewers

Comment 22

[Tiaan Louw]

I have the initial fix in place and updated the tests that is now succeeding. I will be investigating further about some items not participating in baseline alignment next per comment 20

Comment 23

[Daniel Holbert [:dholbert]]

Note, there's at least one interop-2023-grid failure that's due to this bug:
http://wpt.fyi/css/css-grid/alignment/grid-align-baseline-003.html

I think we'll also soon have several flexbox test failures due to this bug -- we render the test correctly (as of bug 1818933) but we misrender the grid-based reference case per this bug here, so the test ends up failing:
http://wpt.fyi/css/css-flexbox/align-items-baseline-row-horz.html
http://wpt.fyi/css/css-flexbox/align-items-baseline-row-vert.html
http://wpt.fyi/css/css-flexbox/align-items-baseline-column-horz.html
http://wpt.fyi/css/css-flexbox/align-items-baseline-column-vert.html

Comment 25

[Daniel Holbert [:dholbert]]

These interop-2023 WPTs also seem to be this same bug (i.e. we've got a grid item with align-self:baseline either directly or via default-applying the align-items from the grid, and we're falling back to start alignment instead of synthesizing a baseline from the border-box):
https://wpt.fyi/results/css/css-grid/alignment/grid-align-baseline.html
https://wpt.fyi/results/css/css-grid/alignment/grid-align-baseline-vertical.html
https://wpt.fyi/results/css/css-grid/alignment/grid-alignment-style-changes-001.html
https://wpt.fyi/results/css/css-grid/alignment/grid-alignment-style-changes-002.html
https://wpt.fyi/results/css/css-grid/alignment/grid-alignment-style-changes-003.html (see also bug 1500630 though it may no longer be relevant)
https://wpt.fyi/results/css/css-grid/alignment/grid-alignment-style-changes-004.html

Comment 28

[Daniel Holbert [:dholbert]]

This one is this bug as well -- the first grid item here is empty and we're failing to synthesize a baseline for it:
http://wpt.fyi/results/css/css-grid/alignment/self-baseline/grid-self-baseline-changes-grid-area-size-009.html

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5dfc5b6d7ba4
Grid items with align/justify-self should participate in baseline alginment r=emilio,layout-reviewers

Comment 30

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/43495 for changes under testing/web-platform/tests

Comment 31

[Noemi Erli[:noemi_erli]]

Backed out changeset 5dfc5b6d7ba4 (Bug 1609403) for causing failures in grid-self-baseline-changes-grid-area-size-002.html CLOSED TREE

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&selectedTaskRun=V-ZbpG3zSAiNA7tuaH_axQ.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=5dfc5b6d7ba49604062e017cc2cd77b214ea2d63

Log: https://treeherder.mozilla.org/logviewer?job_id=438713852&repo=autoland&lineNumber=14306

Backout: https://hg.mozilla.org/integration/autoland/rev/4009f34666067d6e1c26ce4acf25ced573d8c09d

Comment 32

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 33

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/02d58cff5d1a
Grid items with align/justify-self should participate in baseline alginment r=emilio,layout-reviewers

Comment 34

[Natalia Csoregi [:nataliaCs]]

Backed out for causing failures on grid-self-baseline-changes-grid-area-size-002.html

• backout: https://hg.mozilla.org/integration/autoland/rev/89b2b530951fd31a9a8d634496f919cd65cc921e
• push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&selectedTaskRun=IqonKJvhRMKRypE9JE4Ygw.0&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception%2Crunnable&revision=02d58cff5d1ab112ebbe2ad15cb58f9263684fb9
• failure log: https://treeherder.mozilla.org/logviewer?job_id=438787058&repo=autoland&lineNumber=24835

[task 2023-12-04T20:35:19.574Z] 20:35:19     INFO - TEST-START | /css/css-grid/alignment/self-baseline/grid-self-baseline-changes-grid-area-size-002.html
[task 2023-12-04T20:35:19.575Z] 20:35:19     INFO - PID 1972 | 1701722119574	Marionette	INFO	Testing http://web-platform.test:8000/css/css-grid/alignment/self-baseline/grid-self-baseline-changes-grid-area-size-002.html == http://web-platform.test:8000/css/reference/ref-filled-green-100px-square.xht
[task 2023-12-04T20:35:19.590Z] 20:35:19     INFO - PID 1972 | [Child 2014, Main Thread] WARNING: JSWindowActorChild::SendRawMessage (MarionetteReftest, MarionetteReftestParent:reftestWait) not sent: !CanSend() || !mManager || !mManager->CanSend(): file /builds/worker/checkouts/gecko/dom/ipc/jsactor/JSWindowActorChild.cpp:61
[task 2023-12-04T20:35:19.640Z] 20:35:19     INFO - PID 1972 | 1701722119639	Marionette	INFO	No differences allowed
[task 2023-12-04T20:35:19.708Z] 20:35:19     INFO - TEST-UNEXPECTED-FAIL | /css/css-grid/alignment/self-baseline/grid-self-baseline-changes-grid-area-size-002.html | Testing http://web-platform.test:8000/css/css-grid/alignment/self-baseline/grid-self-baseline-changes-grid-area-size-002.html == http://web-platform.test:8000/css/reference/ref-filled-green-100px-square.xht
[task 2023-12-04T20:35:19.708Z] 20:35:19     INFO - Found 60 pixels different, maximum difference per channel 255

Comment 35

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 36

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3d0a5f6fd8b4
Grid items with align/justify-self should participate in baseline alginment r=emilio,layout-reviewers

Comment 37

[Natalia Csoregi [:nataliaCs]]

Backed out for causing failures on grid-self-baseline-changes-grid-area-size-002.html

• backout: https://hg.mozilla.org/integration/autoland/rev/9d8181714fc20045489715a806dc1d42e60f9128
• push: https://treeherder.mozilla.org/jobs?repo=autoland&revision=3d0a5f6fd8b4a0537cb85ae502a4be6cafec1c23&group_state=expanded&selectedTaskRun=PplcCJzRSQ6qyPzOjpg0Tg.0
• failure log: https://treeherder.mozilla.org/logviewer?job_id=438819096&repo=autoland&lineNumber=10385

[task 2023-12-04T23:31:04.758Z] 23:31:04     INFO - TEST-START | /css/css-grid/alignment/self-baseline/grid-self-baseline-changes-grid-area-size-002.html
[task 2023-12-04T23:31:04.760Z] 23:31:04     INFO - PID 16259 | 1701732664759	Marionette	INFO	Testing http://web-platform.test:8000/css/css-grid/alignment/self-baseline/grid-self-baseline-changes-grid-area-size-002.html == http://web-platform.test:8000/css/reference/ref-filled-green-100px-square.xht
[task 2023-12-04T23:31:04.835Z] 23:31:04     INFO - PID 16259 | 1701732664834	Marionette	INFO	No differences allowed
[task 2023-12-04T23:31:04.920Z] 23:31:04     INFO - TEST-UNEXPECTED-PASS | /css/css-grid/alignment/self-baseline/grid-self-baseline-changes-grid-area-size-002.html | Testing http://web-platform.test:8000/css/css-grid/alignment/self-baseline/grid-self-baseline-changes-grid-area-size-002.html == http://web-platform.test:8000/css/reference/ref-filled-green-100px-square.xht

Comment 38

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 39

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/99e226ce1286
Grid items with align/justify-self should participate in baseline alginment r=emilio,layout-reviewers

Comment 40

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/99e226ce1286

Comment 41

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot