Closed Bug 1097984 Opened 10 years ago Closed 2 years ago

flex item with "flex-basis:auto; width:[non-auto]" ends up at its min-content size, instead of its max-content size

Categories

(Core :: Layout, defect)

defect

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

(Keywords: regression)

Attachments

(3 files)

Attached file testcase 1
See attached testcase.

I'd expect both examples to render with no linebreaks (and use the item's preferred width), but for some reason, the second example renders with the item's min-content width.

This is likely a regression from one of the changes to implement a flexbox spec change in the past few months.
Looks like nsContainerFrame::ComputeAutoSize bails out early (and returns an auto-width of "0xdeadbeef" -- that's hardcoded into ComputeAutoSize), if it sees that we have a non-auto "width" property.

In this bug's case, that's bad, because we really do need that auto-size (to resolve "flex-basis:auto") even though our width happens to have a non-auto value.

So right now, we end up resolving our flex-basis:auto to "0xdeadbeef" (a huge negative number), which then gets clamped to the min-width, which is our min-content size.  So that's why we end up being sized to our min-content width.

Anyway, we need to teach that function that our inline style coord is really "flex-basis" if we're a flex item in a horizontal flex container. (much like the code in nsFrame::ComputeSize and  nsLayoutUtils::ComputeSizeWithIntrinsicDimensions().)
Depends on: 1032922
Assignee: nobody → dholbert
Note that this bug is only possible to hit if "flex-basis:auto" really means "auto-size me".  If we back out the flex-basis:main-size rename (due to possible pending spec changes, per bug 1093316) and revert back to the old meaning of "flex-basis:auto", then we won't have this bug anymore.

In that possible future world, we'd still need to fix this for the "content" keyword, though (which is what's been proposed as the alternative way to request auto-sizing behavior from flex-basis).
Summary: flex item with specified "width" (overridden by flex-basis) ends up shrinking to min-content size → flex item with "flex-basis:auto; width:[non-auto]" ends up shrinking to min-content size, instead of max-content size
Summary: flex item with "flex-basis:auto; width:[non-auto]" ends up shrinking to min-content size, instead of max-content size → flex item with "flex-basis:auto; width:[non-auto]" ends up at its min-content size, instead of its max-content size
Attached patch WIP patchSplinter Review
Here's a WIP patch that fixes the testcase. (WIP because this has some copypasted logic that we probably should really factor out)

We also should perhaps consider making "IsFlexItem()" use a frame-state bit, so that we can check that without requiring a virtual function call. (IsFlexItem uses GetParent()->GetType() right now.)  That'd keep this patch from slowing down ComputeAutoSize.
Flags: needinfo?(dholbert)
Hi,

Do you have any updates on this? Seems like the attached testcase still shows the issue on latest Nightly (48.0a1).
No updates at the moment; this is indeed still broken.
Severity: normal → S3

This is a non-issue at this point. When I originally filed this bug, IIRC the flexbox spec said that flex-basis:auto literally meant "use the item's auto width, and ignore any specified size on it". That behavior was changed and is now behind the content keyword.

We match the expected results if I change the testcase with s/auto/content/, so this was fixed at some point.

Flags: needinfo?(dholbert)
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WORKSFORME
Attached file testcase 2

Here's a testcase with the newer content keyword, to demonstrate that this does indeed work properly.

(For the record: mozregression shows that we stopped collapsing all the way to the min-content width with bug 1093316, so that's essentially what fixed this. We changed behavior later for testcase 2 when we added support for the content keyword.)

Depends on: 1093316
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: