Closed Bug 1387152 Opened 2 years ago Closed 2 years ago

We shouldn't use "flex-basis" for sizing children of a -webkit-box

Categories

(Core :: Layout, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

Attachments

(2 files)

In bug 1382252, we've got a legacy -webkit-box container, whose child ends up unfortunately being 0 sized.

We size the child to 0 because:
 (1) We emulate the -webkit-box container using nsFlexContainerFrame
 (2) For children of a nsFlexContainerFrame, we use "flex-basis" instead of "width" when determining their desired size.
 (3) And in this case, the child happens to have "flex-basis: 0" (via the "flex" shorthand).

So the child ends up being 0-sized. :(

We shouldn't be using "flex-basis" at all in this case -- we should ignore it and use 'width' instead.  (Or equivalently, behave as if it were at its default value "auto", which would also make us use 'width'.)

I have patches for this already -- I'll post them shortly.
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Mats, if you wouldn't mind reviewing this when you're back, that would be awesome. :)  (MozReview seems to have dropped the review request, probably because you're on vacation.)
Flags: needinfo?(mats)
Comment on attachment 8893504 [details]
Bug 1387152 part 1: Adjust indentation and use HasAnyStateBits() instead of manual bitwise arithmetic, in nsFrame size-computation code.

https://reviewboard.mozilla.org/r/164598/#review178630
Attachment #8893504 - Flags: review+
Comment on attachment 8893505 [details]
Bug 1387152 part 2: Don't let unrelated property "flex-basis" influence the sizing inside of -webkit-box containers.

https://reviewboard.mozilla.org/r/164600/#review178632
Attachment #8893505 - Flags: review+
Thanks! I triggered a try run via MozReview, and I'll land assuming that succeeds.
Flags: needinfo?(mats) → needinfo?(dholbert)
(just circling back to this)

My Try run had some failures in dom/tests/mochitest/general/test_interfaces.html (failures like "ByteLengthQueuingStrategy should NOT be defined on the XBL scope" and "DANGER, are you sure you want to expose the new interface ByteLengthQueuingStrategy to all webpages as a property on the window") -- but I'm sure those are unrelated to this patch. I must've been based on top of an iffy cset that was backed out or fixed soon after it landed, and I just got unlucky enough to stack my patches on top of it before the backout.

Anyway -- reftests look good (aside from a few unrelated tests that are failing due to rendering entirely-blank, which have intermittent bugs filed already).  So, I'll go ahead and land.
Flags: needinfo?(dholbert)
Pushed by dholbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/929f225ca720
part 1: Adjust indentation and use HasAnyStateBits() instead of manual bitwise arithmetic, in nsFrame size-computation code. r=mats
https://hg.mozilla.org/integration/autoland/rev/d5110841090c
part 2: Don't let unrelated property "flex-basis" influence the sizing inside of -webkit-box containers. r=mats
https://hg.mozilla.org/mozilla-central/rev/929f225ca720
https://hg.mozilla.org/mozilla-central/rev/d5110841090c
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.