mirror of
https://github.com/simplex-chat/simplex-chat.git
synced 2026-08-14 11:30:18 +00:00
Merge branch 'stable'
This commit is contained in:
+36
-19
@@ -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
|
||||
@@ -680,8 +681,12 @@ fun ComposeView(
|
||||
}
|
||||
}
|
||||
|
||||
suspend fun sendMessageAsync(text: String?, live: Boolean, ttl: Int?, sign: Boolean = false): List<ChatItem>? {
|
||||
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<ChatItem>? {
|
||||
// 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<ChatItem>?
|
||||
var lastMessageFailedToSend: ComposeState? = null
|
||||
val msgText = text ?: cs.message.text
|
||||
@@ -731,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)
|
||||
@@ -789,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) {
|
||||
@@ -805,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
|
||||
@@ -818,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)
|
||||
@@ -828,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"
|
||||
@@ -919,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,
|
||||
@@ -941,7 +948,7 @@ fun ComposeView(
|
||||
// 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 = chatModel.chatId.value == chat.id
|
||||
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)
|
||||
@@ -956,7 +963,7 @@ fun ComposeView(
|
||||
if (wasForwarding && chatModel.draftChatId.value == draftChatId(chat.chatInfo.id, chatScope) && forwardingFromChatId != chat.chatInfo.id && draft != null) {
|
||||
if (sentMessageInCompose) composeState.value = draft
|
||||
} else {
|
||||
clearCurrentDraft()
|
||||
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
|
||||
@@ -972,9 +979,11 @@ fun ComposeView(
|
||||
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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1339,10 +1348,18 @@ 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)
|
||||
// the message being sent must not be kept in the compose state, it is shared with the chat opened next;
|
||||
|
||||
@@ -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.
|
||||
Reference in New Issue
Block a user