[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

()

defect
P2
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: forkest, Assigned: mats)

Tracking

(Blocks 1 bug, {testcase})

52 Branch
mozilla55
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

2 years 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

2 years ago
Component: Untriaged → Layout
Keywords: testcase
Product: Firefox → Core
(Assignee)

Comment 1

2 years ago
Thanks for the bug report.
Assignee: nobody → mats
Blocks: css-grid
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

2 years ago
Duplicate of this bug: 1352314
(Assignee)

Comment 5

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

Updated

2 years ago
Attachment #8858704 - Attachment is obsolete: true
(Assignee)

Updated

2 years 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

2 years 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

2 years 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

2 years 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

2 years 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

2 years ago
Flags: in-testsuite+

Comment 11

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

Comment 12

2 years 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

2 years 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+
(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.