Closed Bug 1597055 Opened 5 years ago Closed 4 years ago

Not setting max-height causes position: sticky to break

Categories

(Core :: Layout, defect, P3)

70 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla74
Tracking Status
firefox74 --- fixed

People

(Reporter: janosh.riebesell, Assigned: emilio)

References

()

Details

Attachments

(2 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/78.0.3904.97 Safari/537.36

Steps to reproduce:

position: sticky has no effect on <div class="styles__TocDiv-r9j83n-0 bEQwbu"> in a grid when its sibling <main class="PageBody___StyledMain-sc-151pelf-0 dVRgQl"> has grid-row: 1 applied to it. Unchecking grid-row: 1 on main makes the sticky div behave as expected. The problem does not occur in Chrome nor Safari.

Actual results:

The div behaves as it it was position: static.

Expected results:

position: sticky

Summary: Setting grid-row on a sibling breaks position: sticky → Not setting max-height causes position: sticky to break

Turns out specifying a max-height on <div class="styles__TocDiv-r9j83n-0 bEQwbu"> also prevents the issue.

Component: Untriaged → Layout
Product: Firefox → Core

Could you attach a test-case that reproduces the issue? Otherwise this is not actionable.

Flags: needinfo?(janosh.riebesell)

I can try. Could you specify what you're thinking of? Like a code sandbox?

A test-case can preferably be a single html file attached to bugzilla via the Attach New File interface, or a link to any online editors like jsfiddle or codepen.

Flags: needinfo?(janosh.riebesell)

aside-not-sticky-in-ff.html demonstrates the issue with minimal html. The <aside> element st sticky in both Chrome and Safari but not in Firefox.

Thanks. So this is about height: max-content being honored or not. In Firefox it behaves like auto. I think Firefox is right per css-sizing:

https://drafts.csswg.org/css-sizing/#valdef-width-max-content

If specified for the inline axis, use the max-content inline size; otherwise compute to auto.

(This compute to was changed to behaves as in css-sizing-4)

That sounds odd. What's the point of specifying max-height: max-content; if it just ends up as auto rather than the height of the content?

The height: max-content has to be valid because in vertical writing modes that is the inline direction.

The height: max-content has to be valid...

Not sure what that means. Could you explain further?

I mean that when you use a vertical writing mode (writing-mode: vertical-lr for example), then height and width are "swapped" in the sense that the inline direction is now determined by height. So height: max-content would have an effect there.

But that still leaves max-height: max-content; producing unexpected behavior in regular horizontal writing mode, wouldn't you agree? In effect, what Firefox is doing right now with max-height: max-content; is filling all of the available height as dictated by the parent container. This is the behavior I would expect from max-height: fill-available;. According to the docs

fill-available: The containing block's height minus the vertical margin, border, and padding.

Well, sure, but it's also the auto behavior for the grid item.

So you're saying Firefox should keep its current (at least in my opinion) unexpected behavior despite differing from Chrome and Safari?

I think so, yeah.

Per spec this looks like a bug in Chrome / Safari. Seems like https://github.com/w3c/csswg-drafts/issues/3973 was opened to discuss this.

BTW, if you don't want to stretch a grid item, just use align-self: start. No need to use height: max-content.

Emilio, while css-sizing-3 says "compute to auto", this was just a mistake against the resolution from #2708. It's supposed to be "behave as auto" like in css-sizing-4.

But from https://drafts.csswg.org/css-align-3/#valdef-justify-self-stretch,

When the box’s computed width/height (as appropriate to the axis) is auto [...]

Unless this is another mistake, the condition doesn't hold for height: max-content.
So if I'm not missing something, Firefox would be wrong.

(In reply to Oriol Brufau [:Oriol] from comment #16)

Unless this is another mistake, the condition doesn't hold for height: max-content.

I think that condition is wrong, fwiw. There's no reason "behaves as auto" wouldn't well, behave as auto :)

Well it seems to me that if an author sets height: max-content, the desire is to size it according to the content instead of stretching it to fill the containing block. Filed https://github.com/w3c/csswg-drafts/issues/4525 for discussion.

Well it seems to me that if an author sets height: max-content, the desire is to size it according to the content instead of stretching it to fill the containing block. Filed https://github.com/w3c/csswg-drafts/issues/4525 for discussion.

I was starting to think I'm the only one. :)

The priority flag is not set for this bug.
:mats, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(mats)
Priority: -- → P3
Assignee: nobody → emilio
Attachment #9122352 - Attachment description: Bug 1597055 - Don't apply automatic minimum size for grid items for non-auto block size. r=mats → Bug 1597055 - Don't stretch grid items with non-auto block-size. r=mats
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4eb80ad3235c
Don't stretch grid items with non-auto block-size. r=mats
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/21423 for changes under testing/web-platform/tests
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Status: UNCONFIRMED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla74
Upstream PR merged by moz-wptsync-bot
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: