element-hq / element-web

A glossy Matrix collaboration client for the web.
https://element.io
Apache License 2.0
10.74k stars 1.89k forks source link

Scrolling: Improving jumps on timeline pagination/scrolling #8565

Open bwindels opened 5 years ago

bwindels commented 5 years ago

Matthew's summary of the problems: https://matrix.to/#/!maVEfftuKxhnPMcckU:matrix.org/$15500048623049743lQqLW:matrix.org?via=matrix.org&via=t2l.io&via=lant.uk

Related bugs:

Update 19/3/2019

Going with a new algorithm, dubbed BACAT (Bottom-Aligned, Clipped-At-Top) scrolling, prototyped here: https://gist.github.com/bwindels/b3a3ba3fa3792f23962d894866fa7b0f

The motivation for the changes is having noticed that setting scrollTop while scrolling tends not work well on at least Chrome on macOs. Also see https://github.com/vector-im/riot-web/issues/528. The approach taken instead manually sets the height of the timeline in 200px increments, with the timeline being aligned to the bottom of the container element. The relatively tiny 200px height comes from the fact that we don't want the user to ever scroll off into pages of whitespace and we have no way of preventing overscroll, so by keeping it to 200px and putting a spinner 100px up from the top of the page, we avoid death by whitespace.

By changing the height of the container, we can compensate for anything that grew below the viewport to maintain what's currently visible in the viewport without jumping. For anything above the viewport growing or shrinking, we don't need to do anything as the timeline is bottom-aligned. We do need to update the height manually. To maintain scroll position after the portion above the viewport changes height, we need to set the scrollTop, as we cannot balance it out with more height changes. We do this 100ms after the user has stopped scrolling, so setting scrollTop has not nasty side-effects.

Obsolete now: Update 5/3/2019:

The resize observer approach doesn't work reliably as it can race with the scroll event, so looks like manually restoring the scroll position from event tile when it can cause a jump is the better option. These things can cause jumps:

bwindels commented 5 years ago

One possible approach to have the timeline prevent jumping in a more reliable way I was thinking of is the following:

We observe the <ol> in the timeline that contains all the tiles, typing notif with a ResizeObserver. It is supported in Chrome, supposed to be supported by Firefox in the nearish future, and has a polyfill available based on mutation events (which we already use elsewhere in Riot). This will give us a JS hook when the size of the timeline changes, so handling anything that could resize the timeline (thumbnails loading, event being decrypted, pagination, ...).

On scroll, we also keep track of which event tile is the last one that is still in view and calculate the offset to the bottom of the viewport.

When the timeline resizes (through the ResizeObserver), we adjust the container scrollTop, so that the offset of the last event tile in view to the bottom of the viewport stays the same.

One added benefit is that it would make it easier to switch to native scrollbars for the timeline, because the timeline does need to be made aware of size changes (which are now tracked through an overlaying iframe with gemini scrollbars), so doing it with a standardized API that will hopefully become widely supported and has a polyfill that from the looks of it should be better performing (mutation events) than the gemini iframe approach seems like a good thing.

bwindels commented 5 years ago

Prototype at https://gist.github.com/bwindels/4aa3a0495264498cca67b8563b580889

bwindels commented 5 years ago

Also try to fix https://github.com/vector-im/riot-web/issues/8792

bwindels commented 5 years ago

Also looking at https://github.com/vector-im/riot-web/issues/8653 as part of this

bwindels commented 5 years ago

Also https://github.com/vector-im/riot-web/issues/8455

ara4n commented 5 years ago

This is definitely a big improvement, but it's worth noting that it can still jump pretty badly on E2E rooms, especially if they contain code blocks. Theory is that if the JS falls behind the DOM events significantly due to GCs, things get out of sync and we still see jumps - so the more work JS is doing, the more likelihood of seeing a jump.

I was able to reproduce this pretty well during the security breach of Apr 11 when trying to rapidly scroll the E2E rooms we were using to coordinate response, which had lots of code blocks as people shared details.

ara4n commented 4 years ago

