[css-grid] Definite/indefinite size wrong detected for grid containers that are flex items |
|||||||||
Issue descriptionFlex items have some special conditions that make their size definite: https://drafts.csswg.org/css-flexbox/#definite-sizes However LayoutBox::hasDefiniteLogicalHeight() doesn't take into account those situations, so we're getting the wrong results in some cases. For example, a flex item with "auto" height can have a definite height in some situations, check LayoutFlexibleBox::mainSizeForPercentageResolution(). I'm attaching an example showing the issue.
,
Jun 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f8393437ca2c05d3334011444260adac7e82530c commit f8393437ca2c05d3334011444260adac7e82530c Author: rego <rego@igalia.com> Date: Wed Jun 08 21:25:25 2016 [css-grid] Fix definite/indefinite size detection on block axis LayoutBox::hasDefiniteLogicalHeight() was wrong as it was not taking into account several cases where the height is definite/indefinite. E.g. it was not checking the special situations for flex items, that despite of having height "auto" can definite sometimes. As the method was buggy, this patch is directly removing it and using LayoutBox::percentageLogicalHeightIsResolvable() instead. Add new test to check that the flexbox case works as expected. BUG= 617876 TEST=fast/css-grid-layout/flex-item-grid-container-percentage-rows.html Review-Url: https://codereview.chromium.org/2037383002 Cr-Commit-Position: refs/heads/master@{#398680} [add] https://crrev.com/f8393437ca2c05d3334011444260adac7e82530c/third_party/WebKit/LayoutTests/fast/css-grid-layout/flex-item-grid-container-percentage-rows-expected.html [add] https://crrev.com/f8393437ca2c05d3334011444260adac7e82530c/third_party/WebKit/LayoutTests/fast/css-grid-layout/flex-item-grid-container-percentage-rows.html [modify] https://crrev.com/f8393437ca2c05d3334011444260adac7e82530c/third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp [modify] https://crrev.com/f8393437ca2c05d3334011444260adac7e82530c/third_party/WebKit/Source/core/layout/LayoutBox.cpp [modify] https://crrev.com/f8393437ca2c05d3334011444260adac7e82530c/third_party/WebKit/Source/core/layout/LayoutBox.h [modify] https://crrev.com/f8393437ca2c05d3334011444260adac7e82530c/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
,
Jun 8 2016
,
Jun 9 2016
Verified the fix on the latest M-53(53.0.2763.0) on Windows-7, Mac OS 10.11.5 and Linux Ubuntu 14.04. This is working as intended on enabling Experimental Web platform features flag.
,
Aug 30 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9a12b00b915eccd82d4444ecba101f27e2761769 commit 9a12b00b915eccd82d4444ecba101f27e2761769 Author: rego <rego@igalia.com> Date: Tue Aug 30 23:00:30 2016 [css-grid] Fix indefinite height detection The change introduced in r398680 was wrong. In that occasion in order to check if the height of an element was indefinite, we were picking the first child and checking if a percentage was resolvable for it. This is not right, for example on a grid container where the first cell has a fixed height, if the first child is in that cell it can resolve a percentage. Thus, we were detecting that the grid container has a definite height when it might not be the case. This patch introduces back a LayoutBlock::hasDefiniteLogicalHeight() reusing the logic from LayoutBox::computePercentageLogicalHeight() that was extracted to a separated static method LayoutBlock::availableLogicalHeightForPercentageComputation(). That way we can call hasDefiniteLogicalHeight() directly on an element to determine if the height is or not definite. This fixes a TODO related to orthogonal flows and also the example explained above. The orthogonal flows test results are updated and a new test is included. BUG= 624301 , 617876 TEST=fast/css-grid-layout/grid-container-percentage-rows.html Review-Url: https://codereview.chromium.org/2154593003 Cr-Commit-Position: refs/heads/master@{#415478} [add] https://crrev.com/9a12b00b915eccd82d4444ecba101f27e2761769/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-container-percentage-rows.html [add] https://crrev.com/9a12b00b915eccd82d4444ecba101f27e2761769/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-item-grid-container-percentage-rows-expected.html [add] https://crrev.com/9a12b00b915eccd82d4444ecba101f27e2761769/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-item-grid-container-percentage-rows.html [modify] https://crrev.com/9a12b00b915eccd82d4444ecba101f27e2761769/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-track-sizing-with-percentages-and-orthogonal-flows.html [add] https://crrev.com/9a12b00b915eccd82d4444ecba101f27e2761769/third_party/WebKit/LayoutTests/fast/css-grid-layout/positioned-grid-container-item-percentage-size-expected.html [add] https://crrev.com/9a12b00b915eccd82d4444ecba101f27e2761769/third_party/WebKit/LayoutTests/fast/css-grid-layout/positioned-grid-container-item-percentage-size.html [modify] https://crrev.com/9a12b00b915eccd82d4444ecba101f27e2761769/third_party/WebKit/Source/core/layout/LayoutBlock.cpp [modify] https://crrev.com/9a12b00b915eccd82d4444ecba101f27e2761769/third_party/WebKit/Source/core/layout/LayoutBlock.h [modify] https://crrev.com/9a12b00b915eccd82d4444ecba101f27e2761769/third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp [modify] https://crrev.com/9a12b00b915eccd82d4444ecba101f27e2761769/third_party/WebKit/Source/core/layout/LayoutBox.cpp [modify] https://crrev.com/9a12b00b915eccd82d4444ecba101f27e2761769/third_party/WebKit/Source/core/layout/LayoutBox.h [modify] https://crrev.com/9a12b00b915eccd82d4444ecba101f27e2761769/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp [modify] https://crrev.com/9a12b00b915eccd82d4444ecba101f27e2761769/third_party/WebKit/Source/core/layout/LayoutGrid.cpp |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by r...@igalia.com
, Jun 8 2016