diff --git a/README.md b/README.md index 601ab5a161..5583fad0b5 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ SimpleX logo -Invest in SimpleX Chat. [Register now](https://simplexchat.typeform.com/crowdfunding). +Invest in SimpleX Chat. [Learn more on Wefunder](https://wefunder.com/simplexchat). # SimpleX - the first messaging platform that has no user identifiers of any kind - 100% private by design! diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ComposeView.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ComposeView.kt index 40d7927264..ff393a3c30 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ComposeView.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ComposeView.kt @@ -497,8 +497,8 @@ fun ComposeView( } } - fun clearCurrentDraft() { - if (chatModel.draftChatId.value == draftChatId(chat.id, chatScope)) { + fun clearCurrentDraft(forChat: Chat = chat) { + if (chatModel.draftChatId.value == draftChatId(forChat.id, chatScope)) { chatModel.draft.value = null chatModel.draftChatId.value = null } @@ -575,9 +575,10 @@ fun ComposeView( } // TODO [short links] connectCheckLinkPreview - fun checkLinkPreview(): MsgContent { - val msgText = composeState.value.message.text - return when (val composePreview = composeState.value.preview) { + // the state is passed in by a send that must not read the current one - see sendMessageAsync + fun checkLinkPreview(cs: ComposeState = composeState.value): MsgContent { + val msgText = cs.message.text + return when (val composePreview = cs.preview) { is ComposePreview.CLinkPreview -> { val parsedMsg = parseToMarkdown(msgText) val url = getMessageLinks(parsedMsg).first @@ -597,6 +598,10 @@ fun ComposeView( composeState.value = composeState.value.copy(inProgress = true) } + // composeState and its inProgress flag are shared between the chats opened in this view, and sending is not cancelled + // when the chat is switched - a send may only clear or reset the state while it still holds the message that was sent + fun composeHasSentMessage(): Boolean = chatModel.chatId.value == chat.id && composeState.value.inProgress + suspend fun sendMemberContactInvitation() { val mc = checkLinkPreview() sending() @@ -604,10 +609,10 @@ fun ComposeView( if (contact != null) { withContext(Dispatchers.Main) { chatsCtx.updateContact(chat.remoteHostId, contact) - clearState() + if (composeHasSentMessage()) clearState() } - } else { - composeState.value = composeState.value.copy(inProgress = false) + } else withContext(Dispatchers.Main) { + if (composeHasSentMessage()) composeState.value = composeState.value.copy(inProgress = false) } } @@ -624,10 +629,10 @@ fun ComposeView( if (contact != null) { withContext(Dispatchers.Main) { chatsCtx.updateContact(chat.remoteHostId, contact) - clearState() + if (composeHasSentMessage()) clearState() } - } else { - composeState.value = composeState.value.copy(inProgress = false) + } else withContext(Dispatchers.Main) { + if (composeHasSentMessage()) composeState.value = composeState.value.copy(inProgress = false) } } @@ -669,15 +674,19 @@ fun ComposeView( chatModel.channelRelayHostnames.remove(groupInfo.groupId) chatModel.groupMembers.value = relayResults.map { it.relayMember } chatModel.populateGroupMembersIndexes() - clearState() + if (composeHasSentMessage()) clearState() } - } else { - composeState.value = composeState.value.copy(inProgress = false) + } else withContext(Dispatchers.Main) { + if (composeHasSentMessage()) composeState.value = composeState.value.copy(inProgress = false) } } - suspend fun sendMessageAsync(text: String?, live: Boolean, ttl: Int?, sign: Boolean = false): List? { - val cs = composeState.value + // toChat is the chat the message was composed in - it differs from the one this view shows only for the live message + // committed by a chat switch, which has no context item, so the forwarding, editing and reporting branches below + // cannot run with a different chat. cs is that send's state, captured before the switch replaced it. + suspend fun sendMessageAsync(text: String?, live: Boolean, ttl: Int?, sign: Boolean = false, toChat: Chat = chat, cs: ComposeState = composeState.value): List? { + // a send for another chat may not write to composeState, even after that chat is opened again - it was handed over + fun composeIsForSend(): Boolean = toChat.id == chat.id var sent: List? var lastMessageFailedToSend: ComposeState? = null val msgText = text ?: cs.message.text @@ -727,8 +736,8 @@ fun ComposeView( fun updateMsgContent(msgContent: MsgContent): MsgContent { return when (msgContent) { - is MsgContent.MCText -> checkLinkPreview() - is MsgContent.MCLink -> checkLinkPreview() + is MsgContent.MCText -> checkLinkPreview(cs) + is MsgContent.MCLink -> checkLinkPreview(cs) is MsgContent.MCImage -> MsgContent.MCImage(msgText, image = msgContent.image) is MsgContent.MCVideo -> MsgContent.MCVideo(msgText, image = msgContent.image, duration = msgContent.duration) is MsgContent.MCVoice -> MsgContent.MCVoice(msgText, duration = msgContent.duration) @@ -785,12 +794,12 @@ fun ComposeView( } val liveMessage = cs.liveMessage - if (!live) { + if (!live && composeIsForSend()) { if (liveMessage != null) composeState.value = cs.copy(liveMessage = null) sending() } if (!cs.forwarding || chatModel.draft.value?.forwarding == true) { - clearCurrentDraft() + clearCurrentDraft(toChat) } if (cs.contextItem is ComposeContextItem.ForwardingItems) { @@ -801,6 +810,8 @@ fun ComposeView( if (cs.message.text.isNotEmpty()) { sent?.mapIndexed { index, message -> if (index == sent!!.lastIndex) { + // the current state, not cs: forwarding is never reached from the chat switch, and keeps what was typed + // while it was in flight send(chat, checkLinkPreview(), quoted = message.id, live = false, ttl = ttl, mentions = cs.memberMentions, sign = sign) } else { message @@ -814,7 +825,7 @@ fun ComposeView( sent = if (updatedMessage != null) listOf(updatedMessage) else null lastMessageFailedToSend = if (updatedMessage == null) constructFailedMessage(cs) else null } else if (liveMessage != null && liveMessage.sent) { - val updatedMessage = updateMessage(liveMessage.chatItem, chat, live) + val updatedMessage = updateMessage(liveMessage.chatItem, toChat, live) sent = if (updatedMessage != null) listOf(updatedMessage) else null } else if (cs.contextItem is ComposeContextItem.ReportedItem) { sent = sendReport(cs.contextItem.reason, cs.contextItem.chatItem.id) @@ -824,7 +835,7 @@ fun ComposeView( val remoteHost = chatModel.currentRemoteHost.value when (val preview = cs.preview) { ComposePreview.NoPreview -> msgs.add(MsgContent.MCText(msgText)) - is ComposePreview.CLinkPreview -> msgs.add(checkLinkPreview()) + is ComposePreview.CLinkPreview -> msgs.add(checkLinkPreview(cs)) is ComposePreview.ChatLinkPreview -> { val linkStr = preview.chatLink.connLinkStr val text = if (msgText.isEmpty()) linkStr else "$msgText\n$linkStr" @@ -915,7 +926,7 @@ fun ComposeView( localPath = file.filePath ) } - val sendResult = send(chat, content, if (index == 0) quotedItemId else null, file, + val sendResult = send(toChat, content, if (index == 0) quotedItemId else null, file, live = if (content !is MsgContent.MCVoice && index == msgs.lastIndex) live else false, ttl = ttl, mentions = cs.memberMentions, @@ -932,23 +943,47 @@ fun ComposeView( val wasForwarding = cs.forwarding val forwardingFromChatId = (cs.contextItem as? ComposeContextItem.ForwardingItems)?.fromChatInfo?.id val lastFailed = lastMessageFailedToSend - if (lastFailed == null) { - clearState(live) - } else { - composeState.value = lastFailed - } - val draft = chatModel.draft.value - if (wasForwarding && chatModel.draftChatId.value == draftChatId(chat.chatInfo.id, chatScope) && forwardingFromChatId != chat.chatInfo.id && draft != null) { - composeState.value = draft - } else { - clearCurrentDraft() + // composeState is shared between the chats opened in this view, and this runs after the send API call, so the user + // could have switched chats or typed another message in the meantime - only the message that was sent may be + // cleared or restored. On Main, so that these checks and changes are not interleaved with the user switching + // chats or typing. + withContext(Dispatchers.Main) { + val chatIsOpen = composeIsForSend() && chatModel.chatId.value == chat.id + // a live message is held in the compose state of the chat it is sent to, but only while that chat is the one open + val liveSend = live || cs.liveMessage != null + val sentMessageInCompose = chatIsOpen && (liveSend || composeState.value.inProgress) + if (sentMessageInCompose) { + if (lastFailed == null) { + clearState(live) + } else { + composeState.value = lastFailed + } + } + val draft = chatModel.draft.value + if (wasForwarding && chatModel.draftChatId.value == draftChatId(chat.chatInfo.id, chatScope) && forwardingFromChatId != chat.chatInfo.id && draft != null) { + if (sentMessageInCompose) composeState.value = draft + } else { + clearCurrentDraft(toChat) + // liveSend excluded: a failing keystroke send would otherwise write a draft on every attempt + if (!sentMessageInCompose && !liveSend && lastFailed != null) { + // the message was not sent, so it is restored in the chat it was composed in, or kept as its draft if another chat is open + if (chatIsOpen && composeState.value.empty) { + composeState.value = lastFailed + } else if (saveLastDraft) { + chatModel.draft.value = lastFailed + chatModel.draftChatId.value = draftChatId(chat.id, chatScope) + } + } + } } return sent } - fun sendMessage(ttl: Int?, sign: Boolean = false) { + // toChat and composed are for the chat switch, which hands the compose state over to the chat it opened; passing + // toChat without doing that leaves the sent message in the input + fun sendMessage(ttl: Int?, sign: Boolean = false, toChat: Chat = chat, composed: ComposeState? = null) { withLongRunningApi(slow = 120_000) { - sendMessageAsync(null, false, ttl, sign) + sendMessageAsync(null, false, ttl, sign, toChat, composed ?: composeState.value) } } @@ -1313,13 +1348,29 @@ fun ComposeView( KeyChangeEffect(chatModel.chatId.value) { prevChatId -> val cs = composeState.value if (cs.liveMessage != null && (cs.message.text.isNotEmpty() || cs.liveMessage.sent)) { - sendMessage(null) + // the chat is already switched, so the live message goes to the chat with the id it had before the switch + val liveMessageChat = if (prevChatId == null || prevChatId == chat.id) chat else chatsCtx.getChat(prevChatId) + // if that chat is gone there is nowhere to send it, and it must not be sent to the chat opened instead + // cs is captured on this thread, before the compose state is replaced below + if (liveMessageChat != null) sendMessage(null, toChat = liveMessageChat, composed = cs) else clearState() resetLinkPreview() clearPrevDraft(prevChatId) deleteUnusedFiles() + // the sent message belongs to the chat it was composed in; the chat opened next shows its own draft + val draft = chatModel.draft.value + composeState.value = if (draft != null && chatModel.draftChatId.value == draftChatId(chatModel.chatId.value, chatScope)) draft + else ComposeState(useLinkPreviews = useLinkPreviews) } else if (cs.inProgress) { clearPrevDraft(prevChatId) - composeState.value = cs.copy(inProgress = false, progressByTimeout = false) + // the message being sent must not be kept in the compose state, it is shared with the chat opened next; + // if it fails to send it is restored in this chat or saved as its draft + clearState() + // clearState() does not load the draft of the chat opened next, and without this it is never shown and is + // dropped when that chat is left + val draft = chatModel.draft.value + if (draft != null && chatModel.draftChatId.value == draftChatId(chatModel.chatId.value, chatScope)) { + composeState.value = draft + } } else if (!cs.empty) { if (cs.preview is ComposePreview.VoicePreview && !cs.preview.finished) { recState.value = RecordingState.NotStarted diff --git a/apps/simplex-support-bot/src/messages.ts b/apps/simplex-support-bot/src/messages.ts index c35789d26b..33f7f9ccef 100644 --- a/apps/simplex-support-bot/src/messages.ts +++ b/apps/simplex-support-bot/src/messages.ts @@ -2,7 +2,10 @@ import {isWeekend} from "./util.js" export const welcomeMessage = `Hello! This is a *SimpleX team* support bot - not an AI. *Join public groups* at https://simplex.chat/directory or [via directory bot](https://smp4.simplex.im/a#lXUjJW5vHYQzoLYgmi8GbxkGP41_kjefFvBrdwg-0Ok) -Please ask any questions about SimpleX Chat.` + +We just launched [equity crowdfunding on Wefunder](https://wefunder.com/simplex.chat)! + +Please ask any questions about SimpleX Chat and about our crowdfunding.` export function queueMessage(timezone: string, grokEnabled: boolean): string { const hours = isWeekend(timezone) ? "48" : "24" diff --git a/images/github-banner.jpg b/images/github-banner.jpg index f7a0d730b8..ef3cb5e6f5 100644 Binary files a/images/github-banner.jpg and b/images/github-banner.jpg differ diff --git a/plans/2026-07-25-fix-inflight-send-writes-other-chat-compose.md b/plans/2026-07-25-fix-inflight-send-writes-other-chat-compose.md new file mode 100644 index 0000000000..7a27fdd9d1 --- /dev/null +++ b/plans/2026-07-25-fix-inflight-send-writes-other-chat-compose.md @@ -0,0 +1,244 @@ +# Fix: message being sent leaks into another chat's compose/draft, and erases what is typed there + +Branch: `nd/fix-inflight-send-writes-other-chat-compose` (off `origin/stable`) +Date: 2026-07-25 +PR: #7308 + +Line references are against `origin/stable` at `8dc387cb5`, with this fix +applied. Android and desktop only +(`multiplatform/.../views/chat/ComposeView.kt`); iOS has the same defect +but is not addressed here. + +## Problem + +Two reported symptoms, one cause. Both need a send that is still in +flight when the chat is switched (slow network, large file, or the send +just hanging with the progress circle showing): + +1. **The message ends up in another chat's draft.** Reply to a message + (or just type), press send, switch to another chat while it is + sending: the text *and the reply context* appear in that chat's input, + and leaving it saves them as that chat's draft. No forwarding + involved. +2. **A late send erases what you typed.** Press send, the progress circle + keeps spinning, switch to another chat and back, type a new message — + when the original send finally succeeds, the newly typed message is + erased. + +## Cause + +The compose state is shared, and the send outlives the chat: + +- `ChatView.kt:134` — one `MutableState` per `ChatView` + instance, `rememberSaveable` with no keys, reused for every chat that + the view displays. +- `Utils.kt:43-46` — `withLongRunningApi` launches on + `CoroutineScope(Dispatchers.Default)`, a standalone scope with no tie + to the composition or to the chat, and `sendMessage` + (`ComposeView.kt:972-976`) uses it. Leaving the chat never cancels an + in-flight send. + +Two writes then act on the wrong chat: + +- **On the chat switch** — `ComposeView.kt:1343-1347`: the `cs.inProgress` + branch used to keep the message in the shared compose state + (`composeState.value = cs.copy(inProgress = false, progressByTimeout = false)`) + and only cleared the *previous* chat's saved draft. The text and the + quote were therefore sitting in the input of the chat opened next, and + `ComposeView.kt:1348-1358` (`!cs.empty`) then saved them as *that* + chat's draft on the next switch. Symptom 1. +- **When the send completes** — `ComposeView.kt:943-968`, running in the + detached coroutine after the switch: `clearState(live)` on success, or + `composeState.value = lastFailed` on failure, where `lastFailed = + cs.copy(inProgress = false, preview = preview)` + (`ComposeView.kt:729`) **keeps `contextItem`, i.e. the reply**. On + success this wipes whatever is in the input now — including a message + typed after coming back (symptom 2); on failure it dumps the old + message into whichever chat is open (symptom 1 again). + +The same function was already inconsistent about which chat it acts on: +its draft bookkeeping (`clearCurrentDraft()`, and the forwarding +condition) uses the **captured** `chat` — the chat the message was +composed in — while its `composeState` writes hit whatever chat is +displayed at that moment. + +## Fix + +Two changes, both in `ComposeView.kt`. + +**1. Do not keep the message being sent in the shared compose state** +(`ComposeView.kt:1343-1347`). On switching away with a send in flight the +compose state is cleared, so nothing leaks into the chat opened next: + +```kotlin +} else if (cs.inProgress) { + clearPrevDraft(prevChatId) + // the message being sent must not be kept in the compose state, it is shared with the chat opened next; + // if it fails to send it is restored in this chat or saved as its draft + clearState() +} +``` + +`clearState()` is used rather than assigning an empty `ComposeState` so that +the link preview state is reset too (`pendingLinkUrl` still points at the +sent message's link, and its fetch would otherwise set a preview on the +input of the chat opened next), and so that the attachment size limit is +carried over the same way as everywhere else. + +In-flight content is deliberately **not** saved as a draft here: the +message has been submitted and will most likely be sent, and a draft is +for messages that are not sent yet. + +`clearState()` alone would leave the chat opened next with an empty input +even when it has a draft: this branch, like the live message one above it, +returns before the branch that loads a draft +(`chatModel.draftChatId.value == draftChatId(chatModel.chatId.value, chatScope)`), +so that draft was never shown - and, being still in the slot but not in +any compose state, it was then dropped by `clearPrevDraft` on the next +chat switch. It is loaded here instead. This is not the one-slot +limitation below: nothing else is competing for the slot, the draft is +simply lost. + +**2. Only touch the compose state if it still holds the message that was +sent** (`ComposeView.kt:936-968`): + +```kotlin +withContext(Dispatchers.Main) { + val chatIsOpen = chatModel.chatId.value == chat.id + val liveSend = live || cs.liveMessage != null + val sentMessageInCompose = chatIsOpen && (liveSend || composeState.value.inProgress) + if (sentMessageInCompose) { + if (lastFailed == null) { + clearState(live) + } else { + composeState.value = lastFailed + } + } + val draft = chatModel.draft.value + if (wasForwarding && chatModel.draftChatId.value == draftChatId(chat.chatInfo.id, chatScope) && forwardingFromChatId != chat.chatInfo.id && draft != null) { + if (sentMessageInCompose) composeState.value = draft + } else { + clearCurrentDraft() + if (!sentMessageInCompose && lastFailed != null) { + // the message was not sent, so it is restored in the chat it was composed in, or kept as its draft if another chat is open + if (chatIsOpen && composeState.value.empty) { + composeState.value = lastFailed + } else if (saveLastDraft) { + chatModel.draft.value = lastFailed + chatModel.draftChatId.value = draftChatId(chat.id, chatScope) + } + } + } +} +``` + +Both the checks and the changes run on `Dispatchers.Main` (the block has +no suspension points), so they cannot be interleaved with the user +switching chats or typing - `KeyChangeEffect`, which does change 1, runs +there too. + +`inProgress` is the marker that the compose state is still the submitted +message: it is set by `sending()` (`ComposeView.kt:596-598`), preserved by +`copy` while sending (the only other write during a send is +`progressByTimeout` at `ComposeView.kt:1610-1617`), reset when switching +away (change 1), and never set by typing a new message. So a chat switch +*or* newly typed text both make the guard false. + +A **failed** send is different from an in-flight one - the message was not +sent, so it is an unsent message. It is put back into the input if that +chat is open and nothing else is being composed there, and kept as that +chat's draft otherwise, so it never appears in another chat (see the +limitations below for when it is still dropped). Staying in the chat is +unaffected: the guard is true there +and the failed message is restored into the input as before, keeping +"preserving long message when failed to send" (`e61babdc8`) working. + +Deliberately unchanged: + +- The condition of the forwarding branch. Gating the whole branch would + send a forward that completed after the user left to `clearCurrentDraft()` + instead, **deleting** the destination chat's draft that the branch + exists to preserve - only the compose write inside it is gated. +- Live message sends (`live`, or `cs.liveMessage != null` for the send + that finalises a live message when leaving the chat, `ComposeView.kt:1338-1342`), + as long as their chat is the one open. They never call `sending()`, so a + guard based on `inProgress` would change their behaviour: failed live + sends would stop restoring and would write a draft on every failing + keystroke send. That is why `liveSend` is an alternative to `inProgress` + inside the guard, and why it is excluded from the restore/draft branch - + not gating it there would produce exactly that draft-per-keystroke. + + What they are **not** exempt from is `chatIsOpen`. An earlier revision + had `live || cs.liveMessage != null` outside it, which holds only while + a live message is always sent to the chat that is open. #7323 removes + that: the live message committed by a chat switch is sent to the chat it + was composed in, while this view already shows another one, so an + unguarded clause here would clear *that* chat's compose state - the leak + this fix exists to prevent. Standalone this changes nothing except a + live send that completes after its chat was left, which now leaves the + opened chat alone. `sendMessageAsync` reads `composeState` inside the + coroutine (`ComposeView.kt:684`), so that branch cannot clear the state + itself without racing the send; #7323 adds the `composed` parameter that + makes the captured state explicit. + +**3. The same check where the flag is shared** (`ComposeView.kt:600-602` +and the three senders that connect a prepared chat). They call the same +`sending()`, so an unguarded `clearState()` or `inProgress` reset from +one of them corrupts the state of a send started in the chat opened next. + +## Behaviour after the fix + +| situation | before | after | +| --- | --- | --- | +| send, stay in chat, succeeds | input cleared | input cleared (unchanged) | +| send, stay in chat, fails | message restored in input | message restored in input (unchanged) | +| send, switch chats, succeeds | message left in the other chat's input, saved as its draft | other chat untouched | +| send, switch chats, fails | message dumped into the other chat's input | message restored in the chat it was composed in, or kept as its draft | +| send hangs, switch away and back, type, then it succeeds | typed message erased | typed message kept | +| forward send, still in destination chat | destination chat's draft restored | unchanged | +| live message sent on leaving the chat | compose state cleared by the send | unchanged | + +## Limitations + +Kept deliberately, to not grow the change: + +- A message that failed to send is dropped, rather than kept, when the + "Message draft" privacy setting is off, when the destination chat of a + failed forward already has a draft (its own draft is preserved + instead), and when the single draft slot is later taken by another + chat - drafts are one global slot, so the last write wins. +- The three senders that connect a prepared chat share the same + `sending()` flag, so they use the same check (`ComposeView.kt:604-616`, + `618-640`, `659-685`). Without it a connect completing after the chat + was switched would clear `inProgress` for a send started in the chat + opened next, and that sent message would then stay in the input. They + have no failed-message restore, so their typed message is dropped when + the chat is switched instead of being carried into the next chat. +- Typing in the same chat while its own send is in flight is still + cleared when the send completes: `inProgress` is preserved by `copy`, + so the guard stays true. Unchanged from before, and different from the + reported symptom, which needs the chat to be switched. +## Verification + +- `./gradlew :common:compileKotlinDesktop` — passes. +- Manual (needs a slow or failing send — e.g. airplane mode, or a large + file). On desktop any chat switch exercises it; on Android only an + in-place switch does (member info → open chat), because leaving to the + chat list destroys the view: + 1. Reply + type in A, send, switch to B while sending. B's input must + stay empty; leaving B must not create a draft in B. If the send + failed, A must hold the message (with the reply) as its draft. + 2. Send in A with the network off so the circle keeps spinning, switch + to B and back to A, type a new message, restore the network. The + typed message must survive the old send completing. + 3. Regression: ordinary send in A (input clears), failed send while + staying in A (message comes back in the input), forward into a chat + that has a draft (draft restored after sending). + +Rebased onto the scope-aware draft ids introduced by #7309: the draft +written here for a message that failed to send uses +`draftChatId(chat.id, chatScope)`, like every other draft write. + +Related: `plans/2026-07-25-fix-forward-moves-draft-to-target-chat.md` +(PR #7307) — different cause (stale `chat` captured by the desktop +`onDispose`), same shared-compose-state design. diff --git a/plans/2026-07-29-fix-live-message-sent-to-wrong-chat.md b/plans/2026-07-29-fix-live-message-sent-to-wrong-chat.md new file mode 100644 index 0000000000..bd37361dc5 --- /dev/null +++ b/plans/2026-07-29-fix-live-message-sent-to-wrong-chat.md @@ -0,0 +1,248 @@ +# Fix: live message is sent to the chat opened after switching chats + +Branch: `nd/fix-live-message-sent-to-wrong-chat` (off `origin/stable`) +Date: 2026-07-29 + +Line references are against `origin/stable` at `970ef8932`, with this fix +applied. Android and desktop (`commonMain/ComposeView.kt`). + +## Problem + +Typing a live message and switching to another chat sends that message to +the chat that was opened, without the user sending anything. Reported on +desktop, where every chat switch reuses the same view. + +## Cause + +A live message is committed when the chat is switched +(`ComposeView.kt:1353-1369`), which before this change was: + +``` + if (cs.liveMessage != null && (cs.message.text.isNotEmpty() || cs.liveMessage.sent)) { + sendMessage(null) +``` + +`KeyChangeEffect` is `LaunchedEffect(key1) { block(prev) }` +(`Utils.kt:683-698`), so when the key changes, `remember(key1)` rebuilds +it from the lambda of the composition that is running *now* - and by then +`chatModel.chatId` is already the new chat, `ChatView` has recomposed +`ComposeView` with the new `chat`, and the block that runs captured that +one. + +`sendMessage(null)` → `sendMessageAsync` → `send(chat, ...)` uses the +captured `chat`, while the message content comes from `composeState`, +which is shared between the chats opened in this view. So the content of +the chat that was left is sent to the chat that was opened: + +- `liveMessage.sent == false` - a new message is created in the wrong + chat, which is what is seen; +- `liveMessage.sent == true` - `apiUpdateChatItem` is called with the new + chat's type and id and the item id from the previous chat, which the + backend cannot resolve. + +The same mismatch made the post-send `clearCurrentDraft()` clear the +draft of the chat opened after the switch, deleting a draft that was +never sent. + +## Fix + +`sendMessageAsync` and `sendMessage` take the chat the message was +composed in, defaulting to the chat this view shows +(`ComposeView.kt:685-694`, `988-993`). Only what a live message can reach +uses it: the message send, the update of an already sent live message, +and the two places that clear the draft after sending. Live messages have +no context item (`SendMsgView.kt:156-165` only offers the button when the +compose is empty and has none), so the forwarding, editing and reporting +branches cannot run with a chat other than the view's and keep using +`chat` - the parameter is not threaded through them. + +The chat switch resolves the chat by the id it had before the switch: + +```kotlin +val liveMessageChat = if (prevChatId == null || prevChatId == chat.id) chat else chatsCtx.getChat(prevChatId) +// if that chat is gone there is nowhere to send it, and it must not be sent to the chat opened instead +if (liveMessageChat != null) sendMessage(null, toChat = liveMessageChat, composed = cs) else clearState() +``` + +`prevChatId == chat.id` keeps the view's own chat, which is what secondary +(member support) chat views need - they share the group's chat id, and +only their `chat` carries the scope. + +If the previous chat can no longer be found the message is not sent at +all, and the compose state is cleared so it does not leak into the chat +that was opened. Sending it to the chat that is open now is the defect +being fixed, so it is not used as a fallback. + +### Handing the compose state over to the opened chat + +Sending to the right chat is not enough on its own: `composeState` is +shared between the chats opened in this view, and this is the only branch +of `KeyChangeEffect` that neither resets it nor loads the opened chat's +draft - the branch that loads a draft (`else if (chatModel.draftChatId +.value == draftChatId(chatModel.chatId.value, chatScope) ...)`) is later +in the same `if` chain and cannot be reached. So the live message stayed +in the compose state of a view that now shows another chat, and that +chat's draft was never read. + +`sendMessageAsync` then made it visible. It runs on `Dispatchers.Default`, +so its writes land after the switch: + +```kotlin +val liveMessage = cs.liveMessage +if (!live) { + if (liveMessage != null) composeState.value = cs.copy(liveMessage = null) // the whole composed state + sending() // and its spinner +} +``` + +The opened chat's input showed the text composed in the previous one until +the send completed and `clearState()` emptied it; the draft it should have +shown was still in the model, and the next switch away dropped it. This +predates this fix - without it the same writes happen, and there +`clearCurrentDraft()` resolves to the opened chat and deletes its draft +outright. + +Four changes, all following from "this send no longer owns the compose +state": + +- `sendMessageAsync` takes its `cs` as a parameter defaulting to + `composeState.value`, and `sendMessage` takes `composed: ComposeState? = + null`, so only the chat switch passes a state and every other sender + still reads it inside the coroutine, exactly where the send read it + before. The chat switch + captures it on the main thread before replacing it - without that 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 it is reached by every text live + message through `updateMsgContent`, so with the compose state handed + over it would have rebuilt the message from the opened chat's draft, or + from nothing - overwriting the live message instead of committing it. + Only the calls a live message can reach pass the state. The forwarding + call site keeps reading the current one - it is unreachable from the + chat switch, and `forwardItem` suspends before it, so passing the + captured state there would drop what was typed while the forward was in + flight. The three senders that connect a prepared chat keep reading the + current one too. +- Every `composeState` write in `sendMessageAsync` is guarded by + `composeIsForSend()` (`toChat.id == chat.id`): directly for the two at + the start, and through `chatIsOpen` for the clear/restore at the end, + which #7308 already routes through `sentMessageInCompose`. It compares + the two chats rather than checking which one is open, so the send made + by a chat switch never takes the compose state back, not even if that + chat is opened again before the send completes. + `clearCurrentDraft(toChat)` is already keyed on the chat and needs no + guard. Whether the *view's own* send may still write when its chat has + been switched away is #7308's question, not this one's. +- The chat-switch branch then resets `composeState` to the opened chat's + draft, or to an empty state, like the branches below it do. + +## Blast radius + +`toChat` defaults to the chat this view shows, so every other send passes +no chat: the send button (`SendMsgView.kt`), the live updates while typing +(`sendMessageAsync(live = true)`), forwarding, editing and reporting. For +all of them `composeIsForSend()` is true, so every guard added here is a +no-op and they behave exactly as before. Only the send started by the chat +switch passes a different chat, and only the branches it can reach were +changed. + +The one change not behind that guard is `checkLinkPreview` reading the +state passed in. It matters only where the two can differ, which is after +a suspension: the forwarding branch waits on `forwardItem`, so that call +site deliberately keeps reading the current state (it is unreachable from +the chat switch anyway). The other call sites are reached with nothing +suspending since the state was captured. + +The live message update loop is not affected: it is started once +(`SendMsgView.kt:523-559`) with the `::updateLiveMessage` reference of the +composition in which live mode started, so its updates already go to the +chat the message belongs to. It exits because the chat switch replaces the +compose state with the opened chat's, which has no `liveMessage` - on the +main thread, as the chat is switched, rather than when the send completes +as before. Only that send was created fresh on every composition, which is +why it was the one going to the wrong chat. + +Not covered, and unchanged: a live message in a member support chat that +is closed without changing the chat id is never committed - the effect +that commits it is keyed on the chat id, which does not change when that +view is closed. + +`chatsCtx.getChat` searches the context's own list, and a secondary +context (member support, reports) is built with an empty one, so there it +can only return null. That branch is not reached from a support chat in +practice - it shares the group's chat id, so `prevChatId == chat.id` holds +and the view's own `chat` is used - and if it ever were, the message is +discarded rather than sent to the chat that was opened, which is the +behaviour intended for "the chat is gone" anyway. + +## Verification + +- `./gradlew :common:compileKotlinDesktop` — passes. +- Manual: + 1. Start a live message in **A**, type, and switch to **B** while + typing. The message must appear in **A**; nothing is sent in **B**, + and B's input and draft are untouched. + 2. Repeat with a draft already saved in **B** - it must still be there + after the switch. This is the case that was found failing: B showed + the text composed in A, then emptied when the send completed, and B's + draft was dropped on the next switch. Watch B's input from the moment + of the switch, not only after the send finishes. + 3. Slow or failing send (network off) while doing 1 and 2, so the window + between the switch and the send completing is long enough to type in + **B** - what is typed there must survive the send completing. + 4. The live message must carry a **link preview**: type a URL in **A**, + let the preview load, then switch. The message committed to A must be + the text that was composed - not the opened chat's draft, and not + empty. Every text live message is rebuilt through + `updateMsgContent` -> `checkLinkPreview`, so this is what breaks if + that one stops reading the state it was given. + 5. Switch **back**: live message in A, switch to B, return to A and type + something new before the send completes. What is typed in A must + survive - the send handed the compose state over at the switch and + must not take it back. + 6. Regressions: an ordinary send goes to the chat it was typed in; + forwarding still targets the chat it was forwarded to, and text typed + while a forward is in flight is still appended to it; reporting a + message still reports it in the chat it belongs to; sending in a + member support chat still goes to that scope. + +## Merged with #7308 + +#7308 (a send that is still in flight when the chat is switched) landed in +`stable` first, so this branch was merged with it. Both changed the end of +`sendMessageAsync`, and the two guards are **not** the same rule - the +merge keeps both: + +- here, `composeIsForSend()` = `toChat.id == chat.id` - is this send for + the chat this view shows, or for another one; +- in #7308, `chatIsOpen` = `chatModel.chatId.value == chat.id` - is the + chat this view shows still the one open. + +`chatIsOpen` becomes the conjunction, +`composeIsForSend() && chatModel.chatId.value == chat.id`. Where `toChat` +is `chat` - every send but the one made by a chat switch - that reduces to +#7308's own check, so its behaviour is unchanged. + +Nothing else in that block had to move. #7308 already routes both compose +writes through `sentMessageInCompose`, which derives from `chatIsOpen`, so +guarding `chatIsOpen` guards them; the rest of the change there is one +call site taking `toChat`, `clearCurrentDraft`. 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. + +An earlier revision of this note said that #7308's `cs.liveMessage != null` +clause "already covers the send made by the chat switch". **It did not.** +At the time that clause sat outside the `chatIsOpen` check: + +```kotlin +val sentMessageInCompose = live || cs.liveMessage != null || (chatIsOpen && composeState.value.inProgress) +``` + +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, the clause +*exempts* the chat-switch send from the very guard that protects the +opened chat, and a merge that followed it reintroduced the leak described +above. #7308 shipped with the live clauses moved inside `chatIsOpen`, +which was a no-op on its own branch and is what makes this merge work. diff --git a/website/langs/en.json b/website/langs/en.json index 3cbcb12ee2..482fe50042 100644 --- a/website/langs/en.json +++ b/website/langs/en.json @@ -264,6 +264,8 @@ "index-hero-h1": "Be
Free", "index-hero-h2": "In Your Network", "index-hero-p1": "The first network without user IDs.
You own your contacts, groups and channels.", + "index-hero-invest": "Invest in SimpleX Chat.", + "index-hero-invest-cta": "Learn more on Wefunder.", "index-hero-download-desktop-btn-title": "Download SimpleX Desktop App", "index-testflight-title": "SimpleX iOS beta-release on TestFlight", "index-f-droid-title": "SimpleX app via F-Droid", diff --git a/website/src/css/design3.css b/website/src/css/design3.css index ea6f20273b..4b645804eb 100644 --- a/website/src/css/design3.css +++ b/website/src/css/design3.css @@ -434,6 +434,16 @@ section.cover div.content p { max-width: calc(var(--sec-vwu) * 53); } +section.cover div.content p.invest { + font-weight: 300; +} + +section.cover div.content p.invest a { + font-weight: 500; + text-decoration: none; + white-space: nowrap; +} + .publications-btns { position: absolute; bottom: 24px; @@ -960,7 +970,7 @@ main .section-bg { @media (max-width: 959px) { section.cover div.content { gap: calc(var(--sec-vhu) * 2.5); - transform: translateY(calc(var(--sec-vhu) * 5)); + transform: translateY(calc(var(--sec-vhu) * 3)); } .publications-btns, @@ -994,6 +1004,14 @@ main .section-bg { max-width: calc(var(--sec-vwu) * 93); } + section.cover div.content p.invest { + font-weight: 400; + } + + section.cover div.content p.invest a { + font-weight: 600; + } + /* --- MAIN SECTIONS --- */ .page .text-container { justify-content: flex-end; diff --git a/website/src/index.html b/website/src/index.html index ba4494ab1d..3ce15f6d56 100644 --- a/website/src/index.html +++ b/website/src/index.html @@ -100,6 +100,7 @@ active_home: true

{{ "index-hero-h1" | i18n({}, lang) | safe }}

{{ "index-hero-h2" | i18n({}, lang) | safe }}

{{ "index-hero-p1" | i18n({}, lang) | safe }}

+

{{ "index-hero-invest" | i18n({}, lang) | safe }} {{ "index-hero-invest-cta" | i18n({}, lang) | safe }}