1531982 - Grid API returns incorrect names for reversed implicit areas

Status: RESOLVED FIXED

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

Description

[Micah [:mtigley] (she/her)]

Test case:
data:text/html,<div style="display: grid; grid-template-columns: [a-end] 100px [b]"><div style="grid-column: a-start / a-end"></div><div>

In Bug 1526975, the API will allow returning all implicit grid lines, including the ones created from implicitly named grid areas. There's an interesting test case where creating an implicit grid area with only a gridline named "<something>-end" causes the area's start line to be further endward than it's end line, so the grid swaps these lines to handle it. See: https://www.w3.org/TR/css-grid-1/#grid-placement-errors

As a result, the example above returns the named gridlines as: [a-start, a-end] [b] [a-end], when instead should be: [a-end] [b] [a-start]

So I think when we do the swap at https://searchfox.org/mozilla-central/rev/00f3836a87b844b5e4bc82f698c559b9966e4be2/layout/generic/nsGridContainerFrame.cpp#2762 , we should also have something flagged that allows us to swap the line names back when it's filtered through the API.

Comment 1

[Brad Werth [:bradwerth]]

I changed the component because I think the place to make the change is in the grid API code, which is technically part of the Platform. I believe the place to fix this is in GridLines::SetLineInfo(), where we can do the right thing without adding more information from the layout process. Specifically, we can do this:

1. In the initial pass through the grid areas, ignore the implicit areas entirely.
2. Add a new post-processing loop over the implicit areas. For each implicit area:
   a) For the line at the start of area "a":
   i) Check if the line already has the name "a-start". If it does, add the name "a-end" to the line at the OTHER end of the area.
   ii) Do the same check for the name "a-end". If the start line has "a-end", add the name "a-start" to the line at the other end.
   b) Do the same check above for the line at the end of area "a". Do this EVEN IF names were added by the earlier case.
   c) If no names were added by any of the above cases, add "a-start" to the start line and "a-end" to the end line.

I think that should work. As an additional test case, see if data:text/html,<div style="display: grid; grid-template-columns: [a-end] 100px [b]"><div style="grid-area: a"></div><div style="grid-column: a-start / 2 a-end"></div><div> gives us four lines with names [a-end] [b] [a-start] [a-end].

Comment 2

[Micah [:mtigley] (she/her)]

I'll assign myself to this since it will be be part of the work in Bug 1526975

Comment 3

[Micah [:mtigley] (she/her)]

Attachment: Bug 1531982 Part 2: Update test expectations to check that implicit line names from implicit named areas are assigned to lines.

Comment 4

[Micah [:mtigley] (she/her)]

Attachment: Bug 1531982 Part 1: Assign correct line names to reversed implicit areas

Comment 5

[Micah [:mtigley] (she/her)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=06a0cf8ef52c36f0ba575a3576907497efe7a528

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/89590e4e8dac
Part 1: Assign correct line names to reversed implicit areas r=bradwerth
https://hg.mozilla.org/integration/autoland/rev/f6e53afcd5eb
Part 2: Update test expectations to check that implicit line names from implicit named areas are assigned to lines. r=bradwerth

Comment 7

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/89590e4e8dac
https://hg.mozilla.org/mozilla-central/rev/f6e53afcd5eb