(It's also possible that the bad GCs triggered by https://github.com/vector-im/riot-web/issues/11267 might make this worse).

separately, @madduck is seeing something that looks like a really bad instance of this (or a new regression in FF72 on Sid?) over at https://scratch.madduck.net/riot-web-scroll-jumping-around-screencast-by-martin.mp4

madduck commented 4 years ago

@ara4n correction, its FF 70. And I do have lots of extensions preventing things, like umatrix etc.

madduck commented 4 years ago

Any news on this? At the moment, I am ashamed to demo Riot to others because it's just unusable, not to mention that I'd like to search history myself every now and then…

madduck commented 4 years ago

I just tried this on FF71 without any plugins and while it's not as bad there, it still jumps across the place too often.

ara4n commented 4 years ago

@madduck is there any chance of a screencapture showing what the jumps look like? even better if you can DM me FF profiler output too to try to correlate. what spec machine are you on? I haven't seen it at all recently on my massive accounts (but then again i'm on a top end MBP)

madduck commented 4 years ago

@ara4n I'll gladly provide that… the next time I encounter the problem. I just saw it again a few days ago, but of course now that I set up the screen recorder and profiler, scrolling works just fine and there is nothing to show or correlate. #heisenbug.

ara4n commented 4 years ago

Lots of reports from mozilla of this being much worse than it should be:

jld When I'm scrolling through a channel in riot-web, sometimes the scroll position will jump back in the opposite direction for no obvious reason, which makes it hard to follow higher-traffic channels (like #developers:mozilla.org). Known bug?

B.J. Herbison jld , how are you scrolling? If it is by holding the left button and dragging the scroll bar then the jumps are because more content is loaded so the scroll bar is reevaluated. That's how I would prefer to scroll and find it very annoying.

jld This was while scrolling with the scroll wheel, so that shouldn't be affected by loading in more messages off-screen. But I've noticed that things like link previews seem to be dynamically inserted, so maybe there's something not quite right about how it compensates for that.

https://matrix.to/#/!pcfWjiETvnVuspPLPl:mozilla.org/$JfS6yiJd9u8LYsCGlEA_yNnbbP0VBh6WJYPReSXoDxc?via=mozilla.org&via=matrix.org&via=maunium.net

turt2live commented 4 years ago

@bwindels I assume you're not still looking at this from last year. If this is wrong, feel free to take it.

bwindels commented 4 years ago

Hitting Page Up apparently causes it to jump around a bunch

madduck commented 4 years ago

This is really becoming a problem in a community I'm trying to convert to Matrix. People are complaining, which is really harming adoption. Any ETA on having this regression fixed?

turt2live commented 4 years ago

@madduck the latest PR was merged 9 hours previous to your comment. Please try riot.im/develop and if you find a problem case, document it here.

bwindels commented 4 years ago

Indeed, I landed a potential improvement to prevent scroll jumps yesterday. Would be great to have feedback (especially for people on macOS) if you notice any difference.

@madduck I assume you are on macOS?

bwindels commented 4 years ago

https://github.com/matrix-org/matrix-react-sdk/pull/4166 seems to have improved things, with jumps occuring less frequently. At least Matthew (on FF, mac) and Neil Alexander (on Safari) are still seeing jumps after back-pagination comes in though, although less frequently than before.

madduck commented 4 years ago

@bwindels no, I am on FF/Linux, and if matrix-org/matrix-react-sdk#4166 is already deployed on riot.im/develop. then it does not fix my problems.

bwindels commented 4 years ago

@bwindels no, I am on FF/Linux, and if matrix-org/matrix-react-sdk#4166 is already deployed on riot.im/develop. then it does not fix my problems.

Interesting @madduck, same config as I have. So no improvement at all?

madduck commented 4 years ago

@bwindels no improvements, no...

dkasak commented 4 years ago

Just wanted to reiterate that scrolling behaviour is still awful for me on Riot Desktop 1.6.0 on Arch Linux.

Apart from the jumping around, the worst thing is that the scrollbar grip (the thing you grab) seems to stick to the top of the scrollbar as new events are loaded. This results in continual pagination deeper and deeper into the history, with no easy way of stopping.

I think the proper behaviour would be for the grip to move downwards from the top after each batch of new events is loaded and then stay there, forcing the user to again move the grip upwards if he wants to scroll even deeper. The point is that each deeper fetch should necessitate explicit user action instead of being automatic. I think #8455 might be talking about this problem?

kegsay commented 4 years ago

This is an issue for me too on Mac (Riot Desktop v1.6.2). It's pretty frustrating. It has been this way for months.

claell commented 3 years ago

This happens to me alot when scrolling through the history of a room. Example of a room: #element-web:matrix.org

@bwindels since you are assigned to this issue and given the priority of it, what is the status? From the comments I read that it might be hard to reproduce?

On my setup, I can easily reproduce (also on different operating systems). I am open to invest some time to get required information in order to proceed with this. We could also try to arrange some virtual meeting, so you can reproduce it on my setup.

