1042151 - Content that overflows off the "start" side of a "flex-direction: *-reverse" flex container is not scrollable (but should be)

Status: RESOLVED FIXED

(Core :: Layout: Flexbox, defect, P2)

Description

[Anthony Ricaud (:rik)]

Attachment: flex-reverse-overflow-testcase.html

See attached testcase.

The two boxes are identical except the first one has a flex-direction: column, the second one column-reverse.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: comparison, demonstrating same issue w/ relative positioning

Hmm.

I think we have this behavior in general, where content that runs off the top of a "overflow:[non-visible]" element isn't reachable via scrolling.  Here's a testcase that demonstrates this with relative positioning.

Comment 2

[Daniel Holbert [:dholbert]]

Chrome reacts to Anthony's initial testcase here by putting the scrollbar all the way at the bottom, in the "column-reverse" example.  (So, scrollTop starts out at something nonzero.)

That seems like a reasonable behavior.

I'm looking for spec text on how this is supposed to work... It looks like the closest thing is this, which defines the scrollable area: 
 http://www.w3.org/TR/cssom-view/#scrolling-area

That definition relies on the "overflow directions", which are defined here: 
 http://www.w3.org/TR/cssom-view/#overflow-directions
...and which seem to be based on block flow direction & inline flow direction. (which aren't influenced by "flex-direction").

So per the letter of the spec,
 - the inline-flow & block-flow directions are still left to right, top to bottom
 - which means the "overflow directions" are rightward & downward
 - which means the top edge of the scrollable area is "The element's top padding edge." (that is -- the flex container's own top padding edge -- *not* anything dependent on its children)
 - which means we're correct in not allowing these children to be reachable via scrolling.

I think Chrome's behavior seems more useful, though... I'll bet flex containers *want* to establish their overflow directions as being the main-end & cross-end edges of the flex container. (instead of being dependent on block / inline direction)

I'll send a www-style post to ask about this later today.

Comment 3

[Daniel Holbert [:dholbert]]

(Also: FWIW, IE11 renders the initial testcase here the same way that we do.)

Comment 4

[Anthony Ricaud (:rik)]

If that can help with www-style discussions, we saw this bug with Jan when prototyping an IRC client. New messages would be inserted as the first children of #container.

Comment 5

[Daniel Holbert [:dholbert]]

Posted to www-style: http://lists.w3.org/Archives/Public/www-style/2014Jul/0380.html

Comment 6

[Jan Keromnes [:janx]]

FWIW, bug 962501 has a similar issue where adding `justify-content: flex-end` prevents any scrolling (in Chrome as well as in Firefox).

For context, Anthony suggested using `justify-content: flex-end` (and reversing the order of the elements in javascript) as a workaround for the present `flex-direction: column-reverse` scroll problem. It turns out we still can't scroll.

Comment 7

[Daniel Holbert [:dholbert]]

I was worried that might be the case, but then I tested locally and came up with something that worked (I thought).  I'll see if I can recreate that.

Comment 8

[Daniel Holbert [:dholbert]]

Attachment: (mock-up IRC client that uses "flex-end" & doesn't need browser-support for overflow off the top of an element)

Yeah, you're right - justify-content:flex-end + overflow:auto don't quite work, on the same element.  But, we can make it work if we put "flex-end" on a flex-container that is the *parent* of the scrollable one. (so that the scrollable one still overflows off of its bottom & creates a scrollbar, and when it's too short to have any overflow, its parent snaps it to the bottom)  This demo has code to keep the scrollbar snapped to the bottom, too, taken from https://developer.mozilla.org/en-US/docs/Web/API/Element.scrollHeight. This makes it behave like my terminal & my IRC client on my desktop machine. 

Anyway, sorry for misleading you & Anthony (slightly) about "flex-end" & overflow yesterday, but hopefully with the wrapper, it does what we need.

Let's take any further IRC-client-discussion somewhere else (maybe the dev.tech.layout newsgroup), and keep this bug focused on the overflow behavior of flex-direction:column-reverse. (particularly after we hear back on www-style)

Comment 9

[Daniel Holbert [:dholbert]]

dbaron suggests the might behavior might fall out naturally from nsLayoutUtils::GetScrolledRect, if we tweak that function to be aware of flexbox main/cross axes.
http://mxr.mozilla.org/mozilla-central/source/layout/base/nsLayoutUtils.h?rev=3a5cfc71b3d3#549
http://mxr.mozilla.org/mozilla-central/source/layout/base/nsLayoutUtils.cpp?rev=50623b494578#1805

Comment 10

[Daniel Holbert [:dholbert]]

*the right behavior

Comment 11

[Jan Keromnes [:janx]]

Comment on attachment 8461267 [details]
(mock-up IRC client that uses "flex-end" & doesn't need browser-support for overflow off the top of an element)

The attachment does seem to work around the issue, thanks!

>    .message {
>      border: 1px dotted purple;
>    }

This class probably needs `flex-shrink: 0` so that messages don't get squashed.

Comment 12

[Daniel Holbert [:dholbert]]

(In reply to Jan Keromnes [:janx] from comment #11)
> The attachment does seem to work around the issue, thanks!

hooray!

> This class probably needs `flex-shrink: 0` so that messages don't get
> squashed.

(I don't see any squashing here, but that may be font-size-dependent. Anyway, I agree that change would make sense in a real-world testcase. (or better: use "flex:none", which is functionally equivalent to flex-shrink:0; flex-basis: [unset]; flex-grow: [unset]; and is more human-readable.)

Comment 13

[Daniel Holbert [:dholbert]]

As noted in the "Overflow Issue" section of [1], there's a proposal to address this in the CSS-align spec[2].

[1] http://lists.w3.org/Archives/Public/www-style/2014Aug/0321.html
[2] http://dev.w3.org/csswg/css-align/#overflow-scroll-position

Comment 15

[Daniel Holbert [:dholbert]]

(...and per http://lists.w3.org/Archives/Public/www-style/2014Sep/0187.html , the WG has now accepted the proposal.)

Comment 16

[chenglou92]

Is there any progress regarding this? The `justify-content` solution won't work if you resize the browser vertically; the messages don't stick to the bottom.

Comment 17

[Daniel Holbert [:dholbert]]

Not currently.

Comment 18

[Kagami [:saschanaz] (they/them)]

Attachment: Another test case. Scroll position should not be changed here without user control.

Comment 20

[Daniel Holbert [:dholbert]]

Broadening this bug's summary slightly, to include the scenario from bug 962501, since it's all basically the same issue, given the way this ended up being specced (per www-style post in comment 15).

Comment 21

[padsterpat]

Attachment: less-js test case

I think I've just run into this myself - if it helps, I've attached my repo case too; I was trying to use as little JS as possible, just detecting when you're at the bottom or not.

In particular, note that (column-reverse, flex-start) on the viewport does work in chrome, but not (column, flex-end). Neither work in FF unfortunately, the former seems to be due to this bug.

Comment 23

[dgcoffman]

position: absolute; bottom: 0; has the same bug.

Comment 26

[gthomazeau]

Same problem here :
colum, row and row-reverse work correctly but not column-reverse
http://jsfiddle.net/qfe7ur1o/13/

Comment 28

[mees]

Hello,

Considering this bug-report has been sitting idle for a year and the general consensus is that the scrolling behavior of Chrome is the expected result, I was wondering if

Comment 29

[mees]

there is any indication on whether this issue will be resolved in the near future.


(Apologies for the break between messages)

Comment 30

[Daniel Holbert [:dholbert]]

It's not a top priority at the moment -- probably not destined to be resolved in the immediate future.

Comment 31

[Daniel Holbert [:dholbert]]

[tagging as a spec change around flexbox, though, per comment 15/comment 16, to be sure it's tracked alongside other such bugs]

Comment 32

[becca]

Hi, I just wanted to add my +1 to this bug report. Here is an example of the chat window I am trying to build, which ideally should show the bottom messages by default, but allow the user to scroll to the top of the conversation. I am having difficulty finding non-javascript workarounds, and would appreciate any updates. https://codepen.io/beccanelson/pen/jeBwgW

Comment 33

[castner.rr]

Another +1 to this bug report. The entire use-case of `overflow-y: scroll` and `flex: column-reverse` seems to be to enable chat windows, and firefox's implementation doesn't allow that to happen.

Comment 34

[Daniel Holbert [:dholbert]]

Ran into this on nytimes.com today, at the "Live Updates from our Reporters" section on this page:
 https://www.nytimes.com/interactive/2018/11/06/us/elections/results-house-forecast.html

The .eln-cards element there has
  display: flex;
  overflow-x: scroll;
  flex-direction: row-reverse;
...with the ~tweet-sized "cards" overflowing off the left side of the container and being unscrollable (in Firefox) due to this bug.

Comment 35

[edi]

Well, it seems like there are years now since this bug has been around, not sure whether this will ever be solved ... I am having the same issue I had 2 years ago, thought I'd just drop it.

Again, this behaviour seems to be the go-to for chat-like applications, where you don't have to programatically scroll the user's window downwards, all the time new content is added, or a simple image loads and extends it's own height.

Everytime content's height changes, it get's broken, and we need to somehow fix that from JS. Having this in place, does it automatically, and gets us rid of that issue.

Seems like Chrome is the only browser so far implementing this correctly, guess I'll just have to limit our users to using that instead.

Comment 36

[khomenkoigor]

@edi
you are right, Chrome is the only one who does it well.
It also works on Safari, but with a small issue with negative scroll offset value, but we can live with it.
That is a strange thing with FF, 5 years left and still not resolved..

Comment 38

[jade]

+1 to this bug for me as well. Our chat app is not scrolling to the bottom in FF when there are new messages using overflow: auto and flex: column-reverse. Chrome and Safari works.

Comment 39

[Max Stoiber (@mxstbr)]

This is our exact use case too, a chat app. I was very excited when this worked in Chrome (no JS chat app scroll?! Yay!), and very sad when I tried it in Firefox.

Comment 40

[u640887]

Same here, chat app and this is still an issue.

Comment 41

[Miguel]

Hey, this is still an issue. I'm pinging this in the hopes that someone will see it an be assigned to this bug.

Comment 42

[edi]

(In reply to Miguel from comment #41)

Hey, this is still an issue. I'm pinging this in the hopes that someone will see it an be assigned to this bug.

It's been more than 5 years now. I doubt Firefox is going to fix this anytime soon. As an workaround, just keep your content scrolled with plugins like stay-scrolled or if you use stuff like React ( on image / content load, just call scrollToBottom, there are tons of examples on SO ).

I've decided to replace this even if it's working just fine on Chrone, Opera, and Edge (DEV).

Comment 43

[u635235]

This bug has been causing problems for a lot of people. Any plans to fix it anytime soon? Why has the bug been lying around for ~5yrs?

Comment 44

[Podge H]

Support has changed in MDN docs for flex-direction. Suport went from fully supported to partially support due to this issue. Can FireFox please fix this issue?

Comment 45

[JP]

Hey, I'd love to help out with this, can anyone point me to relevant code?

Cheers,
Jon

Comment 46

[Emilio Cobos Álvarez (:emilio)]

Daniel is on parental leave, but I can try to help.

The relevant code for the flex layout algorithm is nsFlexContainerFrame.cpp. I'd probably start looking at users of mFlexDirection in layout/, then their callers, etc.

The scrollability of a scroll container is determined in nsHTMLScrollFrame::ReflowScrolledFrame.

Sorry I can't point to exact steps to fix this (as I don't know where the bug is myself), but the above would be the first places where I'd look at.

Also good for debugging are ./mach run -layoutdebug (which allows you to dump the layout tree easily) and rr.

Comment 47

[JP]

Thanks very much! (In reply to Emilio Cobos Álvarez (:emilio) from comment #46)

Daniel is on parental leave, but I can try to help.

The relevant code for the flex layout algorithm is nsFlexContainerFrame.cpp. I'd probably start looking at users of mFlexDirection in layout/, then their callers, etc.

The scrollability of a scroll container is determined in nsHTMLScrollFrame::ReflowScrolledFrame.

Sorry I can't point to exact steps to fix this (as I don't know where the bug is myself), but the above would be the first places where I'd look at.

Also good for debugging are ./mach run -layoutdebug (which allows you to dump the layout tree easily) and rr.

Comment 48

[austin.barnes04]

+1 for this bug, please fix!

Comment 49

[Sean Voisen (:svoisen)]

Noted. It's on our shortlist of bugs to consider this year already. That's all we can promise right now. Voting up the bug does help.

Comment 51

[brose]

Please Fix this. It would be extremely helpful for us. Thanks

Comment 52

[u659096]

(In reply to Sean Voisen (:svoisen) from comment #49)

Noted. It's on our shortlist of bugs to consider this year already. That's all we can promise right now. Voting up the bug does help.

Very happy to read it’s on the shortlist. Anyone concerned, please take above message serious and upvote the bug.

Looking forward to this bug being fixed.

Comment 53

[joshua.r.osullivan]

I've been working on a chat client, too.

Our solution has (rather humorously) been to make some creative use of transform: scaleY(-1).

...yeah.

Comment 55

[candide3]

At least for the use case of chat windows, this bug seems to admit an easy workaround: just wrap the misbehaving flex div in an extra non-flex div. Example adapted from working fix:

(function scrollify() {
  setTimeout(scrollify, 5000) ;
  for (const chatbox of document.getElementsByClassName("overflow-y-scroll flex flex-column-reverse ..."))
  {
    const chatwrap = document.createElement("div") ;
    chatwrap.style.overflowY = "scroll" ;
    chatwrap.style.height    = "100%" ;
    chatbox.style.removeProperty("height") ;
    chatbox.classList.remove("overflow-y-scroll") ;
    chatbox.parentNode.insertBefore(chatwrap, chatbox) ;
    chatwrap.appendChild(chatbox) ;
    chatwrap.scrollTop = 0x7fffffff ;
  }
})()

Feel free to use in polyfills or whatever.

Comment 56

[maggus.staab]

This seems also related for a problem (including a demo) mentioned in https://twitter.com/jaffathecake/status/1250761594612723713

Comment 57

[David Baron :dbaron:]

The central piece of the code that's relevant to this is probably ScrollFrameHelper::GetScrolledRect, particularly near the end where it branches on GetScrolledFrameDir() to handle scroll ranges for right-to-left (RTL) direction pages (e.g., Arabic, Hebrew).

However, there are likely a bunch of interesting side-effects of changing that, such as:

• we need to make sure the various scrolling-related APIs are compatible with other implementations (e.g., Element::scrollTo, Element.scrollTop, Element.scrollLeft) in terms of what point is considered 0 (that is, is it the left/top edge or is it the initial position?). (We might have incompatibility with other browsers for the RTL case here today.)
• there might be some pieces of code that make assumptions about not scrolling above the initial position, or the initial scroll position being 0, or the vertical scroll position never being negative

The underlying design question around a bunch of these differences is that it's not clear whether to model this as the initial scroll position being 0 and the rest of the range being negative (which I believe is how we model it for RTL in Gecko), or whether to model it as the scroll range all being positive positions but the initial scroll position being nonzero. So it's worth looking into what other browsers expose both for the flex case and for the RTL case.

Comment 58

[Sean Voisen (:svoisen)]

Now that we triage by severity, setting this bug's priority to P2 to represent near-term backlog status. See https://wiki.mozilla.org/Platform/Layout#Backlog_Tracking_in_Bugzilla

Comment 59

[dimafet001]

Same problem. I use column-reverse for my chat. No way to go around it with no js and Firefox is the only browser that has doesn't work correctly. I think in 2020 many chat app developers would highly appreciate the fix!

Comment 60

[Bendik]

I totally agree with you. I'm really impressed with how poorly this ticket has been prioritized. Sure you can just throw more javascript at the problem, but when you're the only browser with this problem you're creating extra work for so many developers and discrediting yourself. Can someone explain how new CSS4 features prioritized higher than actually getting yee-old flexbox right? I'm outraged.

Comment 61

[tombonaventure]

Please fix this.

Comment 62

[Kirill Lyubimov]

Same problem. Column-reverse break scroll.

Comment 63

[Ting-Yu Lin [:TYLin] (UTC-8)]

Attachment: Bug 1042151 Part 1 - Sort the #include statements in nsLayoutUtils.cpp.

Comment 64

[Ting-Yu Lin [:TYLin] (UTC-8)]

Attachment: Bug 1042151 Part 2 - Refactor the conditions that decide which edge to clamp when getting scrolled rect.

Depends on D86075

Comment 65

[Ting-Yu Lin [:TYLin] (UTC-8)]

Attachment: Bug 1042151 Part 3 - Extract a helper class containing flexbox's axis information.

This struct is going to be used in the next part in nsLayoutUtils::GetScrolledRect().

Depends on D86076

Comment 66

[Ting-Yu Lin [:TYLin] (UTC-8)]

Attachment: Bug 1042151 Part 4 - Consider flexbox's main & cross axis when getting scrolled rect.

Depends on D86077

Comment 67

[Ting-Yu Lin [:TYLin] (UTC-8)]

Attachment: Bug 1042151 Part 5 - Add a reftest.

scrollbar-no-margin.html is adapted from scrollbar.html with margin
removed from flex-items, to workaround bug 1527539.

font-size: 0 added to .flex is to workaround bug 1302700. Also,
adjust scrollbar-width and scrollbar-color to avoid fuzzy rendering
on rounded-corner slider on some platforms.

Depends on D86078

Comment 68

[Ting-Yu Lin [:TYLin] (UTC-8)]

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

dbaron suggests the might behavior might fall out naturally from
nsLayoutUtils::GetScrolledRect, if we tweak that function to be aware of
flexbox main/cross axes.

Yes, it's sufficient to tweak nsLayoutUtils::GetScrolledRect to let it aware of flexbox's main and cross axes so that it clips the correct physical side of the scrollable rect.

(In reply to David Baron :dbaron: 🏴󠁵󠁳󠁣󠁡󠁿 ⌚UTC-8 from comment #57)

The underlying design question around a bunch of these differences is that it's not clear whether to model this as the initial scroll position being 0 and the rest of the range being negative (which I believe is how we model it for RTL in Gecko), or whether to model it as the scroll range all being positive positions but the initial scroll position being nonzero. So it's worth looking into what other browsers expose both for the flex case and for the RTL case.

In our current implementation regarding various writing-mode & RTL combination, the initial scroll position is always 0, and the rest of the range is negative if the physical scrolling direction is going from right-to-left or bottom-to-top. I believe this is consistent with Google Chrome.

Comment 69

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1eac44fb3f6f
Part 1 - Sort the #include statements in nsLayoutUtils.cpp. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/ad498cd179b9
Part 2 - Refactor the conditions that decide which edge to clamp when getting scrolled rect. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/bf4991db4445
Part 3 - Extract a helper class containing flexbox's axis information. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/14e34cd9dfd8
Part 4 - Consider flexbox's main & cross axis when getting scrolled rect. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/a9a3507991fa
Part 5 - Add a reftest. r=dholbert

Comment 70

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

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

Comment 71

[Cristian Brindusan [:cbrindusan]]

Backed out 5 changesets (bug 1042151) for causing wpt failures in negative-overflow.html.
https://hg.mozilla.org/integration/autoland/rev/b5a790f3355971fa0716f00cfeefb949bb8525a6

Push with failures:https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&revision=a9a3507991faf61868562087c72080baa62e53e2&selectedTaskRun=biwmWMIkTiqHL_92_E1UWw.0

Failure log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=312337424&repo=autoland&lineNumber=6860

Comment 72

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

Upstream PR merged by moz-wptsync-bot

Comment 73

[Ting-Yu Lin [:TYLin] (UTC-8)]

Attachment: flex-wrap-reverse-overflow.html

The negative-overflow.html failures demonstrate the testcase attached here, where reversing the cross-axis (flex-wrap:wrap-reverse) can create overflow. My proposed patches allow the overflow to be scrollable. For now, the testcase doesn't work in Google Chrome 86. I guess Google Chrome only tweaks the scroll area when the main-axis is reversed, but not the cross-axis. I don't see any reason to treat main and cross different in terms of scrollability.

We already have existing failures in negative-overflow.html tracked in bug 1626108, so I'm going to adjust the test expectation in Part 4, and reland my patches. We can do follow-up there and reason the correctness of the testcase.

Daniel, does the plan sound good to you?

Comment 74

[Daniel Holbert [:dholbert]]

(In reply to Ting-Yu Lin [:TYLin] (UTC-7) from comment #73)

I don't see any reason to treat main and cross different in terms of scrollability.

Agreed. Mind filing a Chrome bug on this? (at https://crbug.com/ )

I'm going to adjust the test expectation in Part 4, and reland my patches. We can do follow-up there and reason the correctness of the testcase.

Daniel, does the plan sound good to you?

Works for me. Thanks!

Comment 75

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f9d4918600d7
Part 1 - Sort the #include statements in nsLayoutUtils.cpp. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/5558e9475e40
Part 2 - Refactor the conditions that decide which edge to clamp when getting scrolled rect. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/9dd61fc2cbbd
Part 3 - Extract a helper class containing flexbox's axis information. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/d4f4e419c975
Part 4 - Consider flexbox's main & cross axis when getting scrolled rect. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/56299d5ad3f6
Part 5 - Add a reftest. r=dholbert

Comment 76

[Ting-Yu Lin [:TYLin] (UTC-8)]

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

Agreed. Mind filing a Chrome bug on this? (at https://crbug.com/ )

Filed https://bugs.chromium.org/p/chromium/issues/detail?id=1114306

Comment 77

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/f9d4918600d7
https://hg.mozilla.org/mozilla-central/rev/5558e9475e40
https://hg.mozilla.org/mozilla-central/rev/9dd61fc2cbbd
https://hg.mozilla.org/mozilla-central/rev/d4f4e419c975
https://hg.mozilla.org/mozilla-central/rev/56299d5ad3f6

Comment 78

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

The patch landed in nightly and beta is affected.
:TYLin, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 79

[Ting-Yu Lin [:TYLin] (UTC-8)]

This bug is not trivial and depends on the clean-up patches in bug 1657540.

Comment 80

[raviqqe]

Thank you so much, everyone! My web app finally works well on Firefox 81!!!

Comment 81

[Stefan Sechelmann]

Thank you!

Comment 82

[Derick]

version 78.7.0esr (64-bit) still remains with the bug.

Comment 83

[Ryan VanderMeulen [:RyanVM]]

ESR78 will not be receiving this fix.

Comment 84

[Derick]

Thank you for letting me know.