mirror of
https://github.com/simplex-chat/simplex-chat.git
synced 2026-08-14 22:20:35 +00:00
Narasimha-scandGitHub
f921bd47bb
* android, desktop: fix live message sent to the chat opened after switching A live message is sent when the chat is switched, but by then this view already shows the chat that was opened - the effect that sends it runs with that chat, so the message typed in one chat was sent to another. The chat the message was composed in is passed to the send, and it is resolved by the chat id from before the switch. If that chat is no longer there the message is not sent at all, rather than sent to the chat opened instead. The draft cleared after sending is the one of that chat too. * plan: correct references; clear the draft of the chat the message is sent to * plan: note the blast radius and how to resolve the overlap with #7308 * android, desktop: only pass the chat to what a live message can reach A live message has no context item, so the forwarding, editing and reporting branches of the send cannot run for it - they keep using the chat of the view, and the chat it was composed in is passed only to the message send, to the update of an already sent live message, and to clearing the draft after sending. * android, desktop: give the opened chat its own compose state while the live message is sent Sending the live message to the chat it was composed in is not enough on its own: composeState is shared between the chats opened in this view, and the chat switch branch of KeyChangeEffect is the only one that neither resets it nor loads the opened chat's draft - the branch that loads a draft is later in the same if chain and cannot be reached. sendMessageAsync then made it visible. It runs on Dispatchers.Default, so its writes land after the switch: the whole composed state (via cs.copy(liveMessage = null)) and its spinner (sending()) were written to the compose state of a view that already shows another chat, which then displayed the text composed in the previous one until the send completed. The draft it should have shown was still in the model, and the next switch away dropped it. - sendMessageAsync takes composed, and sendMessage takes it as null by default, so only the chat switch passes a state and every other sender reads it inside the coroutine, where the send read it before. The chat switch captures it on the main thread before replacing it - otherwise the send would read the compose state of the chat that was opened and send its draft to the previous chat. - checkLinkPreview takes that state too. It re-read composeState rather than what was passed in, and every text live message reaches it through updateMsgContent, so it would have rebuilt the message from the opened chat's draft instead of committing what was composed. - every composeState write in sendMessageAsync is guarded by composeIsForSend() (toChat.id == chat.id), which compares the two chats instead of checking which one is open, so this send never takes the compose state back if that chat is opened again before it completes. - the chat switch branch then resets composeState to the opened chat's draft, or to an empty state, like the branches below it do. * plan: document the compose state handoff; correct the #7308 overlap resolution The note on resolving the overlap with #7308 said its cs.liveMessage != null clause "already covers the send made by the chat switch". It does not - in #7308 that clause sits outside the chatIsOpen check, which is correct only while a live message is always sent to the chat that is open, the assumption this fix removes. Read as written it exempts the chat switch send from the guard that protects the opened chat, and a merge that follows it reintroduces the leak. Also records what manual test 2 was found failing on, and adds a slow send variant so the window between the switch and the send completing is long enough to type in the chat that was opened. * android, desktop: keep reading the current state where a forward appends it checkLinkPreview taking the composed state is needed where a live message reaches it, but the forwarding branch is not one of those - it cannot run with a chat other than the view's - and forwardItem suspends before it. So there the captured state is stale by a network round trip, and text typed while the forward was in flight stopped being appended to the message it adds, while still being cleared when the send completed. * plan: correct references and the claims that no longer hold Line references were against the base this branch forked from, before #7308 landed. Also: the live message loop no longer exits because the send clears liveMessage - the chat switch replaces the compose state, on the main thread, before the send runs; checkLinkPreview is not passed the captured state everywhere; and chatsCtx.getChat can only return null in a secondary context, which is not how "the chat is gone" reads. Adds the two manual checks the review implied: a live message carrying a link preview, which is what breaks if checkLinkPreview stops reading the state it was given, and returning to the chat before the send completes. * android, desktop: narrow the change to what the fix needs - the draft id a failed message is saved under keeps using chat: that branch is behind !liveSend, which the send made by a chat switch never satisfies, so toChat is always chat where it is read; - the state the chat switch installs no longer carries maxFileSize over. That field is kept in sync on chat switch by LaunchedEffect(chat.chatInfo), which is why the branches below this one construct it without one, and the paths where that effect does not re-run are the ones where the send overwrites the state anyway; - sendMessageAsync takes its cs as a parameter rather than aliasing a separately named one. * plan: follow the narrowed change * android, desktop: shorten the comments The threading mechanism behind cs is explained where it is used, so the function comment only has to say what it is; the rest is rewording.