claell commented 3 years ago

This also happens on the Desktop app (tested on Linux). Using Element is extremely unsatisfying due to this issue.

Another ping @bwindels @turt2live @ara4n

My offer to help with debuggin this or whatever is needed (in case you cannot reproduce) still stands.

Also the mozilla label is a bit misleading, since it also happens on Firefox, but also on all other browsers, I guess.

turt2live commented 3 years ago

@claell do you have browser and OS information you can give us to investigate? We'd also need to know exactly what you're doing to have it happen so we can reproduce it. Screen recordings help a lot.

The mozilla label has nothing to do with Firefox or browsers - it just means that Mozilla, one of our customers, is having the issue too.

Not sure why I've been pinged on this in any case.

claell commented 3 years ago

@turt2live

@claell do you have browser and OS information you can give us to investigate? We'd also need to know exactly what you're doing to have it happen so we can reproduce it. Screen recordings help a lot.

Tested on Firefox (always used the currently most up to date version) on Windows and Manjaro (both the most current versions). Also happened to me with the element app on Manjaro, don't know about Windows. I also linked a room where this happens.

I basically just scroll throuh the backlog. So I for example go about 600 messages up (what arrived when I was last viewing) and then scroll down towards the most recent ones. While scrolling there are sometimes jumps (happens when scrolling with mouse wheel as well as when scrolling using the Page down key. This seem to happen when using the arrow to jump to the earliest unread message as well as when manually scrolling there before scrolling down. (I think this issue also appears when scrolling up, and not only when scrolling down, but I am not entirely sure about that).

Does that mean you didn't manage to reproduce the issue? If you can't, your system information might also be helpful.

I will see if I can do a screen recording. Before I'd like to get some reply first that somebody is currently actively working on this (or will be after posting the screen recording). Else I guess it won't be worth the effort if this issue is stalled for another half a year afterwards, since then the recording might be considered outdated by then.

The mozilla label has nothing to do with Firefox or browsers - it just means that Mozilla, one of our customers, is having the issue too.

Ah, got it.

Not sure why I've been pinged on this in any case.

I pinged you because you triaged this in the past. Since there has been no action in this issue from the assignee since about half a year and given the set priority and severity (!), I thought it is probably best to also ping people who triaged this in the past in case this is currently stalled.

pwinckles commented 3 years ago

@dkasak's description of the problem is exactly what I'm seeing. I came here to see if there were any open tickets about scroll jumping after finding it incredibly frustrating to attempt to browse the recent history in the big Matrix/Element rooms. Turns out that the problem was that I was using the scrollbar. After reading this issue, I tried using the page up and down keys, and it worked perfectly.

I had not tried to use the keys previously, because I first tried to use the up and down arrow keys, which is how I normally scroll through pages, and saw, to my dismay, that they did not work and so I incorrectly assumed that I had to use the scroll bar, which is completely unusable, without trying page up.

As an aside, I haven't looked if there's an issue open for this, but scroll jumping is also a problem on the "Explore rooms" page, and, unfortunately, only the scroll bar "works" there.

claell commented 3 years ago

@pwinckles Page up and Page down keys cause similar problems for me, but glad to hear that it works for you.

When you say "scroll bar" do you really mean that? Have you tried using the mouse wheel to scroll?

claell commented 3 years ago

Apparently I overeastimated the importance of this issue slightly given there currently are 251 (!) open issues with p1 and severity:major: https://github.com/vector-im/element-web/issues?page=1&q=is%3Aopen+label%3Ap1+label%3Aseverity%3Amajor.

This is a bit discouraging, still I hope that all those issues get much attention to solve them.

grahamperrin commented 3 years ago

Does major bug #3763 fall under the umbrella of this issue?

claell commented 3 years ago

@grahamperrin I had a short look. From what I understand, that issue is not related to this one, but might be wrong.

Tynach commented 3 years ago

I think I've figured out at least part of the problem. I was scrolling through a very active chat and trying to read every post sent since last night, and while it didn't only bug out at these times, it bugged out the most when I reached 12:59 AM. It tried to jump instantly to 12:13 PM the same day, instead of 1:00 AM.

I think something in the logic for finding posts is sorting human-readable timestamps alphabetically, when you shouldn't be using human-readable timestamps at all for this sort of operation. You should instead be using UNIX timestamps (at which point you can safely use alphabetical search just fine if you still store them as strings), and optimally storing them as actual numbers.

I've verified this happens in Element Desktop as well as Element Web in Firefox, so it's not a browser bug or extension causing interference.

claell commented 3 years ago

Interesting finding, that would be a pretty weird cause for this.

Tynach commented 3 years ago

On further reflection, it doesn't make all that much sense, and doesn't behave like that consistently.

I do think there's something very.. Wrong with how it stores information on the sequence of messages. Sometimes scrolling slowly will eventually lead me to a place where I can't scroll any further, but if I scroll fast it'll get me past that... And it'll jump hours or even days into the past/future (depending on if I'm scrolling up or down), and not give anything in between... But if I manage to somehow get to a post in between (like by following someone's reply to such a post), it works until I get to one of the 'jump posts' before, and then it stops working.

On some occasions, when I manage to get into an 'in between' post, it causes Element to take up 100% of the CPU for seemingly no reason, and scrolling is very choppy and slow. Get to one of the points where it'd normally not let me go past it, and that clears up... As soon as it's at a place where it again treats those 'endpoints' as brick walls that I can't scroll past.

I can excuse occasional small jumps while scrolling, which also happen.. After all, I understand that as I scroll, things are removed from one end of the page and added to the other end. It's not a trivial problem to solve, especially when you're limited to browser APIs.

But that's not the only thing that happens. Entire hours or days of chats are inaccessible except by pure luck or arcane tricks. Scrolling will occasionally force a page into an endless loop of loading page after page of chat posts in only a few seconds, I guess because the first segment coming in doesn't move the scrollbar up enough or something? I don't know exactly what triggers that. And just.. Overall, scrolling through large chats isn't just difficult, but many times it's impossible.

And that's not even counting how often Element - whether I scroll or not, but usually when I scroll or shortly after (maybe when it loads another batch of posts?) - just completely blanks out all the posts and shows nothing, just a scrollable empty page until I get to another point where it starts loading more posts.

This tells me that something in the very core of the design is fundamentally broken. If you have that many edge cases that you need to keep accounting for more and more and more, with that bad of behavior even after accounting for many of them... Then you need to stop trying to fix the edge cases, and instead wipe ALL of it from orbit and start over from scratch, because the very essence of the idea used from the very beginning was wrong.

grahamperrin commented 3 years ago

https://develop.element.io/ at the time of writing

From https://github.com/matrix-org/matrix-react-sdk/pull/5920#issuecomment-827148026:

Like, the lowest line of text becomes uppermost (or near uppermost) :ballot_box_with_check: momentarily, then a bounce back of around half a page, …

Today I'm getting this bounce, varying distances, with nearly every Page Down whilst trying to catch up in #freenode_#freebsd:matrix.org

https://matrix.to/#/!qZXgYtEIHagWHrEQQb:matrix.org/$1619805180386752XkYsC:matrix.org?via=matrix.org for example, here (selected) is what was previously the lowermost line:

image

I have smooth scrolling enabled in Firefox, the jumpiness is as if smooth scrolling is not enabled.

I ran a quick search for smooth scroll, found two (merged) PRs https://github.com/matrix-org/matrix-react-sdk/pull/2676 and https://github.com/matrix-org/matrix-react-sdk/pull/3051


… expanded my search and found myself here, #8565, am I in the right place?

If so: I doubt that https://github.com/matrix-org/matrix-react-sdk/pull/5920 has worsened the situation. More likely: I previously never noticed (or habitually ignored) things such as jumpiness because Page Down was so broken that I simply could not use Matrix/Riot/Element to catch up in some rooms.

Postscript

#freenode_#freebsd:matrix.org seemed far less prone to the jumpiness after I disabled experimental, compact IRC-style layout (preferring a "more compact Modern" layout).

After re-enabling the experimental layout:

2021-05-08 08:29:43

– I found things not jumpy :ballot_box_withcheck: – touch wood – in `#freenode#freebsd-bugs:matrix.org`. Maybe worth noting that in this room, most messages are relatively terse, for example:

2021-05-08 08:30:41

claell commented 2 years ago

I recently tested again and can still make element on the web jump (might have improved slightly, though).

claell commented 1 year ago

Just tested again with latest version (and in between as well, just didn't comment here). Right now, I can scroll up rather flawlessly in a chat (so going to older messages seems to work).

The problems occur reproducible every time when I scroll in the other direction again. So going up 10 days or something in a larger chat works. Scrolling back to day 9 or 8 from there gives me the jumps.

Windows 11, latest Element desktop client. Hopefully, this can get finally fixed some day ;)

gy-mate commented 3 weeks ago

Still an issue with the latest version on Safari 17.4.1.