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

Fix updating unread marker if not to latest chunk #5481

Merged
merged 3 commits into from
Apr 7, 2022

Conversation

SpiritCroc
Copy link
Contributor

@SpiritCroc SpiritCroc commented Mar 9, 2022

SetReadMarkerTask was not updating the read marker when both the old and
the new fully read eventId weren't in the last chunk, even when the new
one was after the first one.

Signed-off-by: Tobias Büttner [email protected]

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

Do not assume that the current read marker is in a later chunk then the new requested one, unless it is in the last chunk.

Tests

  1. Open room at permalink in the past
  2. Scroll past the unread marker until you are in a later chunk, but not in the last one
  3. Exit and re-enter room at permalink in the past
  4. Check if read marker line has updated

Tested devices

  • Physical
  • Emulator
  • OS version(s): Android 11

Checklist

SetReadMarkerTask was not updating the read marker when both the old and
the new fully read eventId weren't in the last chunk, even when the new
one was after the first one.
@bmarty bmarty added the Z-Community-PR Issue is solved by a community member's PR label Mar 10, 2022
Imagine scenario:

[this] -> [chunkToCheck] -> [lastForwardChunk]

Then, both `isLastForward` checks will not return, and also the `chunkToCheck.doesNextChunksVerifyCondition { it == this }` will return false.
Since both chunks are connected to the last forward chunk, `isMoreRecent()` will still return `true`, which is wrong in this case.
So do not only check if chunkToCheck has this as any of the next chunks, but also the other way round.
@SpiritCroc SpiritCroc requested a review from ganfra March 11, 2022 10:33
Copy link
Member

@ganfra ganfra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now, thanks!

@ganfra ganfra merged commit 72bd398 into element-hq:develop Apr 7, 2022
@SpiritCroc SpiritCroc deleted the readmarkerupdate branch April 30, 2022 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants