Skip to content

Commit fd45cf8

Browse files
committed
fix(editor): scroll jumps to end after IME commit (Pinyin / Rime)
1 parent d038f50 commit fd45cf8

4 files changed

Lines changed: 156 additions & 10 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1313

1414
### Fixed
1515

16+
- The SQL editor jumped to the end of the document after committing a Chinese (or any IME-marked) word like "测试". `TextView.insertText(_:replacementRange:)` called `unmarkText()` first, which wiped the marked text via `replaceCharacters(_:with: "")`, then re-ran `replaceCharacters` with the same range AppKit had supplied. By then that range was stale. For mid-document marked text it ate the next characters; for end-of-document marked text the second range was out of bounds and the cursor landed at `documentLength` (the visible "scroll to end"). The implementation now resolves the effective range(s) before clearing marked-text bookkeeping and performs a single edit, with a multi-cursor IME path that replaces every marked range in one pass. While here, `TextSelectionManager.didReplaceCharacters` had a latent off-by-N in its delta calculation that the old unmark-then-insert flow happened to mask; same-length replaces now leave selections past the replace point unchanged. Fixes #1012.
1617
- SSH auth-failure alerts now point at the actual cause. The catch-all "Check your credentials or private key." string was wrong for the most common 2FA case (typing a wrong TOTP code), because the credentials were fine; only the verification code was wrong. `SSHTunnelError.authenticationFailed` now carries an `AuthFailureReason` (`password`, `verificationCode`, `privateKey`, `agentRejected`, `generic`), every throw site picks the right one, and the alert text matches: "Verification code rejected. Get a new code from your authenticator app and try again." for kbd-int + TOTP rejections, "SSH password rejected. Check the password and try again." for plain password rejections, and so on. Follow-up to #1005.
1718
- TOTP codes are now fetched lazily from the `TOTPProvider` inside the kbd-int callback rather than once upfront before authentication starts. Fixes two related issues: (1) when the kbd-int handshake straddled a 30-second TOTP rotation boundary, the `AutoTOTPProvider` code that was valid at fetch time had expired by the time PAM validated it; (2) when the server retried after a wrong code (sshd defaults to 3 prompts per session), TablePro replayed the same wrong code instead of asking the user for a new one. `PromptTOTPProvider` now switches its NSAlert wording to "Verification Code Rejected. The previous code wasn't accepted. Wait for your authenticator to refresh, then enter the new code." on retries, matching how OpenSSH re-prompts.
1819
- SSH Auth Method = Password failed against servers that only advertise `keyboard-interactive` (the typical `pam_google_authenticator` setup, where `sshd_config` has `AuthenticationMethods keyboard-interactive`). The bare `userauth_password` request the server doesn't accept was the only attempt, so connection failed even when the user typed the right password. `buildAuthenticator` now always pairs `PasswordAuthenticator` with a `KeyboardInteractiveAuthenticator` that reuses the same password, matching OpenSSH and Sequel Ace's "save the password, the server may also prompt for a code" flow. Follow-up to #1005.

LocalPackages/CodeEditTextView/Sources/CodeEditTextView/TextSelectionManager/TextSelectionManager+Update.swift

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,11 @@ import Foundation
99

1010
extension TextSelectionManager {
1111
public func didReplaceCharacters(in range: NSRange, replacementLength: Int) {
12-
let delta = replacementLength == 0 ? -range.length : replacementLength
12+
// Net shift = chars added - chars removed. Selections past `range.max` move by this delta.
13+
// The previous formula short-circuited to `replacementLength` when non-zero, which dropped
14+
// the chars-removed term and over-shifted selections after a same-length replace (e.g. the
15+
// multi-cursor IME path replaces each marked range char-for-char).
16+
let delta = replacementLength - range.length
1317
for textSelection in self.textSelections {
1418
if textSelection.range.location > range.max {
1519
textSelection.range.location = max(0, textSelection.range.location + delta)

LocalPackages/CodeEditTextView/Sources/CodeEditTextView/TextView/TextView+NSTextInput.swift

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,23 +42,28 @@ extension TextView: NSTextInputClient {
4242
}
4343
}
4444

45-
/// Inserts the string at the replacement range. If replacement range is `NSNotFound`, uses the selection ranges.
46-
private func _insertText(insertString: String, replacementRange: NSRange) {
45+
/// Inserts the string at the replacement ranges. Handles CR → CRLF promotion based on the
46+
/// document's detected line ending.
47+
private func _insertText(insertString: String, replacementRanges: [NSRange]) {
4748
var insertString = insertString
4849
if LineEnding(rawValue: insertString) == .carriageReturn
4950
&& layoutManager.detectedLineEnding == .carriageReturnLineFeed {
5051
insertString = LineEnding.carriageReturnLineFeed.rawValue
5152
}
5253

53-
if replacementRange.location == NSNotFound {
54-
replaceCharacters(in: selectionManager.textSelections.map(\.range), with: insertString)
55-
} else {
56-
replaceCharacters(in: replacementRange, with: insertString)
57-
}
54+
replaceCharacters(in: replacementRanges, with: insertString)
5855

5956
selectionManager.textSelections.forEach { $0.suggestedXPos = nil }
6057
}
6158

59+
/// Inserts the string at the replacement range. If replacement range is `NSNotFound`, uses the selection ranges.
60+
private func _insertText(insertString: String, replacementRange: NSRange) {
61+
let ranges: [NSRange] = replacementRange.location == NSNotFound
62+
? selectionManager.textSelections.map(\.range)
63+
: [replacementRange]
64+
_insertText(insertString: insertString, replacementRanges: ranges)
65+
}
66+
6267
/// Inserts the given string into the receiver, replacing the specified content.
6368
///
6469
/// Programmatic modification of the text is best done by operating on the text storage directly.
@@ -68,10 +73,38 @@ extension TextView: NSTextInputClient {
6873
/// - Parameters:
6974
/// - string: The text to insert, either an NSString or NSAttributedString instance.
7075
/// - replacementRange: The range of content to replace in the receiver’s text storage.
76+
///
77+
/// IME commits arrive here with `replacementRange` either set to `NSNotFound` (replace the
78+
/// marked range(s)) or to the explicit range to replace. Calling `unmarkText()` first would
79+
/// wipe the marked text via `replaceCharacters(_:with: "")` and then `_insertText` would
80+
/// replace a now-stale range, either eating unrelated content or hitting an out-of-bounds
81+
/// range that lands the caret at `documentLength`. Resolve the effective range(s) first,
82+
/// then clear marked-text bookkeeping without touching the storage, so the actual replacement
83+
/// is a single edit. Multi-cursor IME duplicates the marked range across every cursor; the
84+
/// `NSNotFound` path replaces all of them in one pass.
7185
@objc public func insertText(_ string: Any, replacementRange: NSRange) {
7286
guard isEditable, let insertString = anyToString(string) else { return }
73-
unmarkText()
74-
_insertText(insertString: insertString, replacementRange: replacementRange)
87+
88+
let markedRanges = layoutManager.markedTextManager.markedRanges
89+
let hadMarkedText = !markedRanges.isEmpty
90+
91+
if hadMarkedText {
92+
layoutManager.markedTextManager.removeAll()
93+
layoutManager.setNeedsLayout()
94+
needsLayout = true
95+
inputContext?.discardMarkedText()
96+
}
97+
98+
if replacementRange.location != NSNotFound {
99+
_insertText(insertString: insertString, replacementRange: replacementRange)
100+
} else if hadMarkedText {
101+
_insertText(insertString: insertString, replacementRanges: markedRanges)
102+
} else {
103+
_insertText(
104+
insertString: insertString,
105+
replacementRange: NSRange(location: NSNotFound, length: 0)
106+
)
107+
}
75108
}
76109

77110
override public func insertText(_ insertString: Any) {
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
import Testing
2+
import AppKit
3+
@testable import CodeEditTextView
4+
5+
/// Regression tests for IME commits (Pinyin / Rime / any system that uses marked text).
6+
///
7+
/// `TextView.insertText(_:replacementRange:)` used to call `unmarkText()` first and then
8+
/// `_insertText` with the same `replacementRange` AppKit had supplied, which by then pointed
9+
/// at characters that no longer existed because `unmarkText` had already shrunk the document.
10+
/// The result was either content corruption (range still in bounds, but pointing at the wrong
11+
/// chars) or a cursor that landed at `documentLength` after a clamped out-of-bounds replace —
12+
/// the latter is what users see as "scrolls to end of script after typing a Chinese word".
13+
/// See TableProApp/TablePro#1012.
14+
@Suite
15+
@MainActor
16+
struct IMEInputTests {
17+
private func makeLaidOutTextView(_ text: String) -> TextView {
18+
let textView = TextView(string: text)
19+
textView.frame = NSRect(x: 0, y: 0, width: 1_000, height: 1_000)
20+
textView.updateFrameIfNeeded()
21+
textView.layoutManager.layoutLines(in: NSRect(x: 0, y: 0, width: 1_000, height: 1_000))
22+
return textView
23+
}
24+
25+
/// Builds marked text "ceshi" character-by-character at the current selection,
26+
/// matching how an IME progressively shows the in-progress romaji string.
27+
private func typeMarkedCeshi(on textView: TextView) {
28+
for (index, segment) in ["c", "ce", "ces", "cesh", "ceshi"].enumerated() {
29+
textView.setMarkedText(
30+
segment,
31+
selectedRange: NSRange(location: index + 1, length: 0),
32+
replacementRange: NSRange(location: NSNotFound, length: 0)
33+
)
34+
}
35+
}
36+
37+
@Test("IME commit on an empty middle line preserves surrounding content")
38+
func imeCommitInTheMiddleDoesNotCorruptText() throws {
39+
let textView = makeLaidOutTextView("alpha\n\nbeta")
40+
textView.selectionManager.setSelectedRange(NSRange(location: 6, length: 0))
41+
42+
typeMarkedCeshi(on: textView)
43+
44+
// After typing five characters of marked text starting at offset 6, the IME owns
45+
// the range (6, 5) and may pass it as `replacementRange` at commit.
46+
textView.insertText("测试", replacementRange: NSRange(location: 6, length: 5))
47+
48+
#expect(textView.string == "alpha\n测试\nbeta")
49+
let caret = try #require(textView.selectionManager.textSelections.first)
50+
#expect(caret.range == NSRange(location: 8, length: 0))
51+
}
52+
53+
@Test("IME commit at end of document keeps caret at the inserted text, not at length")
54+
func imeCommitAtEndKeepsCaretAtInsertedText() throws {
55+
let textView = makeLaidOutTextView("alpha")
56+
textView.selectionManager.setSelectedRange(NSRange(location: 5, length: 0))
57+
58+
typeMarkedCeshi(on: textView)
59+
textView.insertText("测试", replacementRange: NSRange(location: 5, length: 5))
60+
61+
#expect(textView.string == "alpha测试")
62+
let caret = try #require(textView.selectionManager.textSelections.first)
63+
#expect(caret.range == NSRange(location: 7, length: 0))
64+
}
65+
66+
@Test("IME commit with NSNotFound replacement range still inserts at the marked range")
67+
func imeCommitWithNotFoundReplacementRange() throws {
68+
let textView = makeLaidOutTextView("alpha\n\nbeta")
69+
textView.selectionManager.setSelectedRange(NSRange(location: 6, length: 0))
70+
71+
typeMarkedCeshi(on: textView)
72+
textView.insertText(
73+
"测试",
74+
replacementRange: NSRange(location: NSNotFound, length: 0)
75+
)
76+
77+
#expect(textView.string == "alpha\n测试\nbeta")
78+
let caret = try #require(textView.selectionManager.textSelections.first)
79+
#expect(caret.range == NSRange(location: 8, length: 0))
80+
}
81+
82+
@Test("Marked-text state is cleared after a commit")
83+
func markedTextStateClearedAfterCommit() {
84+
let textView = makeLaidOutTextView("alpha\n\nbeta")
85+
textView.selectionManager.setSelectedRange(NSRange(location: 6, length: 0))
86+
87+
typeMarkedCeshi(on: textView)
88+
textView.insertText("测试", replacementRange: NSRange(location: 6, length: 5))
89+
90+
#expect(textView.hasMarkedText() == false)
91+
#expect(textView.markedRange().location == NSNotFound)
92+
}
93+
94+
@Test("Plain Latin insertText path is unaffected")
95+
func plainInsertTextIsUnaffected() {
96+
let textView = makeLaidOutTextView("alpha")
97+
textView.selectionManager.setSelectedRange(NSRange(location: 5, length: 0))
98+
99+
// No setMarkedText calls — this is the non-IME path.
100+
textView.insertText(
101+
" beta",
102+
replacementRange: NSRange(location: NSNotFound, length: 0)
103+
)
104+
105+
#expect(textView.string == "alpha beta")
106+
#expect(textView.selectionManager.textSelections.first?.range == NSRange(location: 10, length: 0))
107+
}
108+
}

0 commit comments

Comments
 (0)