RESOLVED FIXED 116107
REGRESSION: Basic flexbox layout with auto heights is broken. 
https://bugs.webkit.org/show_bug.cgi?id=116107
Summary REGRESSION: Basic flexbox layout with auto heights is broken.
Dave Hyatt
Reported 2013-05-14 08:50:46 PDT
r147261 broke basic vertical flexbox layout, since flexing items no lay out with height of 0 when auto heights are specified. This is obviously wrong. They should be allowed to flex down to size 0, but when they are just getting an infinite amount of space to play with (which they should when computing an auto height), they should lay out at their preferred size.
Attachments
Simple reduction that illustrates the problem. (186 bytes, text/html)
2013-05-14 08:51 PDT, Dave Hyatt 		no flags
Details
Reduction (173 bytes, text/html)
2013-05-14 08:51 PDT, Dave Hyatt 		no flags
Details
Patch (3.71 KB, patch)
2013-05-14 08:57 PDT, Dave Hyatt 		no flags
DetailsFormatted DiffDiff
Patch (3.97 KB, patch)
2013-05-14 09:02 PDT, Dave Hyatt 		no flags
DetailsFormatted DiffDiff
Patch (fixed the intrinsic auto snafus) (3.97 KB, patch)
2013-05-14 09:05 PDT, Dave Hyatt 		no flags
DetailsFormatted DiffDiff
Patch with layout test. (7.84 KB, patch)
2013-05-14 09:28 PDT, Dave Hyatt 		no flags
DetailsFormatted DiffDiff
Patch (10.50 KB, patch)
2013-05-14 12:04 PDT, Dave Hyatt 		no flags
DetailsFormatted DiffDiff
Patch (10.83 KB, patch)
2013-05-14 12:26 PDT, Dave Hyatt 		no flags
DetailsFormatted DiffDiff
Patch (18.22 KB, patch)
2013-05-14 12:43 PDT, Dave Hyatt 		simon.fraser: review+
DetailsFormatted DiffDiff
Show Obsolete (8) View All Add attachment proposed patch, testcase, etc.
Dave Hyatt
Comment 1 2013-05-14 08:51:20 PDT
Created attachment 201718 [details] Simple reduction that illustrates the problem.
Dave Hyatt
Comment 2 2013-05-14 08:51:52 PDT
Created attachment 201719 [details] Reduction
Dave Hyatt
Comment 3 2013-05-14 08:57:20 PDT
Created attachment 201720 [details] Patch
Dave Hyatt
Comment 4 2013-05-14 09:02:59 PDT
Created attachment 201722 [details] Patch
Dave Hyatt
Comment 5 2013-05-14 09:05:02 PDT
Created attachment 201723 [details] Patch (fixed the intrinsic auto snafus)
Simon Fraser (smfr)
Comment 6 2013-05-14 09:09:03 PDT
Comment on attachment 201723 [details] Patch (fixed the intrinsic auto snafus) Do you plan to add a test case?
Dave Hyatt
Comment 7 2013-05-14 09:28:46 PDT
Created attachment 201726 [details] Patch with layout test.
Dave Hyatt
Comment 8 2013-05-14 11:17:00 PDT
Cleared the review bit, since I misunderstood what flex-basis was. The current rendering is unfortunately correct according to the latest flexbox draft. I think the spec will end up changing though. For now in WebKit only, I'm going to make flex-basis default to auto so that it behaves like the old flexbox. We can figure out what to do later when flex-basis stops churning.
Christian Biesinger
Comment 9 2013-05-14 12:02:11 PDT
*** Bug 115329 has been marked as a duplicate of this bug. ***
Dave Hyatt
Comment 10 2013-05-14 12:04:19 PDT
Created attachment 201738 [details] Patch
Dave Hyatt
Comment 11 2013-05-14 12:26:58 PDT
Created attachment 201741 [details] Patch
WebKit Commit Bot
Comment 12 2013-05-14 12:28:56 PDT
Attachment 201741 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/rendering/RenderFlexibleBox.cpp', u'Source/WebCore/rendering/RenderFlexibleBox.h']" exit_code: 1 Source/WebCore/rendering/RenderFlexibleBox.cpp:746: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Dave Hyatt
Comment 13 2013-05-14 12:43:24 PDT
Created attachment 201744 [details] Patch
Dave Hyatt
Comment 14 2013-05-14 13:36:46 PDT
Landed in r150087.
Ryosuke Niwa
Comment 15 2013-05-14 23:07:16 PDT
It seems like the test added by this patch has been failing on Mac: e.g. http://build.webkit.org/results/Apple%20MountainLion%20Release%20WK1%20(Tests)/r150087%20(9969)/results.html
Note You need to log in before you can comment on or make changes to this bug.