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: Don't emit MsgsNoticed when a message was seen on IMAP #6413

Closed
wants to merge 1 commit into from

Conversation

iequidoo
Copy link
Collaborator

@iequidoo iequidoo commented Jan 6, 2025

receive_imf doesn't emit MsgsNoticed on receipt of an IMAP-seen message, so the code was inconsistent. The easiest fix is to not emit MsgsNoticed when a message was seen on another device, we don't want to emit MsgsNoticed if not all messages are seen, otherwise that removes useful notifications from UIs and may lead to missing messages. Checking that all previous chat messages are at least InNoticed looks complicated, let's not do this for now.

EDIT: This is a replacement of #6351.
EDIT: This approach is wrong because we don't have other events that remove notifications for messages seen on other devices, see #6351 (comment) for more info. Closing the PR.

`receive_imf` doesn't emit `MsgsNoticed` on receipt of an IMAP-seen message, so the code was
inconsistent. The easiest fix is to not emit `MsgsNoticed` when a message was seen on another
device, we don't want to emit `MsgsNoticed` if not all messages are seen, otherwise that removes
useful notifications from UIs and may lead to missing messages. Checking that all previous chat
messages are at least `InNoticed` looks complicated, let's not do this for now.
@iequidoo iequidoo force-pushed the iequidoo/MsgsNoticed-on-seen-msg branch from bb1eb0b to 8037039 Compare January 6, 2025 23:20
@Hocuri
Copy link
Collaborator

Hocuri commented Jan 7, 2025

So, this means that notifications will not be removed anymore, even if the chat was opened on the first device? I don't think we should do that.

And, even if there is some inconsistency, things are working fine today already; in general, we should only change things if they either make future development easier or make things better for the user. And, while an inconsistency could be confusing to users, I don't think that this specific inconsistency is. It's just too small to spend energy on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants