989436 - Add a few new flexbox reftests to catch bugs from early versions of bug 983427's patches

Status: RESOLVED FIXED

(Core :: Layout, defect, P4)

Description

[Daniel Holbert [:dholbert]]

The first (local) versions of my patches for bug 983427 had a few bugs that I caught which weren't revealed by any of our reftests.

Filing this bug to add a few reftests to prevent the introduction of these bugs.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: part 1: variant of existing flex-flow megatest, with 3 items per container instead of 4

This adds an existing variant of the flex-flow megatest.

The current test exercises content with 4 flex items like:
  1 2
  3 4
in all the various orientations (and without wrapping).

The new test is the same, but only with 3 flex items, to make sure that we fill the first row before the second row (regardless of orientation). So e.g. row wrap-reverse should produce:
  3
  1 2

...instead of e.g....
  2 3
  1
(which an early naive version of my patch incorrectly produced, as discussed in bug 983434 comment 1)

Comment 2

[Daniel Holbert [:dholbert]]

(by "and without wrapping", I meant "with and without wrapping")

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: part 2: rename & clone existing flex-item baseline-alignment test to check flex item baseline-alignment when cross axis is bottom-to-top

This patch renames an existing test (flexbox-align-self-baseline-horiz-1.xhtml) to have an "a" suffix, so that I can create a "b" clone with flex-wrap:wrap-reverse.  This just flips the cross axis, which shouldn't actually have any effect on the rendering in this case (hence, "a" and "b" share a reference case).

The point is, the reversed cross-axis exercises the flipping logic added in bug 983427 and makes sure it doesn't mess up baseline-alignment of flex items.

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: part 2: rename & clone existing flex-item baseline-alignment test to check flex item baseline-alignment when cross axis is bottom-to-top

(Fixing typo (s/part 1/part 2/) in commit message)

Comment 5

[Mats Palmgren (inactive)]

Comment on attachment 8398703 [details] [diff] [review]
part 1: variant of existing flex-flow megatest, with 3 items per container instead of 4

r=mats

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: part 3a: Rename multi-line reftests flexbox-align-self-baseline-horiz-[23] to -[45], so I can add single-line reftests named 2 and 3 in next patch

This patch moves some existing multi-line reftests out of the way (increasing the number in their filename), so that I can add a few more single-line reftests adjacent to the existing single-line reftests.

(Not bothering with review on this one since it's just a rename plus a resulting manifest/<link rel="match">-tweak)

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: part 3b: Add reftests for snapping cross-axis margin-edge of baseline-aligned flex item to flex-start edge of container, when container has a fixed size

This patch adds a few tests for positioning of a single baseline-aligned flex item, with margins, in a flex container with a fixed height.

The point of this is to test behavior where there's either extra space or not enough space in the flex container -- in that situation, baseline-aligned flex items snap their appropriate margin-edge to the container's *flex-start* edge. (normally the top of the container, but if we have "flex-flow: wrap-reverse", then it'll need to align to the bottom of the container)

(Similar behavior happens for explicitly "flex-start"-aligned flex items, but that code is more trivial (and better-tested) than the code for baseline-aligned flex items.)

The two tests added here are identical to each other, except for the "flex-wrap: wrap-reverse" flipping of the cross axis.

Comment 8

[Daniel Holbert [:dholbert]]

(The sort of interesting thing about the tests in "part 3b" is that the margins on the "flex-start" side are the only ones that affect the items' positioning, since the flex-start edge is what gets aligned with the container, and any margins sticking off of their other end will just protrude [or not] invisibly.

This is why the two reftests in part 3b have different reference cases based -- when we flip the cross axis, we flip the set of margins actually end up impacting our alignment [top vs. bottom].

This isn't the case in the {a,b} tests in part 2 -- in that case, we render the same regardless of cross-axis orientation -- because there, the container has no fixed height and just shrinkwraps its items' margin-boxes. So there's no extra flex-[start/end] snapping that needs to happen after the baseline-alignment.)

Comment 9

[Daniel Holbert [:dholbert]]

https://tbpl.mozilla.org/?tree=Try&rev=c4caa77716df

Comment 10

[Daniel Holbert [:dholbert]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/aed905bca4d4
https://hg.mozilla.org/integration/mozilla-inbound/rev/6892d0777d0e
https://hg.mozilla.org/integration/mozilla-inbound/rev/5aaa6d2b4b3a
https://hg.mozilla.org/integration/mozilla-inbound/rev/110c1a3fc74c

Comment 11

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/aed905bca4d4
https://hg.mozilla.org/mozilla-central/rev/6892d0777d0e
https://hg.mozilla.org/mozilla-central/rev/5aaa6d2b4b3a
https://hg.mozilla.org/mozilla-central/rev/110c1a3fc74c