[css-grid] Grid computes negative size for abspos containing block |
|||
Issue description
fast/css-grid-layout/mozilla/grid-repeat-auto-fill-fit-005-part-2.html
For the above layout test, grid sets a negative containing block size:
#3 0x00007fffeb11775b in blink::LayoutBox::setOverrideContainingBlockContentLogicalHeight (this=0x1b8691618a30, logicalHeight=-15px)
at ../../third_party/WebKit/Source/core/layout/LayoutBox.cpp:1171
1171 DCHECK_GE(logicalHeight, LayoutUnit(-1));
(gdb)
#4 0x00007fffeb168229 in blink::LayoutGrid::layoutPositionedObjects (this=0x1b8691634250, relayoutChildren=true,
info=blink::LayoutBlock::DefaultLayout) at ../../third_party/WebKit/Source/core/layout/LayoutGrid.cpp:1912
1912 child->setOverrideContainingBlockContentLogicalHeight(rowBreadth);
(gdb) p rowBreadth
$1 = -15px
Found this when I tried to add a DCHECK for positive sizes. This is probably not correct.
,
Aug 22 2016
Does it involve absolutely positioned grid items with vertical writing modes? I ask because of issue 639873
,
Aug 22 2016
No writing modes involved -- all horizontal-bt. But some abspos is there. https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/fast/css-grid-layout/mozilla/grid-repeat-auto-fill-fit-005-part-2.html?q=fast/css-grid-layout/mozilla/grid-repeat-auto-fill-fit-005-part-2.html&sq=package:chromium&l=1
,
Aug 23 2016
Just having the following case on the test is already is already setting -15px: <div class="grid r1 w3"><x></x><x></x><a></a><b></b><x></x></div>
,
Aug 23 2016
I'm attaching a simplified test case in which this issue happens. The thing is that we have a grid with just 1 cell of 100px x 100px. However, the grid container itself is 50px x 50px. We place a positioned item with: grid-column: 2 / auto; grid-row: 2 / auto; "auto" line is resolved to the padding edge, so it's in the 50px position. As the items have a 100px offset, we calculate the breadth like 50px - 100px = -50px. I believe the current behavior is right, as the item is renderer like if it had a 0px containing box, which is the expected result in this situation. However, I agree that storing -50px on the overrideContainingBlockContentLogicalWidth|Height doesn't seem like a good idea. Probably we should avoid it and just store 0px there.
,
Aug 23 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/83e4af423ad2b700659ef45ea0790ae2f8fd30ea commit 83e4af423ad2b700659ef45ea0790ae2f8fd30ea Author: rego <rego@igalia.com> Date: Tue Aug 23 18:25:26 2016 [css-grid] Avoid negative values as override containing block sizes Add DCHECK to verify that we don't set negative values (other than -1 which means indefinite) as override containing block sizes. Modified LayoutGrid::offsetAndBreadthForPositionedChild() to never return negative breadths. BUG= 639620 Review-Url: https://codereview.chromium.org/2268383002 Cr-Commit-Position: refs/heads/master@{#413791} [modify] https://crrev.com/83e4af423ad2b700659ef45ea0790ae2f8fd30ea/third_party/WebKit/Source/core/layout/LayoutBox.cpp [modify] https://crrev.com/83e4af423ad2b700659ef45ea0790ae2f8fd30ea/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
,
Aug 24 2016
|
|||
►
Sign in to add a comment |
|||
Comment 1 by r...@igalia.com
, Aug 22 2016Cc: svil...@igalia.com
Status: Available (was: Untriaged)