Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scrolling: Ability to scrollback can get stuck #18325

Closed
Tracked by #23539 ...
ara4n opened this issue Aug 1, 2021 · 16 comments
Closed
Tracked by #23539 ...

Scrolling: Ability to scrollback can get stuck #18325

ara4n opened this issue Aug 1, 2021 · 16 comments
Labels
A-E2EE A-Timeline O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Major Severely degrades major functionality or product features, with no satisfactory workaround Team: App X-Regression Z-Chronic

Comments

@ara4n
Copy link
Member

ara4n commented Aug 1, 2021

In an encrypted room, it's possible for scrollback to stop loading, despite you not being at the beginning of the room. A workaround is to click on the timestamp of a message towards the top of your timeline in order to permalink it, which causes it start back-paginating again. However, it obviously shouldn't stop refusing to scrollback in the first place.

@SimonBrandner SimonBrandner added A-E2EE A-Timeline P1 S-Major Severely degrades major functionality or product features, with no satisfactory workaround X-Regression X-Release-Blocker labels Aug 2, 2021
@SimonBrandner
Copy link
Contributor

I don't think I am able to repro. Do you have a repro case? Have you seen this before or is this a regression?

@SimonBrandner SimonBrandner added X-Cannot-Reproduce X-Needs-Info This issue is blocked awaiting information from the reporter and removed X-Regression X-Release-Blocker labels Aug 2, 2021
@ara4n
Copy link
Member Author

ara4n commented Aug 2, 2021

i've seen it a few times before; i think it is a regression (but could have regressed months ago). will try to repro.

@ara4n
Copy link
Member Author

ara4n commented Aug 2, 2021

stuck.scrollback.mov

video of me reproing it. i'm afraid it's in an internal room, so i've had to crop the video comically, but suffice it to say that i'm leaning on page-up in order to rapidly backpaginate, and then when it gets stopped, i have to click on the permalink for the first message in the room for it to let me continue backpaginating again. will rageshake too.

@ara4n
Copy link
Member Author

ara4n commented Aug 2, 2021

n.b. there is no spinner when it gets stuck. you just have a big hunk of vertical blank margin which you can't scroll beyond.

@SimonBrandner
Copy link
Contributor

I wonder, this only happens in this room or also other ones? I attempted to repro in a few rooms but I didn't manage to do it

@novocaine novocaine added the O-Uncommon Most users are unlikely to come across this or unexpected workflow label Aug 4, 2021
@ara4n
Copy link
Member Author

ara4n commented Sep 23, 2021

have seen it in many rooms; basically reproducable in any encrypted room if you start scrolling back aggressively.

@ghost
Copy link

ghost commented Nov 22, 2021

Also happens on desktop 1.9.5 (happened in prior versions as well), same problem as stuck.scrollback.mov, one interesting thing is if i manage to get to an older message using search, while scrolling down it will skip the section that was stuck entirely, messages left there are completely unreadable. This does not happen on the android app and the room history is readable as expected.

@kittykat
Copy link
Contributor

kittykat commented Dec 8, 2021

I've seen this happen recently on develop.element.io

@waclaw66
Copy link
Contributor

Same on my side...

capture.mp4

@waclaw66
Copy link
Contributor

It stops loading the history before these events, maybe there is some coincidence...

obrazek

@ara4n
Copy link
Member Author

ara4n commented Sep 17, 2022

I just repro'd this again, simply by hammering page-up in #NVI. It had a "you do not have permission to view encrypted messages before this point." warning at the top of the page. clicking a permalink unstuck it.

@bradtgmurray
Copy link

bradtgmurray commented Sep 30, 2022

I think I figured out what this is:

When you're paginating back through a room and you hit a reply event that's outside of your current timeline set, the client will fetch the replied to event with the /context endpoint with lazy_load_members: true. If the resulting response does not overlap with any of the existing timelines in the timelineSet of the room, it's added to the room as a new timeline in the set, with the state initialized from the context response: https://github.com/matrix-org/matrix-js-sdk/blob/develop/src/client.ts#L5433

