Closed
Bug 1227285
Opened 9 years ago
Closed 9 years ago
[css-grid] Grid item with 100%-wide image ends up hugely tall (approximately nscoord_MAX)
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla45
Tracking | Status | |
---|---|---|
firefox45 | --- | fixed |
People
(Reporter: dholbert, Assigned: MatsPalmgren_bugz)
References
(Blocks 1 open bug)
Details
Attachments
(4 files)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
dholbert
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
dholbert
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
STR:
1. Load attached testcase.
2. See how tall the grid item (red border) & grid (gray border) are.
EXPECTED RESULTS:
Grid & grid item should be as tall as its content, the image. (Or maybe a little taller? In Chrome, they're a few pixels taller for some reason -- there's a strip of white between the img and the bottom borders.)
ACTUAL RESULTS:
Grid & grid item are hugely tall, sticking way off the bottom of the viewport. My debug build says the grid is 1013742296 app units tall, which is in the neighborhood of nscoord_MAX.
I'm using latest Nightly, 45.0a1 (2015-11-23).
(Noticed this bug while tweaking testcase for bug 1227140.)
I'm guessing we're messing things up when computing the "transferred size", or something like that.
Tentatively assigning to Mats, since he's likely who'll end up looking at this.
Reporter | ||
Comment 1•9 years ago
|
||
Assignee | ||
Comment 2•9 years ago
|
||
We need the min-content block-size for the the grid item, which requires a reflow
in this case, so we reflow it with an "infinite" inlinine-size (to force it to
not take any break opportunities other than explicit ones). The problem is that
the grid item is a block frame, and with the default justify-self:stretch it will
happily set its desired inline-size to that huge value (INFINITE_ISIZE_COORD).
That's what its children in turn will use as the available inline-size. In this
case the image with width:100% will get that width and its transferred size
(height) will be the same, which makes the block (grid item) block-size huge too.
Hmm, this is kind of the opposite of what we intended :-)
So I think the fix here is to use the ComputeSizeFlags::eShrinkWrap flag to
avoid the block (grid item) itself to grow beyond what its content demands.
Does that sound right?
(it fixes the problem here, and pass the grid reftests locally)
Assignee | ||
Comment 3•9 years ago
|
||
This adds a generic mechanism for requesting ComputeSizeFlags::eShrinkWrap
behavior when creating a nsHTMLReflowState, by giving COMPUTE_SIZE_SHRINK_WRAP
as a flag to the ctor.
Attachment #8692243 -
Flags: review?(dholbert)
Assignee | ||
Comment 4•9 years ago
|
||
Attachment #8692245 -
Flags: review?(dholbert)
Assignee | ||
Comment 5•9 years ago
|
||
Reporter | ||
Comment 6•9 years ago
|
||
Comment on attachment 8692243 [details] [diff] [review]
part 1 - Add a nsHTMLReflowState ctor flag to request shrink-wrap behavior
Review of attachment 8692243 [details] [diff] [review]:
-----------------------------------------------------------------
r=me
Attachment #8692243 -
Flags: review?(dholbert) → review+
Reporter | ||
Comment 7•9 years ago
|
||
Comment on attachment 8692245 [details] [diff] [review]
[css-grid] Request shrink-wrap behavior when doing a measuring reflow to figure out a grid item's block-size
Review of attachment 8692245 [details] [diff] [review]:
-----------------------------------------------------------------
r=me
Attachment #8692245 -
Flags: review?(dholbert) → review+
Reporter | ||
Comment 8•9 years ago
|
||
Comment on attachment 8692249 [details] [diff] [review]
reftests
Review of attachment 8692249 [details] [diff] [review]:
-----------------------------------------------------------------
Two nits on the test patch:
::: layout/reftests/css-grid/grid-min-max-content-sizing-002-ref.html
@@ +79,5 @@
> +</div>
> +
> +
> +
> +</body>
Nit: consider collapsing these 3 blank lines down to just 1 blank line, before </body> in the testcase & reference case here.
::: layout/reftests/css-grid/grid-min-max-content-sizing-002.html
@@ +7,5 @@
> + <meta charset="utf-8">
> + <title>CSS Grid Test: Testing grid minmax(min-content,max-content) column/rows</title>
> + <link rel="author" title="Mats Palmgren" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1227285">
> + <link rel="help" href="http://dev.w3.org/csswg/css-grid/#valdef-grid-template-columns-max-content">
> + <link rel="match" href="grid-min-max-content-sizing-001-ref.html">
This "match" line has the wrong reference filename. (needs s/001/002/)
Comment 11•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/fd03238ebdb5
https://hg.mozilla.org/mozilla-central/rev/ba839fc1b5d7
https://hg.mozilla.org/mozilla-central/rev/cf48c73a02ea
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in
before you can comment on or make changes to this bug.
Description
•