798235 - "Assertion failure: aCoord.IsCoordPercentCalcUnit()" with flexbox

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Jesse Ruderman]

Attachment: testcase (asserts fatally when loaded)

With
  user_pref("layout.css.flexbox.enabled", true);
the testcase triggers:

Assertion failure: aCoord.IsCoordPercentCalcUnit(), at layout/base/nsLayoutUtils.h:944

Comment 1

[Jesse Ruderman]

Attachment: stack

Comment 2

[Daniel Holbert [:dholbert]]

So this is from a case where we're using "flex-basis" in place of "height" (in a vertical flexbox), and "flex-basis" happens to be set to a special keyword -- -moz-max-content -- which is supported for "width" but not for "height".

I think it makes the most sense to treat these special keywords as if they were "auto", which (for flex-basis in a vertical flexbox) means "just use the value of the 'height' property"

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: fix

Here's the fix.

NOTE: After this patch, the code has this structure:

 if (horizontal) {
   // use flex basis for width
 } else {
   // comments
   if (not enum) {
     // use flex basis for height
   }
 }

That "else { ... if() {" could be collapsed to "else if () {, but I think I like it better with the "if" separated because the if-check is really just to filter out a special-case, and we're ultimately expecting to remove that check.

Happy to collapse it if you prefer, though.

Comment 4

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 668963 [details] [diff] [review]
fix

r=dbaron

Comment 5

[Daniel Holbert [:dholbert]]

Unprefixed testcase (per bug 801098), and pushed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5c67076a5a05

Comment 6

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/5c67076a5a05