[css-grid] min-width:0 on a grid item block in a column less than its min-content inline size makes its max-content block size too small

RESOLVED FIXED in Firefox 54

Status

()

Core
Layout
P2
normal
RESOLVED FIXED
6 months ago
6 months ago

People

(Reporter: forkest, Assigned: mats)

Tracking

(Blocks: 1 bug, {testcase})

52 Branch
mozilla55
testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +
qe-verify -

Firefox Tracking Flags

(firefox54 fixed, firefox55 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

6 months ago
User Agent: Mozilla/5.0 (Windows NT 5.1; rv:52.0) Gecko/20100101 Firefox/52.0
Build ID: 20170316213902

Steps to reproduce:

 <div style="display: grid; width: 5em;">
   <div style="word-wrap: break-word; min-width: 0;">
     first item with a longlonglongword
   </div>
   <div>
     second item
   </div>
 </div>


Actual results:

https://jsfiddle.net/L78foyue/

longlonglongword is printed over the second item


Expected results:

It should resize the first item so it contained all the words, not overflow it.
Check out the fiddle in Chrome.

Updated

6 months ago
Component: Untriaged → Layout
Keywords: testcase
Product: Firefox → Core
(Assignee)

Comment 1

6 months ago
Thanks for the bug report.
Assignee: nobody → mats
Blocks: 616605
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
OS: Unspecified → All
Priority: -- → P2
Hardware: Unspecified → All
Summary: word-wrap in a grid overflows the item instead of expanding it → [css-grid] min-width:0 on a grid item block in a column less than its min-content inline size makes its max-content block size too small
(Assignee)

Updated

6 months ago
Duplicate of this bug: 1352314
(Assignee)

Comment 3

6 months ago
Created attachment 8858703 [details] [diff] [review]
part 1 - [css-grid] Don't shrink-wrap the inline size when we have an available size when measuring block size.
Attachment #8858703 - Flags: review?(dholbert)
(Assignee)

Comment 4

6 months ago
Created attachment 8858704 [details] [diff] [review]
part 2 - [css-grid] Reftest.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=162e2e5d876f4aafb80d8f3db1447d0ad95d3dfa
(Assignee)

Comment 5

6 months ago
Created attachment 8858961 [details] [diff] [review]
part 2 - [css-grid] Reftests.

(added a reftest for bug 1349571 which is also fixed by this bug)
(Assignee)

Updated

6 months ago
Attachment #8858704 - Attachment is obsolete: true
(Assignee)

Updated

6 months ago
Blocks: 1349571
Comment on attachment 8858703 [details] [diff] [review]
part 1 - [css-grid] Don't shrink-wrap the inline size when we have an available size when measuring block size.

Review of attachment 8858703 [details] [diff] [review]:
-----------------------------------------------------------------

Makes sense. r=me
Attachment #8858703 - Flags: review?(dholbert) → review+
(Reporter)

Comment 7

6 months ago
Could the fix be also backported to the ESR channel? I'd like to use grid, but dropping Windows XP users is not an option, and I've found no way to work around this bug on 52 ESR.
(Assignee)

Comment 8

6 months ago
(In reply to forkest from comment #7)
> Could the fix be also backported to the ESR channel? I'd like to use grid,
> but dropping Windows XP users is not an option, and I've found no way to
> work around this bug on 52 ESR.

I'm sorry, I don't think Grid bug fixes fits the criteria for ESR inclusion.
Normally we only take security and crash fixes there.

Comment 9

6 months ago
Pushed by mpalmgren@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/255057a14c43
part 1 - [css-grid] Don't shrink-wrap the inline size when we have an available size when measuring block size.  r=dholbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/1712a5a3fdaf
part 2 - [css-grid] Reftests.
(Reporter)

Comment 10

6 months ago
(In reply to Mats Palmgren (:mats) from comment #8)
> I'm sorry, I don't think Grid bug fixes fits the criteria for ESR inclusion.
> Normally we only take security and crash fixes there.

I've hoped for a special case because of the XP users that will stick with this version forever. Also the patch seems very minor and therefore safe from the security standpoint.
(Assignee)

Updated

6 months ago
Flags: in-testsuite+

Comment 11

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/255057a14c43
https://hg.mozilla.org/mozilla-central/rev/1712a5a3fdaf
Status: ASSIGNED → RESOLVED
Last Resolved: 6 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
(Assignee)

Comment 12

6 months ago
Comment on attachment 8858703 [details] [diff] [review]
part 1 - [css-grid] Don't shrink-wrap the inline size when we have an available size when measuring block size.

Approval Request Comment
[Feature/Bug causing the regression]: CSS Grid feature
[User impact if declined]:broken grid layout
[Is this code covered by automated tests?]:yes
[Has the fix been verified in Nightly?]:yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]:we should take 1356820, 1348857, 1350925 in that order to avoid conflicts (they are technically independent though)
[Is the change risky?]:no
[Why is the change risky/not risky?]:only affects grid layout
[String changes made/needed]:none

I'd like to uplift all three of bug 1356820, bug 1348857, bug 1350925
to Beta v54 to fix some Grid layout bugs that web developers have reported.
Attachment #8858703 - Flags: approval-mozilla-beta?
(Reporter)

Comment 13

6 months ago
Given there is no workaround, I'd like to see it uplifted to ESR too, so the grid were fully usable for all those who target Windows XP users.
Who makes a final decision about it?
Comment on attachment 8858703 [details] [diff] [review]
part 1 - [css-grid] Don't shrink-wrap the inline size when we have an available size when measuring block size.

Fix a css grid issue. Beta54+. Should be in 54 beta 2.
Attachment #8858703 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
status-firefox54: --- → affected

Comment 15

6 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/826d40cdc956
https://hg.mozilla.org/releases/mozilla-beta/rev/4ca8d3cb1da1
status-firefox54: affected → fixed
(In reply to Mats Palmgren (:mats) from comment #12)
> [Feature/Bug causing the regression]: CSS Grid feature
> [User impact if declined]:broken grid layout
> [Is this code covered by automated tests?]:yes
> [Has the fix been verified in Nightly?]:yes
> [Needs manual test from QE? If yes, steps to reproduce]: no

Setting qe-verify- based on Mats' assessment on manual testing needs and the fact that this fix has automated coverage.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.