Closed Bug 1238294 Opened 5 years ago Closed 5 years ago

[css-grid] Tweak abs.pos. area that ends in line 1, or start in the last line

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: mats, Assigned: mats)

Details

Attachments

(5 files)

Attached file Testcase
STR
1. load the attached testcase in a Nightly build

ACTUAL RESULTS
The black areas are grid-aligned abs.pos. children that spans from the padding
edge to a definite line.  In the grid at the top, the areas have auto/1, auto/2
etc. and in the grid below they have 1/auto, 2/auto etc.

As you can see, the first abs.pos. in the top grid and the last in the bottom
grid appear a bit asymmetric to the other ones.

I tend to think we should change this so that they appear symmetrical.
Attached image Expected result
Here's the desired rendering: each abs.pos. is now one gap longer than
the one before it ("gap" here meaning the width of a grey area, i.e.
the distance between two tracks).  Notably, both the aforementioned
problematic abs.pos. areas now just covers the padding area (which
also implies they will have zero width if the padding is removed).
Attachment #8706034 - Attachment filename: grid-before-line-zero.png → Expected result
Attachment #8706034 - Attachment description: grid-before-line-zero.png → Expected result
Attachment #8706034 - Attachment filename: Expected result → grid-before-line-zero.png
So here's the actual fix.  It makes "eBeforeGridGap" of the first line be
at the content area start edge, and "eAfterGridGap" of the last line be at
the content area end edge.
Attachment #8706045 - Flags: review?(dholbert)
I agree that your expected-behavior here makes sense, and I think it matches the current spec text. In particular:
 * The gutters spec-text says "the grid track between two grid lines is the *space between* the gutters that represent them" https://drafts.csswg.org/css-grid/#gutters
 * For abspos, "an auto value ... contributes a special line ... whose position is that of the corresponding padding edge of the grid container". https://drafts.csswg.org/css-grid/#abspos-items

So IIUC, this means an abspos item with auto/1 should span the space *between* the special padding-edge-line & the gutter for line 1. In particular, it should *not* overlap the gutter for line 1, as it does in current mozilla-central.

Point being:
 - I agree this is a bug.
 - I don't think this needs a spec change.
Comment on attachment 8706044 [details] [diff] [review]
part 1 - [css-grid] Make GridLineEdge() a method on the Tracks class rather than a static function (idempotent change).

Review of attachment 8706044 [details] [diff] [review]:
-----------------------------------------------------------------

r=me on part 1
Attachment #8706044 - Flags: review?(dholbert) → review+
Comment on attachment 8706045 [details] [diff] [review]
part 2 - [css-grid] Treat any gaps at the grid edges as "line thickness" so they behave the same as gaps between tracks for positioning areas.

Review of attachment 8706045 [details] [diff] [review]:
-----------------------------------------------------------------

r=me on part 2
Attachment #8706045 - Flags: review?(dholbert) → review+
Comment on attachment 8706046 [details] [diff] [review]
part 3 - [css-grid] Add/tweak reftests for new behavior of gaps at the grid edges.

Review of attachment 8706046 [details] [diff] [review]:
-----------------------------------------------------------------

Two notes on the reftests:

(1) If you don't already have something like this, consider adding a testcase for the "interesting" (IMO) scenario where the last grid-line is effectively one giant gutter, due to having "justify-content: start" or "align-content:start" which smooshes all the tracks to the start edge.

e.g. a grid like the following diagram, with "justify-content:start":

line idx:  1      2      3-is-a-big-gutter-3 auto
        +-------------------------------------+
        |  (padding)                          |
        |  +-------------------------------+  |
        |  |      |      |                 |  |
        |  |track1|track2|                 |  |
        |  |      |      |                 |  |
        |  |      |      | (packing space, |  |
        |  |      |      |  i.e. gutter)   |  |
        |  |      |      |                 |  |
        |  +-------------------------------+  |
        |                                     |
        +-------------------------------------+
...with an abspos element (not shown) that has grid-column:3/auto. In current trunk, IIUC, this abspos element would span all the way back to track2, but now it'll just span the right padding-space.

(Maybe your existing tests provide enough coverage for this scenario; if so, great.)

::: layout/reftests/css-grid/grid-abspos-items-013.html
@@ +31,5 @@
>    float: left;
>    min-width: 70px;
>    margin-right: 2px;
>  }
> +.c.s { 

(2) Nit: drop end-of-line space here.
(In reply to Daniel Holbert [:dholbert] from comment #8)
> (1) If you don't already have something like this,

grid-abspos-items-013.html covers this case

> (2) Nit: drop end-of-line space here.

Fixed, thanks.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.