Skip to content

Commit 44d4e0e

Browse files
authored
fix(editor): rebuild text selection around an anchor and the rect being drawn (#2542)
Claude-Session: https://claude.ai/code/session_011y4GK3oSBTYUXv5ZW1reE2
1 parent d45948e commit 44d4e0e

26 files changed

Lines changed: 762 additions & 161 deletions

CHANGELOG.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2020

2121
### Fixed
2222

23+
- Selection highlight missing from part of a long selection after scrolling back up to it.
24+
- Editor not scrolling to follow a selection extended past the edge of the viewport.
25+
- Find highlight and the run band covering only the first line of a match that spans several lines.
26+
- Nothing selected when double-clicking `=`, `<`, `>` or any other SQL operator.
27+
- Shift+Arrow extending the wrong end of a selection made by dragging.
28+
- Selection starting a few characters away from the press point on a quick drag.
29+
- A pause before the pointer responds when pressing inside selected text.
30+
- Shift+double-click and Shift+triple-click doing nothing.
31+
- Drag-select scrolling faster on a mouse than on a trackpad, and stalling mid-drag.
32+
- Statement selection with `Option+Shift+Down` leaving the highlight behind.
33+
- No arrow-key movement or Shift+Arrow selection in the JSON, DDL and SQL preview editors.
34+
- Explain with AI acting on the previous selection after a right-click somewhere else.
35+
- Selection painted in the accent colour in a window that is not the active one.
36+
- Selection bounds reported to VoiceOver covering only the first line, and no announcement when the selection moved.
2337
- An abandoned editor tab drag reordering the strip anyway, and keeping that order across relaunch.
2438
- An editor tab reorder stopping partway when the pointer strayed a couple of points off the track.
2539
- Text dropped on the editor tab strip reported as accepted and then discarded.

LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/Minimap/MinimapView+TextSelectionManagerDelegate.swift

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,7 @@ import CodeEditTextView
1010

1111
extension MinimapView: TextSelectionManagerDelegate {
1212
public var visibleTextRange: NSRange? {
13-
let minY = max(visibleRect.minY, 0)
14-
let maxY = min(visibleRect.maxY, layoutManager?.estimatedHeight() ?? 3.0)
15-
guard let minYLine = layoutManager?.textLineForPosition(minY),
16-
let maxYLine = layoutManager?.textLineForPosition(maxY) else {
17-
return nil
18-
}
19-
return NSRange(start: minYLine.range.location, end: maxYLine.range.max)
13+
layoutManager?.textRange(covering: visibleRect)
2014
}
2115

2216
public func setNeedsDisplay() {

LocalPackages/CodeEditTextView/Sources/CodeEditTextView/Extensions/CGRectArray+BoundingRect.swift

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,7 @@ extension Array where Element == CGRect {
1212
/// Returns `.zero` if the array is empty.
1313
/// - Returns: The minimum rectangle that contains all rectangles in this array.
1414
func boundingRect() -> CGRect {
15-
guard !self.isEmpty else { return .zero }
16-
let minX = self.min(by: { $0.origin.x < $1.origin.x })?.origin.x ?? 0
17-
let minY = self.min(by: { $0.origin.y < $1.origin.y })?.origin.y ?? 0
18-
let max = self.max(by: { $0.maxY < $1.maxY }) ?? .zero
19-
let origin = CGPoint(x: minX, y: minY)
20-
let size = CGSize(width: max.maxX - minX, height: max.maxY - minY)
21-
return CGRect(origin: origin, size: size)
15+
guard let first = self.first else { return .zero }
16+
return self.dropFirst().reduce(first) { $0.union($1) }
2217
}
2318
}

LocalPackages/CodeEditTextView/Sources/CodeEditTextView/TextLayoutManager/TextLayoutManager+Layout.swift

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,10 @@ extension TextLayoutManager {
8585
var yContentAdjustment: CGFloat = 0
8686
var maxFoundLineWidth = maxLineWidth
8787

88+
// The layout view draws its own decorations into a backing store nothing else invalidates when the
89+
// viewport moves, so a band drawn before its lines were laid out would stay blank forever.
90+
var relaidOutRect: CGRect = .null
91+
8892
#if DEBUG
8993
var laidOutLines: Set<TextLine.ID> = []
9094
#endif
@@ -107,6 +111,14 @@ extension TextLayoutManager {
107111
maxFoundLineWidth: &maxFoundLineWidth
108112
)
109113
yContentAdjustment += yAdjustment
114+
relaidOutRect = relaidOutRect.union(
115+
CGRect(
116+
x: 0,
117+
y: linePosition.yPos,
118+
width: layoutView?.frame.width ?? 0,
119+
height: max(linePosition.height, linePosition.data.lineFragments.height)
120+
)
121+
)
110122
#if DEBUG
111123
laidOutLines.insert(linePosition.data.id)
112124
#endif
@@ -163,6 +175,19 @@ extension TextLayoutManager {
163175
delegate?.layoutManagerHeightDidUpdate(newHeight: lineStorage.height)
164176
}
165177

178+
if !relaidOutRect.isNull {
179+
let movedEverythingBelow = didLayoutChange || yContentAdjustment != 0
180+
let invalidRect = movedEverythingBelow
181+
? CGRect(
182+
x: relaidOutRect.minX,
183+
y: relaidOutRect.minY,
184+
width: relaidOutRect.width,
185+
height: max(maxY - relaidOutRect.minY, relaidOutRect.height)
186+
)
187+
: relaidOutRect
188+
layoutView?.setNeedsDisplay(invalidRect)
189+
}
190+
166191
#if DEBUG
167192
return laidOutLines
168193
#else

LocalPackages/CodeEditTextView/Sources/CodeEditTextView/TextLayoutManager/TextLayoutManager+Public.swift

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,26 @@ extension TextLayoutManager {
5050
}
5151
}
5252

53+
/// The range of text laid out inside a rect.
54+
///
55+
/// This is the geometric question every rect-driven consumer has to ask: given a rect the view has been asked
56+
/// to draw, hit test or measure, which text does it cover? Callers that answer it from the *viewport* instead
57+
/// are wrong: AppKit calls `draw(_:)` with rects outside the visible area under responsive scrolling and caches
58+
/// the result, so a viewport-derived answer bakes the scroll position into the drawing.
59+
///
60+
/// - Parameter rect: The rect to find text for, in the text view's coordinate space.
61+
/// - Returns: The range the rect covers, or `nil` when the rect lies past the end of the document.
62+
public func textRange(covering rect: CGRect) -> NSRange? {
63+
let minY = max(rect.minY, 0)
64+
let maxY = min(rect.maxY, estimatedHeight())
65+
guard maxY >= minY,
66+
let firstLine = textLineForPosition(minY),
67+
let lastLine = textLineForPosition(maxY) else {
68+
return nil
69+
}
70+
return NSRange(start: firstLine.range.location, end: lastLine.range.max)
71+
}
72+
5373
/// Finds text line and returns it if found.
5474
/// Lines are 0 indexed.
5575
/// - Parameter index: The line to find.
@@ -232,10 +252,16 @@ extension TextLayoutManager {
232252
let realRangeStart = textStorage.rangeOfComposedCharacterSequence(at: range.lowerBound)
233253
let realRangeEnd = textStorage.rangeOfComposedCharacterSequence(at: range.upperBound - 1)
234254

255+
// Clamp to this line before rebasing. The requested range is document-absolute, so on every line after
256+
// the first, subtracting the line's own location from it produces a negative start and the fragment
257+
// lookup below matches nothing. That silently reduced any multi-line range to its first line.
258+
let characterAlignedRange = NSRange(start: realRangeStart.lowerBound, end: realRangeEnd.upperBound)
259+
guard let lineIntersection = characterAlignedRange.intersection(line.range) else { return [] }
260+
235261
// Fragments are relative to the line
236262
let relativeRange = NSRange(
237-
start: realRangeStart.lowerBound - line.range.location,
238-
end: realRangeEnd.upperBound - line.range.location
263+
start: lineIntersection.location - line.range.location,
264+
end: lineIntersection.max - line.range.location
239265
)
240266

241267
var rects: [CGRect] = []

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -75,16 +75,16 @@ extension TextSelectionManager {
7575
private func drawSelectedRange(in rect: NSRect, for textSelection: TextSelection, context: CGContext) {
7676
context.saveGState()
7777

78-
let fillColor = (textView?.isFirstResponder ?? false)
78+
// `unemphasizedSelectedTextBackgroundColor` is documented for a window that is not key OR a view without
79+
// key focus, so both conditions gate the accent colour.
80+
let isEmphasized = (textView?.isFirstResponder ?? false) && (textView?.window?.isKeyWindow ?? false)
81+
let fillColor = isEmphasized
7982
? selectionBackgroundColor.safeCGColor
80-
: selectionBackgroundColor.grayscale.safeCGColor
83+
: NSColor.unemphasizedSelectedTextBackgroundColor.safeCGColor
8184

8285
context.setFillColor(fillColor)
8386

84-
let fillRects = getFillRects(in: rect, for: textSelection)
85-
textSelection.boundingRect = fillRects.boundingRect()
86-
87-
context.fill(fillRects)
87+
context.fill(getFillRects(in: rect, for: textSelection))
8888
context.restoreGState()
8989
}
9090

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,11 @@ extension TextSelectionManager {
1717
/// - textSelection: The selection to use.
1818
/// - Returns: An array of rects that the selection overlaps.
1919
func getFillRects(in rect: NSRect, for textSelection: TextSelection) -> [CGRect] {
20+
// Bound the work by the rect we were asked to fill, never by the viewport: under responsive scrolling
21+
// `draw(_:)` is called with rects outside the visible area and the result is cached.
2022
guard let layoutManager,
21-
let range = textSelection.range.intersection(delegate?.visibleTextRange ?? .zero) else {
23+
let drawnRange = layoutManager.textRange(covering: rect),
24+
let range = textSelection.range.intersection(drawnRange) else {
2225
return []
2326
}
2427

@@ -43,8 +46,11 @@ extension TextSelectionManager {
4346
)
4447
}
4548

46-
// Pixel align these to avoid aliasing on the edges of each rect that should be a solid box.
47-
return fillRects.map { $0.intersection(validTextDrawingRect).pixelAligned }
49+
// Pixel align these to avoid aliasing on the edges of each rect that should be a solid box. A fragment
50+
// that misses the drawing rect intersects to `CGRect.null`, whose origin is infinite.
51+
return fillRects
52+
.map { $0.intersection(validTextDrawingRect).pixelAligned }
53+
.filter { !$0.isNull && !$0.isEmpty }
4854
}
4955

5056
/// Find fill rects for a specific line position.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ extension TextSelectionManager {
3131
}
3232
updateSelectionViews()
3333
delegate?.setNeedsDisplay()
34-
NotificationCenter.default.post(Notification(name: Self.selectionChangedNotification, object: self))
34+
notifySelectionChanged()
3535
}
3636

3737
/// Moves a single selection determined by the direction and destination provided.

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ extension TextSelectionManager {
1515
// multi-cursor IME path replaces each marked range char-for-char).
1616
let delta = replacementLength - range.length
1717
for textSelection in self.textSelections {
18+
// A pivot that survives the edit points at an offset that no longer means what it did, and the
19+
// modifying motions subtract against it until the range length goes negative.
20+
textSelection.pivot = nil
21+
textSelection.suggestedXPos = nil
1822
if textSelection.range.location > range.max {
1923
textSelection.range.location = max(0, textSelection.range.location + delta)
2024
textSelection.range.length = 0
@@ -45,6 +49,6 @@ extension TextSelectionManager {
4549

4650
public func notifyAfterEdit(force: Bool = false) {
4751
updateSelectionViews(force: force)
48-
NotificationCenter.default.post(Notification(name: Self.selectionChangedNotification, object: self))
52+
notifySelectionChanged()
4953
}
5054
}

LocalPackages/CodeEditTextView/Sources/CodeEditTextView/TextSelectionManager/TextSelectionManager.swift

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@ public class TextSelectionManager: NSObject {
8080
selection.suggestedXPos = layoutManager?.rectForOffset(range.location)?.minX
8181
textSelections = [selection]
8282
updateSelectionViews()
83-
NotificationCenter.default.post(Notification(name: Self.selectionChangedNotification, object: self))
83+
delegate?.setNeedsDisplay()
84+
notifySelectionChanged()
8485
}
8586

8687
/// Set the selected ranges to new ranges. Overrides any existing selections.
@@ -105,7 +106,7 @@ public class TextSelectionManager: NSObject {
105106
delegate?.setNeedsDisplay()
106107

107108
if oldRanges != textSelections.map(\.range) {
108-
NotificationCenter.default.post(Notification(name: Self.selectionChangedNotification, object: self))
109+
notifySelectionChanged()
109110
}
110111
}
111112

@@ -132,10 +133,20 @@ public class TextSelectionManager: NSObject {
132133
}
133134

134135
updateSelectionViews()
135-
NotificationCenter.default.post(Notification(name: Self.selectionChangedNotification, object: self))
136+
notifySelectionChanged()
136137
delegate?.setNeedsDisplay()
137138
}
138139

140+
/// The single place a selection change is announced, to observers and to assistive clients alike.
141+
func notifySelectionChanged() {
142+
NotificationCenter.default.post(Notification(name: Self.selectionChangedNotification, object: self))
143+
// Only the manager the text view answers to speaks for it. The minimap builds a second manager over the
144+
// same text view and mirrors every selection into it, which would announce each move twice.
145+
if let textView, textView.selectionManager === self {
146+
NSAccessibility.post(element: textView, notification: .selectedTextChanged)
147+
}
148+
}
149+
139150
// MARK: - Selection Views
140151

141152
/// Update all selection cursors. Placing them in the correct position for each text selection and

0 commit comments

Comments
 (0)