[css-flexbox-1] Add a test for definite cross sizes

[cbiesinger]

[syncbot]

Automatic validation checks of commit 7fa4e32 discovered the following problem:

In css-flexbox-1/definite-cross-sizes.html:

• Linked to unversioned specification URL https://drafts.csswg.org/css-flexbox/#definite-sizes. Use https://www.w3.org/TR/css-flexbox-1/... or https://drafts.csswg.org/css-flexbox-1/... instead.

[syncbot]

Automatic validation checks of commit 9dbf389 passed.

[dholbert]

This testcase doesn't seem to set display:flex on anything, so I think it might not be testing what you want it to test... Right now it has:

 .flexbox {
  width: 50px;
  outline: 3px solid black;
}

I expect you want to have display:flex in there?

Also, might be worth giving all of the flex items all min-height:0, so that there's no chance of spuriously "passing" a piece of the test simply because we're stopping a flex item from shrinking below its intrinsic minimum size.

[cbiesinger]

Oh that's a little subtle. The display: flex is set by the included stylesheet:
+<link rel="stylesheet" href="support/flexbox.css">

but the min-height is a good point. Will change.

[dholbert]

Review status: 0 of 1 files reviewed at latest revision, 2 unresolved discussions.

---

css-flexbox-1/definite-cross-sizes.html, line 38 [r1] (raw file):
Nit: This sentence ("Tests that we ...") ends in a period, but all the ones after it do not.

Really, they might all want to end in a colon, since they're all referring to the content directly after them...?

Regardless, probably best to make these consistent.

---

css-flexbox-1/definite-cross-sizes.html, line 65 [r1] (raw file):
Nit: s/an definite/a definite/ (drop the "n" from "an")

---

Comments from the review on Reviewable.io

[dholbert]

Review status: 0 of 1 files reviewed at latest revision, 4 unresolved discussions.

---

css-flexbox-1/definite-cross-sizes.html, line 1 [r1] (raw file):
This test's filename ("definite-cross-sizes") probably needs to include e.g. "001", right? Really, it might want to be named percentage-heights-001.html, since it's about percentage heights, and it's using the same check-layout-th.js library that the existing percentage-heights-000.html testcase uses.

---

css-flexbox-1/definite-cross-sizes.html, line 5 [r1] (raw file):
For consistency, I think this <style> block should come after all of the other boilerplate (the help/author tags in particular).

---

Comments from the review on Reviewable.io

[dholbert]

Review status: 0 of 1 files reviewed at latest revision, 4 unresolved discussions.

---

css-flexbox-1/definite-cross-sizes.html, line 5 [r1] (raw file):
(er, I typed "this <style> block", but I forgot to include backticks, so reviewable ate the <style>.)

---

Comments from the review on Reviewable.io

[syncbot]

Automatic validation checks of commit 17af0dc passed.

[cbiesinger]

css-flexbox-1/percentage-heights-001.html, line 38 at r1 (raw file):

Previously, dholbert (Daniel Holbert) wrote…

Nit: This sentence ("Tests that we ...") ends in a period, but all the ones after it do not.

Really, they might all want to end in a colon, since they're all referring to the content directly after them...?

Regardless, probably best to make these consistent.

Done.

---

Comments from Reviewable

[cbiesinger]

css-flexbox-1/percentage-heights-001.html, line 5 at r1 (raw file):

Previously, dholbert (Daniel Holbert) wrote…

(er, I typed "this <style> block", but I forgot to include backticks, so reviewable ate the <style>.)

Done.

---

Comments from Reviewable

[cbiesinger]

css-flexbox-1/percentage-heights-001.html, line 65 at r1 (raw file):

Previously, dholbert (Daniel Holbert) wrote…

Nit: s/an definite/a definite/ (drop the "n" from "an")

Done.

---

Comments from Reviewable

[cbiesinger]

css-flexbox-1/percentage-heights-001.html, line 1 at r1 (raw file):

Previously, dholbert (Daniel Holbert) wrote…

This test's filename ("definite-cross-sizes") probably needs to include e.g. "001", right? Really, it might want to be named percentage-heights-001.html, since it's about percentage heights, and it's using the same check-layout-th.js library that the existing percentage-heights-000.html testcase uses.

Done.

---

Comments from Reviewable

[cbiesinger]

All changes made, but please hold off while I verify this is all still correct.

[syncbot]

Automatic validation checks of commit cc1f578 passed.

[cbiesinger]

OK, this should be ready for merging -- updated to the current version of the spec (I think), and updated with a couple more tests from the current version of this testcase in blink.

r? @fantasai @dholbert

[wpt-issue-mover]

This issue has been moved to web-platform-tests/wpt#5282; please continue all discussion there.