From 8fc5407214746486a74199d134bbc785eb5456eb Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 21 Aug 2026 22:36:49 +0700 Subject: [PATCH] fix(editor): resolve the ranges input services send against the document before using them --- CHANGELOG.md | 1 + .../Extensions/NSRange+/NSRange+clamped.swift | 18 ++ .../MarkedTextManager/MarkedTextManager.swift | 19 ++ .../TextView/TextView+NSTextInput.swift | 64 ++++- .../NSTextInputRangeGuardTests.swift | 242 ++++++++++++++++++ 5 files changed, 336 insertions(+), 8 deletions(-) create mode 100644 LocalPackages/CodeEditTextView/Tests/CodeEditTextViewTests/NSTextInputRangeGuardTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 4952f80cc..1d63020df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fixed a crash on macOS 26 and later when the editor redrew a diagnostic underline or search highlight whose text had been edited away. +- Fixed a crash when an input method, dictation or Look Up asked the editor about text that had already been edited away. (#2339) - The XLSX, MQL and SQL Import plugins linked to a documentation page that did not exist. They now point at Import & Export. ## [0.67.0] - 2026-08-21 diff --git a/LocalPackages/CodeEditTextView/Sources/CodeEditTextView/Extensions/NSRange+/NSRange+clamped.swift b/LocalPackages/CodeEditTextView/Sources/CodeEditTextView/Extensions/NSRange+/NSRange+clamped.swift index 241614a97..527b9f0b6 100644 --- a/LocalPackages/CodeEditTextView/Sources/CodeEditTextView/Extensions/NSRange+/NSRange+clamped.swift +++ b/LocalPackages/CodeEditTextView/Sources/CodeEditTextView/Extensions/NSRange+/NSRange+clamped.swift @@ -16,4 +16,22 @@ extension NSRange { let end = Swift.min(Swift.max(self.max, 0), length) return NSRange(location: start, length: Swift.max(0, end - start)) } + + /// Returns the range resolved against a document of `length`, or `nil` when it names no + /// position in that document. + /// + /// Use this for a range that came from outside the text view: an input service, an + /// accessibility client, or state stored before an edit. Those may send `NSNotFound`, a + /// negative value, or a length that overflows when added to the location, none of which + /// ``clamped(toLength:)`` can move inside the document, and the second of which traps when + /// `max` is computed. + func resolved(inDocumentOfLength length: Int) -> NSRange? { + guard location != NSNotFound, + location >= 0, + self.length >= 0, + location <= Int.max - self.length else { + return nil + } + return clamped(toLength: length) + } } diff --git a/LocalPackages/CodeEditTextView/Sources/CodeEditTextView/MarkedTextManager/MarkedTextManager.swift b/LocalPackages/CodeEditTextView/Sources/CodeEditTextView/MarkedTextManager/MarkedTextManager.swift index 5270dce8d..31122749c 100644 --- a/LocalPackages/CodeEditTextView/Sources/CodeEditTextView/MarkedTextManager/MarkedTextManager.swift +++ b/LocalPackages/CodeEditTextView/Sources/CodeEditTextView/MarkedTextManager/MarkedTextManager.swift @@ -27,6 +27,25 @@ class MarkedTextManager { markedRanges.removeAll() } + /// Resolves the stored ranges against a document of `length`, dropping any that name no + /// position in it and collapsing any that resolve to the same one. + /// + /// An input session spans many callbacks and this object has no hook for edits made through + /// any other path, so an edit that shrinks the document leaves its ranges behind. Resolving + /// before they are used keeps the next keystroke of a composition computed against text that + /// still exists, rather than replacing at a position the document no longer has. + func resolveRanges(inDocumentOfLength length: Int) { + var resolved: [NSRange] = [] + for range in markedRanges { + guard let clamped = range.resolved(inDocumentOfLength: length), + !resolved.contains(clamped) else { + continue + } + resolved.append(clamped) + } + markedRanges = resolved + } + /// Updates the stored marked ranges. /// /// Two cases here: diff --git a/LocalPackages/CodeEditTextView/Sources/CodeEditTextView/TextView/TextView+NSTextInput.swift b/LocalPackages/CodeEditTextView/Sources/CodeEditTextView/TextView/TextView+NSTextInput.swift index c47fcc5d8..d53871aa2 100644 --- a/LocalPackages/CodeEditTextView/Sources/CodeEditTextView/TextView/TextView+NSTextInput.swift +++ b/LocalPackages/CodeEditTextView/Sources/CodeEditTextView/TextView/TextView+NSTextInput.swift @@ -51,7 +51,7 @@ extension TextView: NSTextInputClient { insertString = LineEnding.carriageReturnLineFeed.rawValue } - replaceCharacters(in: replacementRanges, with: insertString) + replaceCharacters(in: resolvedReplacementRanges(replacementRanges), with: insertString) selectionManager.textSelections.forEach { $0.suggestedXPos = nil } } @@ -85,6 +85,7 @@ extension TextView: NSTextInputClient { @objc public func insertText(_ string: Any, replacementRange: NSRange) { guard isEditable, let insertString = anyToString(string) else { return } + layoutManager.markedTextManager.resolveRanges(inDocumentOfLength: textStorage.length) let markedRanges = layoutManager.markedTextManager.markedRanges let hadMarkedText = !markedRanges.isEmpty @@ -126,6 +127,7 @@ extension TextView: NSTextInputClient { guard isEditable, let insertString = anyToString(string) else { return } // Needs to insert text, but not notify the undo manager. _undoManager?.disable() + layoutManager.markedTextManager.resolveRanges(inDocumentOfLength: textStorage.length) let shouldInsert = layoutManager.markedTextManager.markedRanges.isEmpty // Copy the text selections *before* we modify them. @@ -172,6 +174,7 @@ extension TextView: NSTextInputClient { @objc public func unmarkText() { if layoutManager.markedTextManager.hasMarkedText { _undoManager?.disable() + layoutManager.markedTextManager.resolveRanges(inDocumentOfLength: textStorage.length) replaceCharacters(in: layoutManager.markedTextManager.markedRanges, with: "") _undoManager?.enable() layoutManager.markedTextManager.removeAll() @@ -238,11 +241,49 @@ extension TextView: NSTextInputClient { forProposedRange range: NSRange, actualRange: NSRangePointer? ) -> NSAttributedString? { - let realRange = (textStorage.string as NSString).rangeOfComposedCharacterSequences(for: range) + actualRange?.pointee = .notFound + guard let realRange = composedRange(forProposedRange: range) else { return nil } actualRange?.pointee = realRange return textStorage.attributedSubstring(from: realRange) } + /// Resolves a range handed to us by an input service against the document. + /// + /// Returns `nil` when the range starts outside the document, which is the case + /// `NSTextInputClient` documents as having no answer. A range that starts inside it keeps its + /// position and gives up only the part that no longer exists, so a caret question still has + /// an answer and an empty result means "no characters there" rather than "no such place". + private func composedRange(forProposedRange range: NSRange) -> NSRange? { + let documentLength = textStorage.length + guard let clamped = range.resolved(inDocumentOfLength: documentLength), + clamped.location == range.location else { + return nil + } + guard !clamped.isEmpty else { return clamped } + return (textStorage.string as NSString).rangeOfComposedCharacterSequences(for: clamped) + } + + /// Resolves the ranges an input service asked us to replace against the current document. + /// + /// The ranges an input session works with are computed against the document as it was, and any + /// edit made through another path can leave them behind: marked-text bookkeeping keeps its own + /// ranges and has no hook for those edits. `replaceCharacters` registers undo before it + /// mutates, and building the inverse slices the storage, so a range that outruns the document + /// traps there rather than raising something catchable. Two ranges that resolve to the same + /// position would replace the same text twice, so they collapse to one. + internal func resolvedReplacementRanges(_ ranges: [NSRange]) -> [NSRange] { + let documentLength = textStorage.length + var resolved: [NSRange] = [] + for range in ranges { + guard let clamped = range.resolved(inDocumentOfLength: documentLength), + !resolved.contains(clamped) else { + continue + } + resolved.append(clamped) + } + return resolved + } + /// Returns an attributed string representing the receiver's text storage. /// - Returns: The attributed string of the receiver’s text storage. @objc public func attributedString() -> NSAttributedString { @@ -259,14 +300,21 @@ extension TextView: NSTextInputClient { /// - Returns: The boundary rectangle for the given range of characters, in *screen* coordinates. /// The rectangle’s size value can be negative if the text flows to the left. @objc public func firstRect(forCharacterRange range: NSRange, actualRange: NSRangePointer?) -> NSRect { - if actualRange != nil { - let realRange = (textStorage.string as NSString).rangeOfComposedCharacterSequences(for: range) - if realRange != range { - actualRange?.pointee = realRange - } + actualRange?.pointee = .notFound + + // A range we cannot resolve still has to produce a usable rect: an input service places + // its candidate window, accent popover or dictation indicator here, and a zero rect puts + // all of them in the corner of the display. The end of the document is where the old code + // landed for those, because `rectForOffset` answers any offset past the end that way. + let offset: Int + if let realRange = composedRange(forProposedRange: range) { + actualRange?.pointee = realRange + offset = realRange.location + } else { + offset = textStorage.length } - let localRect = (layoutManager.rectForOffset(range.location) ?? .zero) + let localRect = (layoutManager.rectForOffset(offset) ?? .zero) let windowRect = convert(localRect, to: nil) return window?.convertToScreen(windowRect) ?? .zero } diff --git a/LocalPackages/CodeEditTextView/Tests/CodeEditTextViewTests/NSTextInputRangeGuardTests.swift b/LocalPackages/CodeEditTextView/Tests/CodeEditTextViewTests/NSTextInputRangeGuardTests.swift new file mode 100644 index 000000000..7b7939efb --- /dev/null +++ b/LocalPackages/CodeEditTextView/Tests/CodeEditTextViewTests/NSTextInputRangeGuardTests.swift @@ -0,0 +1,242 @@ +import AppKit +import Testing +@testable import CodeEditTextView + +/// Input services, accessibility clients and the view's own marked-text bookkeeping all hand the +/// text view ranges computed against a document that may since have changed. `NSString` and +/// `NSTextStorage` raise for those, and the undo manager's inverse-mutation slice traps outright, +/// so every one of these used to end the process rather than return an answer. +@Suite() +struct NSTextInputRangeGuardTests { + @Test() + func resolvingKeepsWhatStillFitsInTheDocument() { + #expect(NSRange(location: 2, length: 99).resolved(inDocumentOfLength: 5) == NSRange(location: 2, length: 3)) + #expect(NSRange(location: 9, length: 3).resolved(inDocumentOfLength: 5) == NSRange(location: 5, length: 0)) + #expect(NSRange(location: 0, length: 5).resolved(inDocumentOfLength: 5) == NSRange(location: 0, length: 5)) + #expect(NSRange(location: 0, length: 1).resolved(inDocumentOfLength: 0) == NSRange(location: 0, length: 0)) + #expect( + NSRange(location: 3, length: 1_000_000).resolved(inDocumentOfLength: 5) + == NSRange(location: 3, length: 2) + ) + } + + @Test() + func resolvingRejectsRangesWithNoPosition() { + #expect(NSRange(location: NSNotFound, length: 0).resolved(inDocumentOfLength: 5) == nil) + #expect(NSRange(location: -1, length: 2).resolved(inDocumentOfLength: 5) == nil) + #expect(NSRange(location: 2, length: -1).resolved(inDocumentOfLength: 5) == nil) + #expect(NSRange(location: Int.max, length: 3).resolved(inDocumentOfLength: 5) == nil) + #expect(NSRange(location: 3, length: Int.max).resolved(inDocumentOfLength: 5) == nil) + } + + /// `NSTextInputClient` reserves nil for a range whose location is outside the document. A + /// location inside it has an answer even when no characters fall in the range, and a client + /// reading the caret's context tells those two apart. + @Test() + @MainActor + func attributedSubstringReturnsNilOnlyWhenTheLocationIsOutsideTheDocument() { + let textView = makeTextView(string: "Hello") + + for proposed in [ + NSRange(location: 10, length: 3), + NSRange(location: NSNotFound, length: 0), + NSRange(location: Int.max, length: 1) + ] { + var actual = NSRange(location: 0, length: 0) + #expect(textView.attributedSubstring(forProposedRange: proposed, actualRange: &actual) == nil) + #expect(actual == .notFound) + } + } + + @Test() + @MainActor + func attributedSubstringAnswersACaretPositionWithAnEmptyString() throws { + let textView = makeTextView(string: "Hello") + + var endOfDocument = NSRange(location: 0, length: 0) + let atEnd = try #require( + textView.attributedSubstring(forProposedRange: NSRange(location: 5, length: 1), actualRange: &endOfDocument) + ) + #expect(atEnd.string.isEmpty) + #expect(endOfDocument == NSRange(location: 5, length: 0)) + + var start = NSRange(location: 0, length: 0) + let atStart = try #require( + textView.attributedSubstring(forProposedRange: NSRange(location: 0, length: 0), actualRange: &start) + ) + #expect(atStart.string.isEmpty) + #expect(start == NSRange(location: 0, length: 0)) + } + + @Test() + @MainActor + func attributedSubstringClampsARangeThatOverrunsTheEnd() throws { + let textView = makeTextView(string: "Hello") + + var actual = NSRange(location: 0, length: 0) + let substring = textView.attributedSubstring( + forProposedRange: NSRange(location: 3, length: 99), + actualRange: &actual + ) + + let resolved = try #require(substring) + #expect(resolved.string == "lo") + #expect(actual == NSRange(location: 3, length: 2)) + } + + @Test() + @MainActor + func attributedSubstringAnswersAnEmptyDocumentWithAnEmptyString() throws { + let textView = makeTextView(string: "") + + var actual = NSRange(location: 0, length: 0) + let substring = try #require( + textView.attributedSubstring(forProposedRange: NSRange(location: 0, length: 4), actualRange: &actual) + ) + #expect(substring.string.isEmpty) + #expect(actual == NSRange(location: 0, length: 0)) + } + + @Test() + @MainActor + func attributedSubstringRoundsToTheComposedSequence() throws { + let textView = makeTextView(string: "a👨‍👩‍👧b") + + var actual = NSRange(location: 0, length: 0) + let substring = textView.attributedSubstring( + forProposedRange: NSRange(location: 1, length: 2), + actualRange: &actual + ) + + let resolved = try #require(substring) + #expect(resolved.string == "👨‍👩‍👧") + #expect(actual.upperBound <= (textView.string as NSString).length) + } + + /// An input method asks for the rect of a zero-length range to place its candidate window, so + /// resolving must keep that question answerable. `actualRange` is the observable part here: + /// the rect itself converts through the window, and a detached text view has none. + @Test() + @MainActor + func firstRectResolvesTheCaretAndRejectsARangeWithNoPosition() { + let textView = makeTextView(string: "Hello") + + var caret = NSRange(location: 0, length: 0) + _ = textView.firstRect(forCharacterRange: NSRange(location: 5, length: 0), actualRange: &caret) + #expect(caret == NSRange(location: 5, length: 0)) + + var beyond = NSRange(location: 0, length: 0) + _ = textView.firstRect(forCharacterRange: NSRange(location: 40, length: 2), actualRange: &beyond) + #expect(beyond == .notFound) + + var missing = NSRange(location: 0, length: 0) + let rect = textView.firstRect(forCharacterRange: .notFound, actualRange: &missing) + #expect(missing == .notFound) + #expect(rect == .zero) + } + + /// The guard has to hold with a NULL `actualRange` too: the raising call used to sit inside a + /// branch that only ran when the caller wanted the adjusted range back. + @Test() + @MainActor + func firstRectSurvivesAnOutOfBoundsRangeWithNoActualRange() { + let textView = makeTextView(string: "Hello") + + #expect(textView.firstRect(forCharacterRange: NSRange(location: 99, length: 4), actualRange: nil) == .zero) + } + + @Test() + @MainActor + func insertTextClampsAReplacementRangeThatOutrunsTheDocument() { + let textView = makeTextView(string: "Hello") + + textView.insertText("X", replacementRange: NSRange(location: 4, length: 99)) + + #expect(textView.string == "HellX") + } + + @Test() + @MainActor + func insertTextAppendsWhenTheReplacementRangeStartsPastTheEnd() { + let textView = makeTextView(string: "Hello") + + textView.insertText("X", replacementRange: NSRange(location: 12, length: 3)) + + #expect(textView.string == "HelloX") + } + + @Test() + @MainActor + func setMarkedTextSurvivesAReplacementRangeThatOutrunsTheDocument() { + let textView = makeTextView(string: "Hello") + + textView.setMarkedText( + "ni", + selectedRange: NSRange(location: 2, length: 0), + replacementRange: NSRange(location: 12, length: 3) + ) + + #expect(textView.string == "Helloni") + } + + /// Multi-cursor input marks one range per cursor. Once the document shrinks under them they can + /// all resolve to the same position, and replacing that position once per cursor would insert + /// the text as many times as there were cursors. + @Test() + @MainActor + func replacementRangesThatResolveToTheSamePositionCollapse() { + let textView = makeTextView(string: "Hello") + + let resolved = textView.resolvedReplacementRanges([ + NSRange(location: 9, length: 2), + NSRange(location: 12, length: 4), + NSRange(location: NSNotFound, length: 0), + NSRange(location: 1, length: 1) + ]) + + #expect(resolved == [NSRange(location: 5, length: 0), NSRange(location: 1, length: 1)]) + } + + /// A composition spans many callbacks, and an edit made through any other path in between + /// leaves the marked ranges pointing at text that has moved. Resolving only what gets replaced + /// stops the crash but leaves the bookkeeping stale, so the next keystroke appends instead of + /// replacing what the one before it inserted. + @Test() + @MainActor + func aCompositionSurvivesAnEditMadeOutsideIt() { + let textView = makeTextView(string: "Hello") + textView.selectionManager.setSelectedRange(NSRange(location: 5, length: 0)) + + textView.setMarkedText( + "n", + selectedRange: NSRange(location: 1, length: 0), + replacementRange: NSRange(location: NSNotFound, length: 0) + ) + #expect(textView.string == "Hellon") + + textView.replaceCharacters(in: NSRange(location: 0, length: 6), with: "Hi") + + textView.setMarkedText( + "ni", + selectedRange: NSRange(location: 2, length: 0), + replacementRange: NSRange(location: NSNotFound, length: 0) + ) + #expect(textView.string == "Hini") + + textView.setMarkedText( + "nih", + selectedRange: NSRange(location: 3, length: 0), + replacementRange: NSRange(location: NSNotFound, length: 0) + ) + #expect(textView.string == "Hinih") + } + + @MainActor + private func makeTextView(string: String) -> TextView { + let textView = TextView(string: string) + textView.isEditable = true + textView.frame = NSRect(x: 0, y: 0, width: 1_000, height: 100) + textView.layoutManager.layoutLines(in: CGRect(origin: .zero, size: CGSize(width: 1_000, height: 100))) + return textView + } +}