Created attachment 54536 [details] HTML file to reproduce bug When a flexbox with non-zero box-flex property is resized to smaller size than the specified one, the height of the flexbox gets an unexpected value. If you open the attached HTML file, 100x100 mostly green box rendered is the expected result. However, current implementation renders 100x114 mostly green box.
Created attachment 54538 [details] Proposed Patch
Comment on attachment 54538 [details] Proposed Patch Generally, this looks good. Putting r- for some style nitpicks. LayoutTests/fast/flexbox/child-flexing.html:37 + overflow-y: auto; Some properties are duplicated with verticalOuter. I'd write div.outer { width: 100px; height: 100px; ... } Also, I'm not sure we are setting overflow-y. I think overflow: auto would be fine. LayoutTests/fast/flexbox/child-flexing.html:35 + box-align: top; I don't think box-align is necessary to check this behavior? LayoutTests/fast/flexbox/child-flexing.html:43 + background-color: green; We usually use greens to mean "the test passes". I think it's better to choose a different color. LayoutTests/fast/flexbox/child-flexing.html:53 + border-left: 90px solid olive; As the bug summary contains the word "padding", I think it's better to check paddings as well. LayoutTests/fast/flexbox/child-flexing.html:124 + shouldBe("element.scrollHeight", "100"); I'd add debug("vertically expanding") above this line. In this way we can see which test is failing easily. Or, we can just change the shouldBe lines like shouldBe("document.getElementById('expandVertical')", "100"); LayoutTests/fast/flexbox/child-flexing-expected.txt:9 + horizontally shrinking These messages are unnecessary. It may be better to put all HTMLs above id="console" into a single <div> and remove the div if window.layoutTestController exists. Please check printing/script-tests/pageNumerForElementById.js as an example.
Created attachment 54544 [details] 2010-04-28 Eric Seidel <eric@webkit.org>
Argh, wrong command line arg ended up uploading unrelated patch. Sorry for the noise. Will upload updated patch shortly.
Created attachment 54547 [details] Proposed Patch 2
Comment on attachment 54547 [details] Proposed Patch 2 Looks good. The original C++ code was apparently wrong and the test case appropriately checks the code around the fix. I'd land this manually so I'm setting cq-.
Committed r58630: <http://trac.webkit.org/changeset/58630>