Mark as read when scrolling down after seeing unread marker doesn't always work

[PVince81]

Steps to reproduce

1. Switch to a conversation which has ~2 pages of unread messages
2. Wait for it to load
3. See that the "Unread messages" marker is here
4. Scroll down past the unread message until the unread message itself and the next message becomes invisible
5. Pause for more than one second: this will update the marker internally
6. Scroll down to bottom
7. Wait a few second
8. See left sidebar to see if the counter of unread messages got to zero. If yes: no bug, try again with a different room.
9. Scroll up again to the marker, then down again.

Expected behaviour

After seeing the unread messages marker and scrolling down, always mark the chat as read.

Actual behaviour

Abount 1/3 of the time, it seems the marker is not properly detected and the read marker is not reset when scrolling down. The marker does not get updated and the number of unread messages stay. Need to switch to another conversation then back to reset the broken state.

Talk app

Talk app version: 12.0.0x (the one from c.nc.com)

Needs further research to find out what exactly if preventing the detection. See https://github.com/nextcloud/spreed/issues/5814#issuecomment-874085842 for explanation of the problem

[PVince81]

hmm, seems one needs to scroll way further above the read marker to make it be detected. maybe dependent on the size of the message that is above it.

I'll keep an eye and take notes

[nickvergessen]

Mostlikely attachments break it as rendering the image makes it jump always

[PVince81]

I just saw it happen but the attachment would push down the marker, so if you scroll down you'd cross it anyway. Not sure if there are scenarios where the marker would be pushed up.

A way to analyze this live: next time the bug happens, check the "data-seen" DOM property on the marker. It should be "true" when seen.

[PVince81]

• [ ] Possible race condition, not sure if related: at the time I've reached the bottom of the chat I saw the left sidebar flicker once between non-bold and then back to bold. It's likely that if we're fetching the left sidebar list (with unreadMessages > 0 in the result) and at the same time updating the read marker, the effect might cancel out. Or delay it for 30 more seconds.

[PVince81]

I've observed this a few times also and could see that data-seen was already true, but for some reason when I scrolled to the bottom it did not fire out the call to "/read". From that point I wasn't able to find out why it wouldn't do so. And often times after scrolling up and down again it suddenly got unstuck.

[PVince81]

something I've just observed at least twice: after opening a chat with more than one page of messages, if I scroll down slowly while reading, before I reach the bottom I see a call to "/read" already happening. When I reach the bottom no other call happens.

After inspecting that message id in that call, it seems to be the one where the read marker was already positioned, or at least in previous scrolling pages and not visible.

There's a debounce of 1s for the read marker handler after scrolling, so possibly I've triggered it while reading messages. Maybe findFirstVisibleMessage is not working reliably.

• [ ] try and find out if the message id that got sent was indeed the one from the marker, or one below it
  • :warning: when checking the console window, do not return to the chat window as it will trigger "update visually", so make sure to inspect everything that needs inspecting first
  • [ ] check if said message has "data-seen=true" or not. Usually I have to scroll up then down again to retrigger the "/read" call
  • [ ] does "update visually" cause side effects ?
• [ ] double check scroller element and whether it returns correct values (we did have some recent changes/bugs related to scrolling)

[PVince81]

Got it.

It's indeed a bug with the discrepancy between the visual read marker, which DOM element will have data-seen=true when seeing it the first time and the store read marker, which has already been updated once after scrolling slowly.

The code in updateReadMarkerPosition() is expecting that this.unreadMessageElement has data-seen=true to proceed. However, after the first read marker update, the visual DOM element (old marker) and the store marker have a different position. The message element from the store will never have data-seen=true because it doesn't have the actual visible marker there (the logic for this attribute is tied specifically to that one message element with marker, due to 'observe visibility' to be used sparingly).

We'll need to adjust the code to take this situation into account. If we can't move the visual marker to the new element, we might still want to observe its visibility to check if we should move the marker further. Or: we could always still rely on the message which is on the DOM to compute the next unread message. The latter is probably simpler.

[PVince81]

I've updated the first post to mention the steps to reproduce. They work also on my development instance, all one needs is to pause for 1+ seconds while in the middle of scrolling down.

[PVince81]

Fix is here: https://github.com/nextcloud/spreed/pull/5945