1500627 - Div height is 0 with min-height: -moz-min-content in Nightly

Status: VERIFIED FIXED

(Core :: Layout, defect, P2)

Description

[Adam Stevenson [:adamopenweb]]

Attachment: screenshot

Originally reported: https://github.com/webcompat/web-bugs/issues/20057

Tested on:
Windows 10, MacOS 10.13, Android 9
Firefox Nightly 64

Steps to reproduce:
1. Navigate to: https://jselby.net/

Expected result:
Rectangular white divs with Github repos

Actual result:
Divs have 0 height

Looks like the tile class sets min-height: -moz-min-content

Mozregression returned:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=a552ab852b35f17b53441f1ce51f95a215f6329f&tochange=963840088024c60bc7e52d6046312989a2935705

Comment 1

[Emilio Cobos Álvarez (:emilio)]

min-height: -moz-min-content is supposed to behave as auto, so clearly bug 1496558 missed one place :)

Boris, do you have cycles to look into this? Just ni?ing so it doesn't fall off the radar.

Comment 2

[Boris Chiou [:boris]]

Yes, I guess I didn't catch the keywords in nsFlexContainerFrame.cpp properly. I am looking at this bug. Thanks. :)

Comment 3

[Emilio Cobos Álvarez (:emilio)]

FWIW it looks like a bunch of other assertions like:

  https://searchfox.org/mozilla-central/rev/9cb3e241502a2d47e2d5057ca771324a446b6695/layout/base/nsLayoutUtils.cpp#4879

Need to also be updated.

And looks like this font-inflation caller would also be bogus:

  https://searchfox.org/mozilla-central/rev/9cb3e241502a2d47e2d5057ca771324a446b6695/layout/base/nsLayoutUtils.cpp#8459

And this HeightDependsOnContainer:

  https://searchfox.org/mozilla-central/rev/9cb3e241502a2d47e2d5057ca771324a446b6695/layout/style/nsStyleStruct.h#1498

Where it'd actually depend on the writing mode, so probably should be changed to ISizeDependsOnContainer and BSizeDependsOnContainer.

Comment 4

[Boris Chiou [:boris]]

OK. Let me fix this bug (I have found the root cause), and do more fixes according to emilio's suggestion. :)

Comment 5

[Sean Voisen (:svoisen)]

Thanks for jumping on this Boris and Emilio.

Comment 6

[Boris Chiou [:boris]]

Attachment: Bug 1500627 - Treat min-main-size:[keyword] as "auto" for flex items, if main axis is the item's block axis

Keywords on the sizing properties in the block axis should behave as
initial values in the flex frame. We store the keywords as enum, instead
of auto or none in nsStyleCoord, so we have to handle it explicitly.

Comment 7

[Boris Chiou [:boris]]

Attachment: Bug 1500627 - Fix some potential bugs of sizing properties for keywords in the block axis

There are still some potential assertions and bugs on handling keywords
in the block size, so we should fix them.

Depends on D9438

Comment 8

[Boris Chiou [:boris]]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0aeb3112cb64e4c47f5671f793e428c36b031ff4

Comment 9

[Boris Chiou [:boris]]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8c4340399b1a822b6de7f7e60c0f8abbbb9d793c

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4ed8b1a5fee1
Treat min-main-size:[keyword] as "auto" for flex items, if main axis is the item's block axis r=dholbert
https://hg.mozilla.org/integration/autoland/rev/35a90bca7f63
Fix some potential bugs of sizing properties for keywords in the block axis r=mats

Comment 11

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/4ed8b1a5fee1
https://hg.mozilla.org/mozilla-central/rev/35a90bca7f63

Comment 12

[Boris Chiou [:boris]]

Comment on attachment 9019162 [details]
Bug 1500627 - Treat min-main-size:[keyword] as "auto" for flex items, if main axis is the item's block axis

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1496558

User impact if declined: The layout result is not correct if designers use keywords on sizing properties. This is pretty obvious.

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: Yes

If yes, steps to reproduce: Just make sure https://jselby.net/ could be rendered correctly, compared with our current beta version.

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Just a bug fix and add more testcases.

String changes made/needed: N/A

Comment 13

[Boris Chiou [:boris]]

Comment on attachment 9019163 [details]
Bug 1500627 - Fix some potential bugs of sizing properties for keywords in the block axis

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1496558

User impact if declined: This is related to bug 1496558 and fix more potential bugs if users use keywords on sizing properties.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Just a bug fix followed by my previous patch.

String changes made/needed:

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9019162 [details]
Bug 1500627 - Treat min-main-size:[keyword] as "auto" for flex items, if main axis is the item's block axis

[Triage Comment]
Fixes incorrect layout if designers use keywords on sizing properties. Approved for 64.0b6.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/1a9849bb7eae
https://hg.mozilla.org/releases/mozilla-beta/rev/3dcc599a53b5

Comment 16

[Hani Yacoub, Desktop QA]

Verified as fixed on Firefox Nightly 65.0a1 (2018-10-31) on Windows 10 x 64, Windows 7 x32, Mac OS X 10.13 and on Ubuntu 16.04 x64.

Comment 17

[Hani Yacoub, Desktop QA]

Verified as fixed on Firefox 64.0b6 on Windows 10 x 64, Windows 7 x32, Mac OS X 10.13 and on Ubuntu 16.04 x64.