Closed Bug 1356820 Opened 7 years ago Closed 7 years ago

[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

Categories

(Core :: Layout, defect, P2)

52 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: forkest, Assigned: MatsPalmgren_bugz)

References

(Blocks 1 open bug)

Details

(Keywords: testcase)

Attachments

(2 files, 1 obsolete file)

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.
Component: Untriaged → Layout
Keywords: testcase
Product: Firefox → Core
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
(added a reftest for bug 1349571 which is also fixed by this bug)
Attachment #8858704 - Attachment is obsolete: true
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+
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.
(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.
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.
(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.
Flags: in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/255057a14c43
https://hg.mozilla.org/mozilla-central/rev/1712a5a3fdaf
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
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?
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.

Attachment

General

Created:
Updated:
Size: