946835 - Add a few more reftests for multi-line flexbox

Status: RESOLVED FIXED

(Core :: Layout, defect, P4)

Description

[Daniel Holbert [:dholbert]]

Per bug 939901 comment 32 through 34,, I want to add a few more reftests for multi-line flexbox.

Quoting 939901 comment 32:
> The things I'd still like to write tests are:
>  - Baseline alignment of items on multiple lines, simultaneously, in a
> multi-line flex container (each line calculating its own baseline,
> independently)
> 
>  - Computing the baseline of a vertical flex container (single and
> multi-line) (We already have tests for computing the baseline of a
> horizontal single-line flex container, and [...]
> a horizontal multi-line flex container).

...and per bug 939901 comment 34, I'm also splitting out:
 - Computing the baseline of a horizontal flex container *using the negotiated baseline-alignment position of any baseline-aligned items its first line*

...and it'd also probably be worth adding a very basic reftest for all the possible values of the "flex-flow" shorthand, to the w3c test submissions directory. (We already kind of test flex-flow via mochitests & property_database.js, but that's not submitted upstream to the w3c testsuite, and I also don't think we explicitly verify that e.g. "flex-flow: column-reverse wrap" triggers those exact subproperty values).

(I'm spinning this off as a followup, since I'm pretty confident that these things all work; while I'm writing the reftests & running them through Try to make sure they're pixel-aligned everywhere, I think the multi-line flexbox patches can land with the tests that I've already written.)

Comment 1

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #0)
> >  - Computing the baseline of a vertical flex container (single and
> > multi-line)

BTW, I realized I was mistaken - we already do have tests for computing the baseline of a single-line vertical flex container after all -- all of the "vert" ones listed here: http://mxr.mozilla.org/mozilla-central/find?string=flexbox-baseline

So we only need new tests for getting the baseline of a multi-line vertical flex container, for that bullet-point.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: wip

These are nearly ready. Here's what I've got so far. I think I just want to add a few more "flexbox-baseline-multi-line-vert-*" tests (analogs of the existing & included-in-the-patch -horiz variants), and then this will be ready.

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: part 1: 'flex-flow' reftest

(In reply to Daniel Holbert [:dholbert] from comment #0)
> ...and it'd also probably be worth adding a very basic reftest for all the
> possible values of the "flex-flow" shorthand

Here's a patch to add the "flex-flow" reftest. This just tests all the possible values, on a flex container with 4 children. The reference case just uses a block with floated children, with their order hardcoded to match the testcase's horizontal and vertical ordering of the children.

I verified that this reftest passes on Firefox Nightly, Chrome dev channel, and Opera (presto) 12.16.

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: part 2: reftest for baseline-aligned items in 'flex-wrap:wrap' flex container

(In reply to Daniel Holbert [:dholbert] from comment #0)
> >  - Baseline alignment of items on multiple lines, simultaneously, in a
> > multi-line flex container (each line calculating its own baseline,
> > independently)

This patch does that ^^, by comparing a three-line flex container (in the testcase) to three consecutive single-line flex containers (in the reference case).

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: part 3: reftest for baseline-aligned items in 'flex-wrap:wrap-reverse' flex container

...and here's a forked version of that last reftest, but changed to use 'wrap-reverse'.

(I split this out into its own patch so that it could benefit from 'hg cp' and only touch the lines that are actually different from previous patch. Basically, this just does s/wrap/wrap-reverse/, tweaks the headers, and changes the order of the lines in the reference case.)

Comment 6

[Mats Palmgren (inactive)]

Comment on attachment 8346272 [details] [diff] [review]
part 3: reftest for baseline-aligned items in 'flex-wrap:wrap-reverse' flex container

r=mats

Comment 7

[Daniel Holbert [:dholbert]]

Landed parts 1-3:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/1125323d8a20
  https://hg.mozilla.org/integration/mozilla-inbound/rev/7f94fdaa1b42
  https://hg.mozilla.org/integration/mozilla-inbound/rev/31c92e4b907f

Additional patches coming soon, with reftests for computing the baseline of multi-line flex container itself. [leave open] in the meantime.

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1125323d8a20
https://hg.mozilla.org/mozilla-central/rev/7f94fdaa1b42
https://hg.mozilla.org/mozilla-central/rev/31c92e4b907f

Comment 9

[Daniel Holbert [:dholbert]]

Attachment: part 4: reftest for baseline *of* flex container, with baseline-aligned items on first line, and flex-wrap:wrap

Here's a reftest for how we compute the baseline the container itself, for a multi-line container with baseline-aligned items.

The spec says this:
 # If any of the flex items on the flex container’s
 # first line participate in baseline alignment, the
 # flex container’s main-axis baseline is the baseline
 # of those flex items.
http://dev.w3.org/csswg/css-flexbox/#flex-baselines

...which basically means we should use the baseline-alignment-position of the items on the first flex line as the baseline of the container.

The reference case mimics the multi-line flex container using a series of horizontal single-line flex containers, inside a vertical single-line flex container.  Conveniently, this nested-flex-container construct ends up with the same expected baseline. (A vertical single-line flex container's baseline is the baseline of its first flex item -- i.e. its first single-line-horizontal-flex-container-child -- and the baseline of that child is the baseline-alignment position of its one and only flex line.)

Comment 10

[Daniel Holbert [:dholbert]]

Attachment: part 5: reftest for baseline *of* flex container, with baseline-aligned items on first line, and flex-wrap:wrap-reverse

...and this patch adds a copy of that last test, with 'hg cp', using "wrap-reverse" to place the first flex line at the bottom instead of the top.

(Conveniently, we can mimic "wrap-reverse" in the reference case by just flipping the polarity of its vertical flex container. (and the baseline semantics still match the testcase))

Comment 11

[Mats Palmgren (inactive)]

Comment on attachment 8349170 [details] [diff] [review]
part 5: reftest for baseline *of* flex container, with baseline-aligned items on first line, and flex-wrap:wrap-reverse

r=mats

Comment 12

[Daniel Holbert [:dholbert]]

part 4: https://hg.mozilla.org/integration/mozilla-inbound/rev/4d47519bbf2b
part 5: https://hg.mozilla.org/integration/mozilla-inbound/rev/ea9b00cce3bb

Comment 13

[Daniel Holbert [:dholbert]]

Pushed a followup to parts 4 & 5 to drop the <link rel="help"> tags that I'd accidentally copied into the reference cases. (The w3c test harness only expects those tags to be in testcases.)
 https://hg.mozilla.org/integration/mozilla-inbound/rev/8b526dae1a89

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4d47519bbf2b
https://hg.mozilla.org/mozilla-central/rev/ea9b00cce3bb

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8b526dae1a89

Comment 16

[Daniel Holbert [:dholbert]]

I landed 2 last patches here, without explicit review since they're just tweaks to & variations on a few existing tests.

Part 6 tweaks some existing reftests to use 20px-sized flex items instead of 18px, since that makes the math easier in their (upcoming) vertical variants:
 https://hg.mozilla.org/integration/mozilla-inbound/rev/8d5036916ab7

Part 7 adds the aforementioned vertical variants:
 https://hg.mozilla.org/integration/mozilla-inbound/rev/ee7541e01eb1

(Note that I didn't create vertical variants of flexbox-baseline-multi-line-horiz-3.html or flexbox-baseline-multi-line-horiz-4.html because those tests are specifically about deriving a flex container's baseline from a flex line's baseline-alignment-position, which is only possible to do when the container's main axis is horizontal.) (at least, until we support vertical writing-modes)

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8d5036916ab7
https://hg.mozilla.org/mozilla-central/rev/ee7541e01eb1