From fd45cf8544cb064393d1e65d6af8b3f67fe1a65d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ng=C3=B4=20Qu=E1=BB=91c=20=C4=90=E1=BA=A1t?= Date: Wed, 6 May 2026 11:49:22 +0700 Subject: [PATCH] fix(editor): scroll jumps to end after IME commit (Pinyin / Rime) --- CHANGELOG.md | 1 + .../TextSelectionManager+Update.swift | 6 +- .../TextView/TextView+NSTextInput.swift | 51 +++++++-- .../CodeEditTextViewTests/IMEInputTests.swift | 108 ++++++++++++++++++ 4 files changed, 156 insertions(+), 10 deletions(-) create mode 100644 LocalPackages/CodeEditTextView/Tests/CodeEditTextViewTests/IMEInputTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 2984195f7..705a931ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- 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. - 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. - 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. - 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. diff --git a/LocalPackages/CodeEditTextView/Sources/CodeEditTextView/TextSelectionManager/TextSelectionManager+Update.swift b/LocalPackages/CodeEditTextView/Sources/CodeEditTextView/TextSelectionManager/TextSelectionManager+Update.swift index 0c319681d..f7c51faa2 100644 --- a/LocalPackages/CodeEditTextView/Sources/CodeEditTextView/TextSelectionManager/TextSelectionManager+Update.swift +++ b/LocalPackages/CodeEditTextView/Sources/CodeEditTextView/TextSelectionManager/TextSelectionManager+Update.swift @@ -9,7 +9,11 @@ import Foundation extension TextSelectionManager { public func didReplaceCharacters(in range: NSRange, replacementLength: Int) { - let delta = replacementLength == 0 ? -range.length : replacementLength + // Net shift = chars added - chars removed. Selections past `range.max` move by this delta. + // The previous formula short-circuited to `replacementLength` when non-zero, which dropped + // the chars-removed term and over-shifted selections after a same-length replace (e.g. the + // multi-cursor IME path replaces each marked range char-for-char). + let delta = replacementLength - range.length for textSelection in self.textSelections { if textSelection.range.location > range.max { textSelection.range.location = max(0, textSelection.range.location + delta) diff --git a/LocalPackages/CodeEditTextView/Sources/CodeEditTextView/TextView/TextView+NSTextInput.swift b/LocalPackages/CodeEditTextView/Sources/CodeEditTextView/TextView/TextView+NSTextInput.swift index d64a5a0b3..c47fcc5d8 100644 --- a/LocalPackages/CodeEditTextView/Sources/CodeEditTextView/TextView/TextView+NSTextInput.swift +++ b/LocalPackages/CodeEditTextView/Sources/CodeEditTextView/TextView/TextView+NSTextInput.swift @@ -42,23 +42,28 @@ extension TextView: NSTextInputClient { } } - /// Inserts the string at the replacement range. If replacement range is `NSNotFound`, uses the selection ranges. - private func _insertText(insertString: String, replacementRange: NSRange) { + /// Inserts the string at the replacement ranges. Handles CR → CRLF promotion based on the + /// document's detected line ending. + private func _insertText(insertString: String, replacementRanges: [NSRange]) { var insertString = insertString if LineEnding(rawValue: insertString) == .carriageReturn && layoutManager.detectedLineEnding == .carriageReturnLineFeed { insertString = LineEnding.carriageReturnLineFeed.rawValue } - if replacementRange.location == NSNotFound { - replaceCharacters(in: selectionManager.textSelections.map(\.range), with: insertString) - } else { - replaceCharacters(in: replacementRange, with: insertString) - } + replaceCharacters(in: replacementRanges, with: insertString) selectionManager.textSelections.forEach { $0.suggestedXPos = nil } } + /// Inserts the string at the replacement range. If replacement range is `NSNotFound`, uses the selection ranges. + private func _insertText(insertString: String, replacementRange: NSRange) { + let ranges: [NSRange] = replacementRange.location == NSNotFound + ? selectionManager.textSelections.map(\.range) + : [replacementRange] + _insertText(insertString: insertString, replacementRanges: ranges) + } + /// Inserts the given string into the receiver, replacing the specified content. /// /// Programmatic modification of the text is best done by operating on the text storage directly. @@ -68,10 +73,38 @@ extension TextView: NSTextInputClient { /// - Parameters: /// - string: The text to insert, either an NSString or NSAttributedString instance. /// - replacementRange: The range of content to replace in the receiver’s text storage. + /// + /// IME commits arrive here with `replacementRange` either set to `NSNotFound` (replace the + /// marked range(s)) or to the explicit range to replace. Calling `unmarkText()` first would + /// wipe the marked text via `replaceCharacters(_:with: "")` and then `_insertText` would + /// replace a now-stale range, either eating unrelated content or hitting an out-of-bounds + /// range that lands the caret at `documentLength`. Resolve the effective range(s) first, + /// then clear marked-text bookkeeping without touching the storage, so the actual replacement + /// is a single edit. Multi-cursor IME duplicates the marked range across every cursor; the + /// `NSNotFound` path replaces all of them in one pass. @objc public func insertText(_ string: Any, replacementRange: NSRange) { guard isEditable, let insertString = anyToString(string) else { return } - unmarkText() - _insertText(insertString: insertString, replacementRange: replacementRange) + + let markedRanges = layoutManager.markedTextManager.markedRanges + let hadMarkedText = !markedRanges.isEmpty + + if hadMarkedText { + layoutManager.markedTextManager.removeAll() + layoutManager.setNeedsLayout() + needsLayout = true + inputContext?.discardMarkedText() + } + + if replacementRange.location != NSNotFound { + _insertText(insertString: insertString, replacementRange: replacementRange) + } else if hadMarkedText { + _insertText(insertString: insertString, replacementRanges: markedRanges) + } else { + _insertText( + insertString: insertString, + replacementRange: NSRange(location: NSNotFound, length: 0) + ) + } } override public func insertText(_ insertString: Any) { diff --git a/LocalPackages/CodeEditTextView/Tests/CodeEditTextViewTests/IMEInputTests.swift b/LocalPackages/CodeEditTextView/Tests/CodeEditTextViewTests/IMEInputTests.swift new file mode 100644 index 000000000..cca0b027e --- /dev/null +++ b/LocalPackages/CodeEditTextView/Tests/CodeEditTextViewTests/IMEInputTests.swift @@ -0,0 +1,108 @@ +import Testing +import AppKit +@testable import CodeEditTextView + +/// Regression tests for IME commits (Pinyin / Rime / any system that uses marked text). +/// +/// `TextView.insertText(_:replacementRange:)` used to call `unmarkText()` first and then +/// `_insertText` with the same `replacementRange` AppKit had supplied, which by then pointed +/// at characters that no longer existed because `unmarkText` had already shrunk the document. +/// The result was either content corruption (range still in bounds, but pointing at the wrong +/// chars) or a cursor that landed at `documentLength` after a clamped out-of-bounds replace — +/// the latter is what users see as "scrolls to end of script after typing a Chinese word". +/// See TableProApp/TablePro#1012. +@Suite +@MainActor +struct IMEInputTests { + private func makeLaidOutTextView(_ text: String) -> TextView { + let textView = TextView(string: text) + textView.frame = NSRect(x: 0, y: 0, width: 1_000, height: 1_000) + textView.updateFrameIfNeeded() + textView.layoutManager.layoutLines(in: NSRect(x: 0, y: 0, width: 1_000, height: 1_000)) + return textView + } + + /// Builds marked text "ceshi" character-by-character at the current selection, + /// matching how an IME progressively shows the in-progress romaji string. + private func typeMarkedCeshi(on textView: TextView) { + for (index, segment) in ["c", "ce", "ces", "cesh", "ceshi"].enumerated() { + textView.setMarkedText( + segment, + selectedRange: NSRange(location: index + 1, length: 0), + replacementRange: NSRange(location: NSNotFound, length: 0) + ) + } + } + + @Test("IME commit on an empty middle line preserves surrounding content") + func imeCommitInTheMiddleDoesNotCorruptText() throws { + let textView = makeLaidOutTextView("alpha\n\nbeta") + textView.selectionManager.setSelectedRange(NSRange(location: 6, length: 0)) + + typeMarkedCeshi(on: textView) + + // After typing five characters of marked text starting at offset 6, the IME owns + // the range (6, 5) and may pass it as `replacementRange` at commit. + textView.insertText("测试", replacementRange: NSRange(location: 6, length: 5)) + + #expect(textView.string == "alpha\n测试\nbeta") + let caret = try #require(textView.selectionManager.textSelections.first) + #expect(caret.range == NSRange(location: 8, length: 0)) + } + + @Test("IME commit at end of document keeps caret at the inserted text, not at length") + func imeCommitAtEndKeepsCaretAtInsertedText() throws { + let textView = makeLaidOutTextView("alpha") + textView.selectionManager.setSelectedRange(NSRange(location: 5, length: 0)) + + typeMarkedCeshi(on: textView) + textView.insertText("测试", replacementRange: NSRange(location: 5, length: 5)) + + #expect(textView.string == "alpha测试") + let caret = try #require(textView.selectionManager.textSelections.first) + #expect(caret.range == NSRange(location: 7, length: 0)) + } + + @Test("IME commit with NSNotFound replacement range still inserts at the marked range") + func imeCommitWithNotFoundReplacementRange() throws { + let textView = makeLaidOutTextView("alpha\n\nbeta") + textView.selectionManager.setSelectedRange(NSRange(location: 6, length: 0)) + + typeMarkedCeshi(on: textView) + textView.insertText( + "测试", + replacementRange: NSRange(location: NSNotFound, length: 0) + ) + + #expect(textView.string == "alpha\n测试\nbeta") + let caret = try #require(textView.selectionManager.textSelections.first) + #expect(caret.range == NSRange(location: 8, length: 0)) + } + + @Test("Marked-text state is cleared after a commit") + func markedTextStateClearedAfterCommit() { + let textView = makeLaidOutTextView("alpha\n\nbeta") + textView.selectionManager.setSelectedRange(NSRange(location: 6, length: 0)) + + typeMarkedCeshi(on: textView) + textView.insertText("测试", replacementRange: NSRange(location: 6, length: 5)) + + #expect(textView.hasMarkedText() == false) + #expect(textView.markedRange().location == NSNotFound) + } + + @Test("Plain Latin insertText path is unaffected") + func plainInsertTextIsUnaffected() { + let textView = makeLaidOutTextView("alpha") + textView.selectionManager.setSelectedRange(NSRange(location: 5, length: 0)) + + // No setMarkedText calls — this is the non-IME path. + textView.insertText( + " beta", + replacementRange: NSRange(location: NSNotFound, length: 0) + ) + + #expect(textView.string == "alpha beta") + #expect(textView.selectionManager.textSelections.first?.range == NSRange(location: 10, length: 0)) + } +}