Then, if you continue to paginate back in the room, eventually you'll have a /messages response that overlaps with the old /context response, and the timelines will be linked together. However, the timeline created from the /context response will still just have the partial room state returned due to the lazy_load_members: true filter.

This becomes a problem when we call checkForPreJoinUISI in the TimelinePanel: https://github.com/matrix-org/matrix-react-sdk/blob/ff59f68a9fa0ac5bc467afe0e8d9af02d15e1666/src/components/structures/TimelinePanel.tsx#L1504

This function scans through the timline and tries to find when the current logged in user first joined the room by looking for the absense of a join event in the timeline. If we find a decryption error in the timeline before we have a membership for ourself in the timeline, we assume that this means that we're about to show the user a wall of decryption errors, as we've paginated into a portion of the rooom history that pre-dated us joining the room. We artificially stop paginating the room at this point, and only show events after we've "joined" the room.

The problem is, this timeline created by the /context endpoint doesn't necessarily have all the members in the room, and if you didn't say anything during this period, the /context endpoint won't include your membership event. This means that if there's a decryption error around this point, and we didn't say anything in the room around this point, checkForPreJoinUISI will wrongly calculate that you're not in the room at this point and stop pagination at this point.

The fix for this isn't super clear to me, hence the huge comment and not a PR.

We could:

  1. Try to fix up the timeline state when first creating a timeline from the /context endpoint by populating all the members from somewhere, but this seems to defeat the purpose of lazy_load_members
  2. Try to fix up the timeline state when we join the timeline created from the /messages endpoint with the timeline of the /context endpoint, but this seems pretty error prone to merge the state of the timeline from two sources
  3. Don't join timelines from different "sources" (/messages vs /context) together, and if you hit an overlap throw one of them out? Not sure how messy this is going to end up, sounds potentially weird to throw away timelines?
  4. Add some kind of property to timelines to indicate their states have lazy loaded members and shouldn't be trusted? This seems promising...
  5. Alter checkForPreJoinUISI to not use the state events in the timeline to figure out who's in the room, but this seems like it's just covering up that we don't know if timeline's have correct states or not.

Other ideas? Especially from folks that are more familiar with the timeline code than I?

matrix-org/matrix-react-sdk#3881 is the PR that introduced this pagination handling in encrypted rooms, for context.

@ara4n
Copy link
Member Author

ara4n commented Oct 2, 2022

@bradtgmurray thanks for tracking this one down :) My bias would be towards option 3 - the timeline created by /context in order to view a reply could definitely be thrown away in favour of the more detailed timeline from /messages when it's returned. we could flag the /context timeline as being dispensable (and not to bother trying to merge with it).

alternatively, option 4 could work too, and have the checkForPreJoinUISI just ignore timelines where the members haven't been loaded.

@Johennes Johennes changed the title Ability to scrollback can get stuck Scrolling: Ability to scrollback can get stuck Jan 30, 2023
@ara4n
Copy link
Member Author

ara4n commented Feb 15, 2023

I just got bitten by this on today's nightly.

@kegsay
Copy link
Contributor

kegsay commented Apr 12, 2024

I think @pmaier1 hit this today. Hard to know for sure though, but he failed to scrollback far enough in an encrypted room stating:

my EW blanks out in scrollback when I reach a certain point in time yesterday

@t3chguy
Copy link
Member

t3chguy commented May 10, 2024

Fixed by matrix-org/matrix-react-sdk#12464 which removes the checkForPreJoinUISI code

@t3chguy t3chguy closed this as completed May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-E2EE A-Timeline O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Major Severely degrades major functionality or product features, with no satisfactory workaround Team: App X-Regression Z-Chronic
Projects
None yet
Development

No branches or pull requests