[css-grid] wrong number of repeat(auto-fill) rows with grid-auto-flow:column |
|||||
Issue descriptionVersion: Version 55.0.2859.0 dev (64-bit) OS: Linux What steps will reproduce the problem? (1) load https://people.mozilla.org/~mpalmgren/tests/chrome-bug-intrinsic-repeat-row.html (2) (3) What is the expected output? https://people.mozilla.org/~mpalmgren/tests/chrome-bug-intrinsic-repeat-row.png (this screenshot is from my local build of Firefox where I've just fix this, it should be in Nightly in a few days) What do you see instead? Only two rows, causing the blue item to be placed in the second column. Please use labels and text to provide additional information.
,
Sep 29 2016
> According to the specs the definite min size should only be used > if there is no definite size and max size (note that it says "Otherwise" Not exactly. I agree it says that you should use the min size for calculating the number of tracks when the size and max size are indefinite. But, it does NOT say anything about how the min size should be used when a size or max size is also specified. In the absence of anything explicit, I think it's reasonable to assume that the min size trumps the used value of size/max size like it always does, per CSS 2.2: https://www.w3.org/TR/CSS22/visudet.html#min-max-widths "The following algorithm describes how the two properties influence the used value of the 'width' property: ..." BTW, Chrome already applies the 'min-height' value when a 'height' is specified. The bug only occurs for 'max-height', but 'min-height' should trump that too per CSS 2.2.
,
Sep 29 2016
Here's the 'height' + 'min-height' test, which works as expected.
,
Oct 3 2016
,
Oct 10 2016
Would you take this one, svillar@ and work out a clarification of the spec or WontFix this issue? Otherwise perhaps set to Available, thanks.
,
Oct 12 2016
,
Oct 13 2016
Regarding what Mats mentions in https://bugs.chromium.org/p/chromium/issues/detail?id=648814#c2 then this means that both Blink and FF are doing it wrong when having a definite size and a definite min-size. Check http://jsbin.com/reduhitira/edit?html,css,output In the second grid both engines are computing 3 repetitions because the width is 80px, but according to your reasoning they should be computing 4 (because min-width is 100px)
,
Oct 13 2016
> In the second grid both engines are computing 3 repetitions because the width > is 80px, but according to your reasoning they should be computing 4 (because > min-width is 100px) No, it should be 3 repetitions in that case. The spec is very clear that when the size or max size is definite then we should only fill as many repetitions that fits *within* that size: https://drafts.csswg.org/css-grid/#auto-repeat "if the grid container has a definite size or max size in the relevant axis, then the number of repetitions is the largest possible positive integer that does not cause the grid to overflow..." It's only when *both* the size and max size are indefinite that we should use the 2nd part of the algo that says "Otherwise, if the grid container has a definite min size in the relevant axis, the number of repetitions is the smallest possible positive integer that fulfills that minimum requirement.". You seem to argue that when min-width "wins" over a specified width/max-width we should use the second part of the algo (for min size), but there is no spec text to support that as far as I can see. Please note that the second part starts with a very clear "Otherwise".
,
Oct 13 2016
Regarding the example, yeah it's clearly 3, I somehow mixed some things up there. Regarding the second thing, I think I am not understanding you. I said in comment 2 "So as there is a definite max size (50px) we use it to compute just 2 repetitions. According to the specs the definite min size should only be used if there is no definite size and max size (note that it says "Otherwise"." I think you're saying exactly the same in comment 8, something that is really confusing to me because it's totally the opposite to what you have said in comment 2 unless I'm missing something.
,
Oct 13 2016
I replied too fast, I think I didn't mix things up. I think that it can never be 3, actually it should be either 2 or 4 depending on how the specs are interpreted. There are 2 possibilities: a) min-size is used always (as you defend on comment 2): then the number of repetitions must be 4, because even if width:80px you must obey min-width:100px, then you need at least 4 repetitions to fulfill it. b) min-size is used only if size and max-size are indefinite (as I defend, and as you seem to suggest in comment 8 contradicting comment 2?): then the number of repetitions is 2, because we're using 80px and any extra repetition will overflow.
,
Oct 13 2016
I think you're confused by CSS specified values and used values. When you have "width: 80px; min-width: 100px;" the _used width_ is 100px (per CSS2.1 as I explained in comment 2). This is the value we should use (as "size" in part 1 of the algo) when calculating the number of repeat tracks. When the size or max size is definite, (i.e. not 'auto'/'none' respectively, and not a percentage of an indefinite percent basis) then the value to use in part 1 of the repeat algorithm is the 'used value' (as defined per CSS2.1), and that includes applying the computed 'min-width' to calculate that used value. That doesn't imply that just because a 'min-width' was specified we should use part 2 of the algorithm. The spec clearly does not want that since part 2 starts with "Otherwise".
,
Oct 13 2016
OK I got your point, I was confused by comment 2, but I guess we're on the same page now. I thought that you were defending using the min-size unconditionally but now I understand what you meant. Fixing it...
,
Oct 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8f030c73af4b8b82752c373cc98ef60a7e87a780 commit 8f030c73af4b8b82752c373cc98ef60a7e87a780 Author: svillar <svillar@igalia.com> Date: Fri Oct 14 15:24:54 2016 [css-grid] Constrain by min-height on auto-repeat computation The max-height (if definite) is used to compute the number of auto repeat rows whenever the height is indefinite. We were using the min-height only in case both values were indefinite. However, it's reasonable to assume that the min-height trumps the used value of height/max-height like it always does, per CSS 2.2. Note that the number of rows still needs to fit within that size even if using min-height, because we're just using min-height to compute the used value of the height property. If both height and max-height are indefinite we keep doing the same, i.e., compute the minimum number of rows that fulfill min-height (if definite). BUG= 648814 Review-Url: https://codereview.chromium.org/2421593002 Cr-Commit-Position: refs/heads/master@{#425327} [modify] https://crrev.com/8f030c73af4b8b82752c373cc98ef60a7e87a780/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-auto-fill-rows.html [modify] https://crrev.com/8f030c73af4b8b82752c373cc98ef60a7e87a780/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-indefinite-size-auto-repeat-crash-expected.txt [modify] https://crrev.com/8f030c73af4b8b82752c373cc98ef60a7e87a780/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-indefinite-size-auto-repeat-crash.html [modify] https://crrev.com/8f030c73af4b8b82752c373cc98ef60a7e87a780/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
,
Oct 14 2016
Closing |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by svil...@igalia.com
, Sep 29 2016