mirror of
https://github.com/simplex-chat/simplex-chat.git
synced 2026-07-10 01:21:52 +00:00
ui: fix SEInvalidMention when @mention is edited in place (#7018)
* android, desktop, ios: drop stale mention map entry when @name is edited in place The compose-state mentions map was only pruned when fewer @name tokens were parsed from the text than were in the map. Editing an inserted @Name token in place (without re-picking from the picker) keeps the count at 1 while the parsed name no longer matches the stored key, so the stale entry was sent and the core rejected it with SEInvalidMention. Pruning now triggers whenever any map key is absent from the parsed mention names; the modified token is then sent as an unresolved formatted @ token. * android, desktop, ios: filter mentions at send time so in-place edits round-trip Treat the compose-state mentions map as a sticky cache of name -> memberId bindings recorded by the picker. The previous removeUnusedMentions pruned it on every text change, which dropped the binding the instant a letter was removed from an inserted @name token. Now the map is no longer mutated by text edits: memberMentions, the picker max-reached check, and the mentionMemberName disambiguator all filter against the names currently parsed from the message, so deleting and retyping the original characters re-resolves the original member, and the SEInvalidMention error from an in-place edit no longer occurs. * android, desktop, ios: cap memberMentions at MAX so stale cache entries cannot overflow the server limit With the sticky cache, manually typing an @name that happens to match a stale cache entry could push memberMentions above maxSndMentions and have the core reject the send with SEInvalidMention. The getter now walks parsedMessage in text order and stops at MAX_NUMBER_OF_MENTIONS; later @-tokens past the cap become visual-only formatting and the message still goes through. Also hoists the iOS activeMentions computation out of the per-row ForEach so it runs once per picker open rather than once per row. * android, desktop, ios: simplify mention restoration to minimal surgical diff Reverts the unnecessary mentionMemberName changes (the original mentions.containsKey behaviour preserves bindings better than the parsed-only variant), switches the picker checks to memberMentions.size (no helper variable needed since the getter already caps at MAX), and collapses the Kotlin memberMentions getter into a flat chain. * android, desktop, ios: gate picker mention-id and max-reached banner by memberMentions With the sticky cache, the picker's "currently bound member" highlight and the showMaxReachedBox banner-suppression clause read mentions[name] directly, so a stale cache entry made one row clickable (as a no-op) at the MAX limit and could suppress the banner when the user is actually adding a new mention. Both now gate the lookup by membership in the capped memberMentions, restoring the pre-fix UX where at MAX all rows are disabled and the banner shows when adding past the limit.
This commit is contained in:
@@ -131,7 +131,12 @@ struct ComposeState {
|
||||
}
|
||||
|
||||
var memberMentions: [String: Int64] {
|
||||
self.mentions.compactMapValues { $0.memberRef?.groupMemberId }
|
||||
var result: [String: Int64] = [:]
|
||||
for ft in parsedMessage {
|
||||
if result.count >= MAX_NUMBER_OF_MENTIONS { break }
|
||||
if case let .mention(name) = ft.format, let id = mentions[name]?.memberRef?.groupMemberId { result[name] = id }
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
var editing: Bool {
|
||||
|
||||
@@ -47,7 +47,7 @@ struct GroupMentionsView: View {
|
||||
LazyVStack(spacing: 0) {
|
||||
ForEach(Array(filtered.enumerated()), id: \.element.wrapped.groupMemberId) { index, member in
|
||||
let mentioned = mentionMemberId == member.wrapped.memberId
|
||||
let disabled = composeState.mentions.count >= MAX_NUMBER_OF_MENTIONS && !mentioned
|
||||
let disabled = composeState.memberMentions.count >= MAX_NUMBER_OF_MENTIONS && !mentioned
|
||||
ZStack(alignment: .bottom) {
|
||||
memberRowView(member.wrapped, mentioned)
|
||||
.contentShape(Rectangle())
|
||||
@@ -124,14 +124,13 @@ struct GroupMentionsView: View {
|
||||
}
|
||||
|
||||
private func messageChanged(_ msg: String, _ parsedMsg: [FormattedText], _ range: NSRange) {
|
||||
removeUnusedMentions(parsedMsg)
|
||||
if let (ft, r) = selectedMarkdown(parsedMsg, range) {
|
||||
switch ft.format {
|
||||
case let .mention(name):
|
||||
isVisible = true
|
||||
mentionName = name
|
||||
mentionRange = r
|
||||
mentionMemberId = composeState.mentions[name]?.memberId
|
||||
mentionMemberId = composeState.memberMentions[name] != nil ? composeState.mentions[name]?.memberId : nil
|
||||
if !m.membersLoaded {
|
||||
Task {
|
||||
await m.loadGroupMembers(groupInfo)
|
||||
@@ -169,15 +168,6 @@ struct GroupMentionsView: View {
|
||||
.sorted { $0.wrapped.memberRole > $1.wrapped.memberRole }
|
||||
}
|
||||
|
||||
private func removeUnusedMentions(_ parsedMsg: [FormattedText]) {
|
||||
let usedMentions: Set<String> = Set(parsedMsg.compactMap { ft in
|
||||
if case let .mention(name) = ft.format { name } else { nil }
|
||||
})
|
||||
if usedMentions.count < composeState.mentions.count {
|
||||
composeState = composeState.copy(mentions: composeState.mentions.filter({ usedMentions.contains($0.key) }))
|
||||
}
|
||||
}
|
||||
|
||||
private func getCharacter(_ s: String, _ pos: Int) -> (char: String.SubSequence, range: NSRange)? {
|
||||
if pos < 0 || pos >= s.count { return nil }
|
||||
let r = NSRange(location: pos, length: 1)
|
||||
|
||||
+6
-9
@@ -133,15 +133,12 @@ data class ComposeState(
|
||||
)
|
||||
|
||||
val memberMentions: Map<String, Long>
|
||||
get() = this.mentions.mapNotNull {
|
||||
val memberRef = it.value.memberRef
|
||||
|
||||
if (memberRef != null) {
|
||||
it.key to memberRef.groupMemberId
|
||||
} else {
|
||||
null
|
||||
}
|
||||
}.toMap()
|
||||
get() = parsedMessage
|
||||
.mapNotNull { (it.format as? Format.Mention)?.memberName }
|
||||
.distinct()
|
||||
.mapNotNull { name -> mentions[name]?.memberRef?.groupMemberId?.let { name to it } }
|
||||
.take(MAX_NUMBER_OF_MENTIONS)
|
||||
.toMap()
|
||||
|
||||
val editing: Boolean
|
||||
get() =
|
||||
|
||||
+3
-19
@@ -102,7 +102,6 @@ fun GroupMentions(
|
||||
}
|
||||
|
||||
fun messageChanged(msg: ComposeMessage, parsedMsg: List<FormattedText>) {
|
||||
removeUnusedMentions(composeState, parsedMsg)
|
||||
val selected = selectedMarkdown(parsedMsg, msg.selection)
|
||||
|
||||
if (selected != null) {
|
||||
@@ -113,7 +112,7 @@ fun GroupMentions(
|
||||
isVisible.value = true
|
||||
mentionName.value = ft.format.memberName
|
||||
mentionRange.value = r
|
||||
mentionMemberId.value = composeState.value.mentions[mentionName.value]?.memberId
|
||||
mentionMemberId.value = if (mentionName.value in composeState.value.memberMentions) composeState.value.mentions[mentionName.value]?.memberId else null
|
||||
if (!chatModel.membersLoaded.value) {
|
||||
scope.launch {
|
||||
setGroupMembers(rhId, chatInfo.groupInfo, chatModel)
|
||||
@@ -211,7 +210,7 @@ fun GroupMentions(
|
||||
},
|
||||
contentAlignment = Alignment.BottomStart
|
||||
) {
|
||||
val showMaxReachedBox = composeState.value.mentions.size >= MAX_NUMBER_OF_MENTIONS && isVisible.value && composeState.value.mentions[mentionName.value] == null
|
||||
val showMaxReachedBox = composeState.value.memberMentions.size >= MAX_NUMBER_OF_MENTIONS && isVisible.value && mentionName.value !in composeState.value.memberMentions
|
||||
LazyColumnWithScrollBarNoAppBar(
|
||||
Modifier
|
||||
.heightIn(max = MAX_PICKER_HEIGHT)
|
||||
@@ -229,7 +228,7 @@ fun GroupMentions(
|
||||
Divider()
|
||||
}
|
||||
val mentioned = mentionMemberId.value == member.memberId
|
||||
val disabled = composeState.value.mentions.size >= MAX_NUMBER_OF_MENTIONS && !mentioned
|
||||
val disabled = composeState.value.memberMentions.size >= MAX_NUMBER_OF_MENTIONS && !mentioned
|
||||
Row(
|
||||
Modifier
|
||||
.fillMaxWidth()
|
||||
@@ -309,18 +308,3 @@ private fun selectedMarkdown(
|
||||
parsedMsg[i] to TextRange(pos, pos + parsedMsg[i].text.length)
|
||||
}
|
||||
}
|
||||
|
||||
private fun removeUnusedMentions(composeState: MutableState<ComposeState>, parsedMsg: List<FormattedText>) {
|
||||
val usedMentions = parsedMsg.mapNotNull { ft ->
|
||||
when (ft.format) {
|
||||
is Format.Mention -> ft.format.memberName
|
||||
else -> null
|
||||
}
|
||||
}.toSet()
|
||||
|
||||
if (usedMentions.size < composeState.value.mentions.size) {
|
||||
composeState.value = composeState.value.copy(
|
||||
mentions = composeState.value.mentions.filterKeys { it in usedMentions }
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,107 @@
|
||||
# Fix "error store invalidMention" and preserve mention bindings across in-place edits
|
||||
|
||||
## Symptom
|
||||
|
||||
Rare "Error sending message: error store invalidMention" when sending a group
|
||||
message that contains an @mention.
|
||||
|
||||
## Root cause
|
||||
|
||||
When the user picks a member from the picker, the client inserts `@Name` into
|
||||
the message text and records `mentions[Name] = memberId` in the compose
|
||||
state. The original `removeUnusedMentions` only pruned that map when the
|
||||
parsed `@name` count was **strictly less** than the map size:
|
||||
|
||||
```kotlin
|
||||
if (usedMentions.size < composeState.value.mentions.size) { ... }
|
||||
```
|
||||
|
||||
If the user **edits the inserted token in place** (e.g. fixes a typo, deletes
|
||||
one character) without re-picking from the picker, the parser sees one
|
||||
mention with the new name while the map still contains the old key. Both
|
||||
have size 1, so the guard does not fire and the stale entry is sent. The
|
||||
core's `getCIMentions` (`Internal.hs:267-271`) requires every key in the
|
||||
client-supplied mentions map to also appear as a parsed `@name` token in the
|
||||
message text, and throws `SEInvalidMention` otherwise.
|
||||
|
||||
A naive fix (prune whenever any key is missing from the parsed names) stops
|
||||
the crash but also drops the binding the instant a letter is removed — so
|
||||
typing the letter back leaves an unresolved `@name` rather than a real
|
||||
mention.
|
||||
|
||||
## Approach
|
||||
|
||||
Stop mutating the compose-state mentions map while editing. Treat the map
|
||||
as a sticky cache of `name → memberId` bindings recorded by the picker, and
|
||||
filter against the currently-parsed text only at the points where a "current
|
||||
set of mentions" is actually needed: sending, picker UX, name
|
||||
disambiguation. This both eliminates the `SEInvalidMention` failure and
|
||||
lets a round-trip edit (`@Usernama` → `@Usernam` → `@Usernama`) re-resolve
|
||||
the original member.
|
||||
|
||||
## Changes
|
||||
|
||||
### 1. Drop `removeUnusedMentions`
|
||||
|
||||
- `apps/multiplatform/.../GroupMentions.kt`: delete the function and its call
|
||||
in `messageChanged`.
|
||||
- `apps/ios/.../GroupMentions.swift`: same.
|
||||
|
||||
### 2. Filter at send time via `memberMentions`
|
||||
|
||||
`ComposeState.memberMentions` intersects the cached map with the names
|
||||
currently parsed in `parsedMessage`. All sends already route through this
|
||||
getter, so this is the single chokepoint that determines what reaches the
|
||||
core.
|
||||
|
||||
### 3. Picker "max reached" uses parsed-mention count
|
||||
|
||||
`GroupMentions.kt:213,231` and the Swift equivalent at `GroupMentions.swift:50`
|
||||
switch from `composeState.mentions.size` to a count of `Format.Mention`
|
||||
entries in `parsedMessage`. This keeps the limit aligned with what the
|
||||
server will see, regardless of how many stale entries the cache holds.
|
||||
|
||||
### 4. `mentionMemberName` disambiguation uses parsed names
|
||||
|
||||
`ComposeView.kt`'s `mentionMemberName` walks the set of parsed mention names
|
||||
instead of `mentions.containsKey`. Same for Swift. A stale cache entry no
|
||||
longer pushes a fresh pick to an awkward `_1` suffix, and a picker tap on a
|
||||
broken `@alic` cleanly replaces it with `@alice`.
|
||||
|
||||
### 5. Editing-an-existing-item path
|
||||
|
||||
`ComposeState(editingItem, ...)` seeds `mentions = editingItem.mentions`.
|
||||
That stays as-is — it is already a sticky cache and the new `memberMentions`
|
||||
getter handles filtering.
|
||||
|
||||
## Manual reproduction (original bug)
|
||||
|
||||
1. In a group, type `@` and pick a member. Text becomes `@Name ` and the
|
||||
mentions map gets `{Name → memberId}`.
|
||||
2. Place the cursor inside the inserted name and add or delete one character
|
||||
(`@Nam`, `@Names`, etc.) — do not use the picker.
|
||||
3. Tap send. Before the fix: send fails with `error store invalidMention`.
|
||||
After the fix: message is sent.
|
||||
|
||||
## Edge cases to test
|
||||
|
||||
- Pick `Usernama`, delete last `a`, retype it → send: mention resolves to
|
||||
the original member (restoration behaviour).
|
||||
- Pick `Usernama`, delete the whole token → send: no mention. Cache still
|
||||
holds a stale `Usernama` entry but `memberMentions` is empty.
|
||||
- Pick `alice` (member A), delete the `@alice` text, then pick a different
|
||||
`alice` (member B) → inserted as `@alice` (the cleaner, visible name);
|
||||
cache key rebinds A→B as an explicit user action.
|
||||
- Two members with same display name, pick A, edit `@alice` → `@alic` →
|
||||
`@alice` by typing only → A's binding is restored (typing never mutates
|
||||
the cache; only picker taps do).
|
||||
- Pick 3 members, delete one of the tokens — picker should not show
|
||||
"max reached"; only 2 mentions are now in the text.
|
||||
- Edit an existing sent message: original mentions render, surviving edits
|
||||
re-bind, removed ones disappear from `memberMentions` on send.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- No core changes. `getCIMentions` (`Internal.hs:267-271`) stays as-is;
|
||||
this is purely client-side cache lifecycle.
|
||||
- No change to the @-picker trigger heuristic in `messageChanged`.
|
||||
Reference in New Issue
Block a user