deltachat-core-rust
deltachat-core-rust copied to clipboard
fix: markseen_msgs: Limit not yet downloaded messages state to InNoticed (#2970)
Closes #2970
So the question is if it's doable in the apps to mark a fully downloaded message as seen again (when the user sees it) so that it's InSeen and an MDN is issued.
But, I'm concerned that we will send duplicate MDNs by doing this: Right now, even if the message is marked as
\Seenon IMAP, it's changed toInNoticedand a read receipt is sent. So, every device downloading the message is going to send a read receipt.
Yes, but currently there are no MDNs for big encrypted messages at all. Probably the correct solution is to set the IMAP \Seen flag only for messages for which an MDN is sent (if it should be), what exactly happens for small messages. But this means that a partially downloaded messages may be InSeen, but w/o the \Seen IMAP flag. Maybe then ignore markseen_msgs() call for partially downloaded messages at all?
Also, what I don't understand in the existing code: Why isn't a read receipt sent when the user sees the partially downloaded message?
markseen_msgs()looks like it unconditionally sends an MDN.
Because if it's an encrypted message, it "doesn't want" MDN until it's fully downloaded :)
EDIT: So, if a big message isn't encrypted, with this change there will be two MDNs even in a single-device setup. Looks not good.
Maybe then ignore
markseen_msgs()call for partially downloaded messages at all?
At least \Seen flag should not be set when message is not fully downloaded. Otherwise if you have two devices, and the second device has fully downloaded the message, first device may "read" not fully downloaded message and set \Seen flag without sending MDN. Then second device will not send MDN even if user actually reads the message.
Fixed it in another way, now should work correctly even for multi-device
Fixed one more bug, see fix: Update state of message when fully downloading it.
Overall the logic seems complicated, so probably more tests are needed. This has already failed several review iterations, so let it be draft for now.
EDIT: At least there must be a test that the message state isn't downgraded to InFresh after fully downloading it.
EDIT: This is already covered by test_markseen_not_downloaded_msg() after i pushed the last commit. Added a comment on this.
Fixed one more bug, see
fix: Update state of message when fully downloading it.
In fact this bug was a little hidden, fix: markseen_msgs: Limit not yet downloaded messages state to InNoticed exposes it completely. Before, a partially downloaded message was often already InSeen, so its state didn't need to be updated from IMAP. Still, the bug was already there, just not being catched by any tests.
@iequidoo just re-request a review from & mark the PR as ready for review (= not a draft) once it's ready
I suggest to merge this as this fixes the bug and to avoid any merge conflicts when doing #6210. @Hocuri