Closed Bug 1500627 Opened 1 year ago Closed 1 year ago

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

Categories

(Core :: Layout, defect, P2)

64 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla65
Tracking Status
firefox-esr60 --- unaffected
firefox63 --- unaffected
firefox64 + verified
firefox65 + verified

People

(Reporter: adamopenweb, Assigned: boris)

References

()

Details

(Keywords: regression)

Attachments

(3 files)

Attached image 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
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.
Blocks: 1496558
Flags: needinfo?(boris.chiou)
Yes, I guess I didn't catch the keywords in nsFlexContainerFrame.cpp properly. I am looking at this bug. Thanks. :)
Flags: needinfo?(boris.chiou)
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.
Assignee: nobody → boris.chiou
OK. Let me fix this bug (I have found the root cause), and do more fixes according to emilio's suggestion. :)
Thanks for jumping on this Boris and Emilio.
Priority: -- → P2
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.
There are still some potential assertions and bugs on handling keywords
in the block size, so we should fix them.

Depends on D9438
Status: NEW → ASSIGNED
Attachment #9019162 - Attachment description: Bug 1500627 - Use initial values for keywords in the block axis for flexbox frame → Bug 1500627 - Treat min-main-size:[keyword] as "auto" for flex items, if main axis is the item's block axis
Attachment #9019163 - Attachment description: Bug 1500627 - Fix some potential bugs of sizing properties for keywords in the block axis → Bug 1500627 - Fix some potential bugs of sizing properties for keywords in the block axis.
Attachment #9019162 - Attachment description: Bug 1500627 - Treat min-main-size:[keyword] as "auto" for flex items, if main axis is the item's block axis → Bug 1500627 - Treat min-main-size:[keyword] as "auto" for flex items, if main axis is the item's block axis.
Attachment #9019162 - Attachment description: Bug 1500627 - Treat min-main-size:[keyword] as "auto" for flex items, if main axis is the item's block axis. → Bug 1500627 - Treat min-main-size:[keyword] as "auto" for flex items, if main axis is the item's block axis
Attachment #9019163 - Attachment description: Bug 1500627 - Fix some potential bugs of sizing properties for keywords in the block axis. → Bug 1500627 - Fix some potential bugs of sizing properties for keywords in the block axis
Keywords: leave-open
Pushed by bchiou@mozilla.com:
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 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
Attachment #9019162 - Flags: approval-mozilla-beta?
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:
Attachment #9019163 - Flags: approval-mozilla-beta?
Keywords: leave-open
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Flags: qe-verify+
Flags: in-testsuite+
Target Milestone: --- → mozilla65
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.
Attachment #9019162 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9019163 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
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.
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.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Attachment #9018765 - Attachment description: 68747470733a2f2f776562636f6d7061742e636f6d2f75706c6f6164732f323031382f31302f30396231333466652d393766642d343232652d396266392d3163333463623464383932642d7468756d622e6a706567.jpg → screenshot
You need to log in before you can comment on or make changes to this bug.