1488080 - position: sticky doesn't work properly with flexbox

Status: RESOLVED FIXED

(Core :: Layout: Positioned, defect, P3)

Description

[mailnew4ster]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/68.0.3440.106 Safari/537.36

Steps to reproduce:

Context:
https://stackoverflow.com/questions/52137323/position-sticky-works-in-chrome-but-not-in-firefox/52137686

Demo:
http://jsfiddle.net/1p4gtruk/34/

Works properly on Chrome and Edge, so it's probably a Firefox issue.

HTML:
====================================
<div id="container" class="d-flex flex-column">
    <div class="flex-item">1</div>
    <div class="flex-item">2</div>
    <div class="d-flex flex-column last">
      <div class="flex-item mt-auto last-inner">3</div>
    </div>
</div>

CSS:
====================================
#container {
    height: 1000px;
}

#container .last {
    flex: 1;
}

#container .last-inner {
    position: sticky;
    bottom: 0;
}


Actual results:

The bottom div is at the bottom of the containing div.


Expected results:

The bottom div will be at the bottom of the viewport instead of at the bottom of the containing div.

Comment 1

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=2ac2b364cf5d83c6a336622cbda6b3ddb5f1e799&tochange=55602e9462ae04c0023339cda4b10ffa40d481c3


Regressed by: Bug 1298008



:bradwerth,
Your bunch of patch causes the regression, can you please look into this?

Comment 2

[Alice0775 White]

Attachment: stand alone test html

Comment 3

[Liz Henry (:lizzard) (relman/hg->git project)]

Happy to take a patch in nightly; if it seems low risk enough please feel free to request uplift to 65 beta.

Comment 4

[Andy Earnshaw]

Attachment: Self-contained example

I've attached an example demonstrating this failing my use case, hopefully it will serve as an addition check for any patch.  If you add `width:100%; height:100%;` to the sticky element then it renders, but removing `right: 0;` breaks it again.

Comment 6

[jonjohnjohnson]

Hey bradwerth, the only thing noteworthy in the duplicate bug 1553434 is that the current SCR creation in gecko is relative to the space in a flex container left aside from other flex items.

If you add this ruleset into the attached "stand alone test html" - https://bug1488080.bmoattachments.org/attachment.cgi?id=9005905

.last::before { content: ''; height: calc(2000px - 100vh - 16px); background: #99f; }

You'll see the bottom sticky block will scroll into view just as you start scrolling from the top of the page, because a previous sibling element takes up space in the flex container, which is sized to the containers height minus the margin on the body and the scrollport height.

Comment 7

[Daniel Holbert [:dholbert]]

Brad's been focused on other priorities, so I'll tentatively steal this from him [with his blessing], though I may not get to it right away.

Comment 8

[Andy Jakubowski]

Any updates on this? I’m having an issue where my last flex item is sticky, but there’s no room for it when you scroll all the way to the end of the flex container’s content. That is, the sticky flex item always overlaps content. It seems to work in Blink and WebKit, but not in Gecko.

Demo:
https://andyjakubowski.github.io/visual-explorations/238/

Comment 9

[Daniel Holbert [:dholbert]]

No updates, sorry. This slipped off my radar - hopefully I can get to it before much longer.

Also CC'ing TYLin who's been tackling flex bugs recently, in case he can get to this before I can.

Comment 10

[Andy Jakubowski]

Thanks, Daniel—I appreciate the quick response.

I think this would unlock some nice interface use cases without workarounds. For example, you could have a list of scrollable tab flex items, followed by a sticky button flex item that’s always visible regardless of the scroll position.

Right now the workaround seems to be separating the scrolling tab flex items and the sticky buttons in markup, and using position: absolute for the sticky button.

(In reply to Daniel Holbert [:dholbert] from comment #9)

No updates, sorry. This slipped off my radar - hopefully I can get to it before much longer.

Also CC'ing TYLin who's been tackling flex bugs recently, in case he can get to this before I can.

Comment 11

[volpatorv]

Hi,

I also encountered a problem that I think is related to this.
When using a sticky position inside of a flexbox, a div partially hidden behind a sticky div is scrollable in all browsers other than Firefox.

Here's a codepen that reproduces the issue: https://codepen.io/ricvolpe/pen/poddVzm

Actual results:
Content div scrolls until item six is behind footnote, making it invisible

Expected results:
Content div scrolls until item six is above footnote, making it visible

Comment 12

[Daniel Holbert [:dholbert]]

That sounds like this issue, yes. (BTW: I think TYLin is planning to investigate & fix this bug in the near future; if he doesn't end up having cycles for it, I've got it on a list of bugs to follow up on myself.)

Comment 13

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Fixed by bug 1377072. Testcases in comment 8 and comment 11 work fine on Firefox Nightly 99.0a1 (2022-03-06).

Comment 14

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Reopen because the testcase in comment 2 and the testcase in bug 1553434 still render differently on Firefox Nightly 99.0a1 (2022-03-06) and Chrome. They are not fixed by bug 1377072.

Comment 15

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Simplied testcase with only one flex item

I think the root cause is that Bug 1298008 stores the resolved auto margin in flex item's frame property in [1], but StickyScrollContainer::ComputeStickyLimits is trying to stick the flex item's margin-rect within the containing block's content box [2].

See the attached testcase. There are two scroll containers, and each contains a flex container. The left flex container has an item with margin-top: auto, so the item is aligned to the bottom of the flex container. The right flex container achieves a similar alignment by using justify-content: flex-end. If the item weren't position:sticky, both should render the same. But since the items are sticky, the item in the left flex container has a large resolved margin-top value, and its entire margin-rect is considered when computing the sticky boundary, where margin-rect of the item in the right container is just the orange background box.

Daniel, I don't have enough time to fix this before my planned leave, so I leave a trace here in case you or someone in our team want to figure out the solution.

[1] https://searchfox.org/mozilla-central/rev/9ca193b4233957439583f2eadabbd3cfb4cd9fed/layout/generic/nsFlexContainerFrame.cpp#5376-5384 (Remove this code fixed this bug, but of course it breaks bug 1298008.)
[2] https://searchfox.org/mozilla-central/rev/9ca193b4233957439583f2eadabbd3cfb4cd9fed/layout/generic/StickyScrollContainer.cpp#200-207

Comment 16

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1488080 Part 1 - Convert GetAllInFlowRects flags to EnumSet. r?#layout

Comment 17

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1488080 Part 2 - Use margin-box with 'auto' margin resolved as zero when computing the sticky limits. r?#layout,emilio,dholbert

In block or flex layout, an 'auto' margin can be used to align the element by
consuming rest of the space of a particular side in its container [1]. However,
if we used the resolved 'auto' margin as part of the element's margin-box, it
won't stick within its scroll container if the margin is larger than the scroll
container's size in that dimension. See bug 1488080 comment 15 for more
explanation.

[1] https://drafts.csswg.org/css-flexbox-1/#auto-margins

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8e5e55f87205
Part 1 - Convert GetAllInFlowRects flags to EnumSet. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/417324801b94
Part 2 - Use margin-box with 'auto' margin resolved as zero when computing the sticky limits. r=dholbert

Comment 19

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/46059 for changes under testing/web-platform/tests

Comment 20

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/8e5e55f87205
https://hg.mozilla.org/mozilla-central/rev/417324801b94

Comment 21

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 22

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1298008

Comment 23

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:TYLin, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox126 to wontfix.

For more information, please visit BugBot documentation.

Comment 24

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

If no, please set status-firefox126 to wontfix.

I'll just let the patch ride the train.