Closed Bug 1042151 Opened 7 years ago Closed 6 months ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
81 Branch
Tracking Status
firefox70 --- wontfix
firefox71 --- wontfix
firefox72 --- wontfix
firefox73 --- wontfix
firefox74 --- wontfix
firefox75 --- wontfix
firefox76 --- wontfix
firefox77 --- wontfix
firefox78 --- wontfix
firefox79 --- wontfix
firefox80 --- wontfix
firefox81 --- fixed

People

(Reporter: rik, Assigned: TYLin)

References

(Blocks 1 open bug, )

Details

(Whiteboard: [layout:backlog:quality][layout:backlog:2020], [wptsync upstream])

Attachments

(11 files)

618 bytes, text/html
Details
545 bytes, text/html
Details
1.45 KB, text/html
Details
949 bytes, text/html
Details
1.26 KB, text/html
Details
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
1.04 KB, text/html
Details
See attached testcase.

The two boxes are identical except the first one has a flex-direction: column, the second one column-reverse.
Version: unspecified → Trunk
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.
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.
(Also: FWIW, IE11 renders the initial testcase here the same way that we do.)
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.
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.
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.
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)
Attachment #8461267 - Attachment description: (demo IRC client that uses "flex-end" & doesn't rely on this) → (mock-up IRC client that uses "flex-end" & doesn't need browser-support for overflow off the top of an element)
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
*the right behavior
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.
(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.)
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
Duplicate of this bug: 1063967
(...and per http://lists.w3.org/Archives/Public/www-style/2014Sep/0187.html , the WG has now accepted the proposal.)
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.
Not currently.
Attachment #8680682 - Attachment filename: file_1042151.txt → file_1042151.html
Attachment #8680682 - Attachment mime type: text/plain → text/html
Attachment #8680682 - Attachment filename: file_1042151.html → file_1042151.txt
Attachment #8680682 - Attachment filename: file_1042151.txt → file_1042151.html
Duplicate of this bug: 962501
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).
Summary: flex-direction: column-reverse with overflow-y: auto is not scrollable → flex-direction: column-reverse (or "flex-direction:column; justify-content:flex-end") with overflow-y: auto is not scrollable
Attached file 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.
Duplicate of this bug: 1322236
Summary: flex-direction: column-reverse (or "flex-direction:column; justify-content:flex-end") with overflow-y: auto is not scrollable → Content that overflows off the "start" side of a "flex-direction: *-reverse" flex container is not scrollable (but should be)
position: absolute; bottom: 0; has the same bug.
Duplicate of this bug: 1331754
Duplicate of this bug: 1402184
Same problem here :
colum, row and row-reverse work correctly but not column-reverse
http://jsfiddle.net/qfe7ur1o/13/
Duplicate of this bug: 1495157
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
there is any indication on whether this issue will be resolved in the near future.


(Apologies for the break between messages)
Component: Layout → Layout: Flexbox
It's not a top priority at the moment -- probably not destined to be resolved in the immediate future.
[tagging as a spec change around flexbox, though, per comment 15/comment 16, to be sure it's tracked alongside other such bugs]
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
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.
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.
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.
@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..
Duplicate of this bug: 1531233

+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.

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.

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

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

(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).

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?

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?

Flags: needinfo?(svoisen)
Flags: needinfo?(svoisen)
Whiteboard: [layout:backlog]

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

Cheers,
Jon

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.

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.

Whiteboard: [layout:backlog] → [layout:backlog:quality]
Flags: needinfo?(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.

Flags: needinfo?(svoisen)
Priority: -- → P3
Whiteboard: [layout:backlog:quality] → [layout:backlog:quality][layout:backlog:2020]
Duplicate of this bug: 1619042

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

(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.

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.

Blocks: 1625674
No longer blocks: 1625674
Duplicate of this bug: 1625674

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.

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

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.

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

Priority: P3 → P2

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!

Please fix this.

Same problem. Column-reverse break scroll.

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

Depends on D86076

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

Assignee: nobody → aethanyc
Severity: normal → S3
Status: NEW → ASSIGNED

(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.

Attachment #9168283 - Attachment description: Bug 1042151 Part 3 - Extract a helper struct containing flexbox's axis information. → Bug 1042151 Part 3 - Extract a helper class containing flexbox's axis information.
Pushed by aethanyc@gmail.com:
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
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/24908 for changes under testing/web-platform/tests
Whiteboard: [layout:backlog:quality][layout:backlog:2020] → [layout:backlog:quality][layout:backlog:2020], [wptsync upstream]
Upstream PR merged by moz-wptsync-bot

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?

Flags: needinfo?(aethanyc) → needinfo?(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!

Flags: needinfo?(dholbert)
Pushed by aethanyc@gmail.com:
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

(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

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.

Flags: needinfo?(aethanyc)

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

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

Thank you!

You need to log in before you can comment on or make changes to this bug.