Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Cotabby.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
6E01052209B73D7361C12CEF /* ClipboardContentDistiller.swift in Sources */ = {isa = PBXBuildFile; fileRef = 96495E4147D828C0B1B22765 /* ClipboardContentDistiller.swift */; };
6E49ADEB31D04DC77A47DEB0 /* FileLogHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = CE8C2569A8217EE9BD3B197F /* FileLogHandler.swift */; };
744B06C2488156B178675615 /* PermissionManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85BF316556FDA64CB8AD07B6 /* PermissionManager.swift */; };
76FD91607794883F8E121450 /* CaretGeometrySelector.swift in Sources */ = {isa = PBXBuildFile; fileRef = E3C84377F352140759B448C9 /* CaretGeometrySelector.swift */; };
7B6A63F5DCC2C163CDFD2A5C /* Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = BC4F887528AE74AC0DD30314 /* Assets.xcassets */; };
7D6BB9AF72F7076A4E5EE96F /* DownloadableModelCatalogView.swift in Sources */ = {isa = PBXBuildFile; fileRef = BB5C2AE9A7E55495D26AD074 /* DownloadableModelCatalogView.swift */; };
7E9413CE7C999C4612348248 /* SuggestionSessionReconcilerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9C8F07AC52C7A482F5FE34C5 /* SuggestionSessionReconcilerTests.swift */; };
Expand Down Expand Up @@ -305,6 +306,7 @@
DEB16474A67CE1D210B944C9 /* SuggestionSubsystemContracts.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SuggestionSubsystemContracts.swift; sourceTree = "<group>"; };
E1D2782B6C7BE3F56BCB22DE /* LlamaVisualContextSummarizer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LlamaVisualContextSummarizer.swift; sourceTree = "<group>"; };
E217A66717D78E1E49350EC8 /* DownloadOutcomeClassifierTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DownloadOutcomeClassifierTests.swift; sourceTree = "<group>"; };
E3C84377F352140759B448C9 /* CaretGeometrySelector.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CaretGeometrySelector.swift; sourceTree = "<group>"; };
E43E587E421AF544A8300CE4 /* CustomRulesCatalog.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CustomRulesCatalog.swift; sourceTree = "<group>"; };
E6423D6CC8CC371D2DA899DE /* PermissionOverlayTracker.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PermissionOverlayTracker.swift; sourceTree = "<group>"; };
E7F42112F14026E6253BB865 /* PermissionAndContextModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PermissionAndContextModelTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -591,6 +593,7 @@
352AF5B2834FEE1F597394E4 /* ApplicationBundleMetadata.swift */,
AC70775535A3428991025AB8 /* AXHelper.swift */,
AA33F5FFAC5B99384E15CE3E /* BundledRuntimeLocator.swift */,
E3C84377F352140759B448C9 /* CaretGeometrySelector.swift */,
96495E4147D828C0B1B22765 /* ClipboardContentDistiller.swift */,
D3A2AC525DC664DB540D4F19 /* ClipboardRelevanceFilter.swift */,
C7B2D34A6F3AC9DFD61350F7 /* CotabbyDebugOptions.swift */,
Expand Down Expand Up @@ -744,6 +747,7 @@
C4C6734678797669055988E0 /* AppUpdateManager.swift in Sources */,
66C23A7C2FCDE0266FF425F8 /* ApplicationBundleMetadata.swift in Sources */,
3CBBC3BFAC0DC8952EE24EF7 /* BundledRuntimeLocator.swift in Sources */,
76FD91607794883F8E121450 /* CaretGeometrySelector.swift in Sources */,
6E01052209B73D7361C12CEF /* ClipboardContentDistiller.swift in Sources */,
D2CAA59F69BCBC07DDA2B0D8 /* ClipboardContextProvider.swift in Sources */,
157A55EB796BEB7819B90D5D /* ClipboardRelevanceFilter.swift in Sources */,
Expand Down
94 changes: 19 additions & 75 deletions Cotabby/Services/Focus/FocusSnapshotResolver.swift
Original file line number Diff line number Diff line change
Expand Up @@ -119,26 +119,25 @@ struct FocusSnapshotResolver {
}

// The input target and the geometry source don't need to be the same element.
// Native AppKit apps give exact caret rects on the input target itself. Chrome's
// AXTextArea, by contrast, answers BoundsForRange(loc-1, 1) with a multi-line union
// rect — labeled `.derived` here but actually unusable. The leaf AXStaticText holding
// the active line carries its own zero-length selection range and
// AXSelectedTextMarkerRange, so the deep BFS in `resolveDeepGeometrySource` can
// synthesize a real `.exact` rect via Branch 1.5 (TextMarker) on that leaf.
//
// Precedence:
// 1. primary `.exact` (single API call, perfect — no walk needed)
// 2. deep `.exact` (beats primary `.derived` so we escape Chrome's union-rect trap)
// 3. primary `.derived`
// 4. deep `.derived`
// 5. primary `.estimated` / unknown fallback
// The walk is skipped entirely when primary is already `.exact`, and otherwise throttled to
// one BFS per `deepWalkThrottleInterval` while focus stays in the same field. Chromium
// editors keep the focused element at `.derived`, so without the throttle the ~200-node walk
// ran on every keystroke and pinned a CPU core. Within the window we reuse the previous deep
// result, which can trail the live caret by up to one throttle interval of fast typing.
// Native AppKit apps give exact caret rects on the input target itself. The deep BFS in
// `resolveDeepGeometrySource` can recover a real `.exact` rect from a leaf AXStaticText
// (via Branch 1.5 (TextMarker) on its zero-length selection range) when the focused input
// only exposes weak geometry. Selection precedence and the search decision live in the
// pure `CaretGeometrySelector`:
// 1. primary `.exact` (single API call, perfect — no walk needed)
// 2. primary `.derived` (trusted; the walk is skipped entirely for it)
// 3. deep (any) (only reached when primary is `.estimated`/unknown)
// 4. primary (any, fallback)
// The walk is skipped whenever primary geometry is already trustworthy (`.exact`/`.derived`),
// and otherwise throttled to one BFS per `deepWalkThrottleInterval` while focus stays in the
// same field, so the ~200-node walk does not run on every keystroke and pin a CPU core.
// Within the window we reuse the previous deep result, which can trail the live caret by up
// to one throttle interval of fast typing.
let deepResult: CaretGeometryResult?
if resolvedCandidate.caretQuality == .exact {
if !CaretGeometrySelector.shouldSearchDeep(
primaryRect: resolvedCandidate.caretRect,
primaryQuality: resolvedCandidate.caretQuality
) {
deepResult = nil
} else {
deepResult = deepWalkThrottle.result(
Expand All @@ -153,7 +152,7 @@ struct FocusSnapshotResolver {
}
}

guard let caret = Self.selectCaretGeometry(
guard let caret = CaretGeometrySelector.select(
primaryRect: resolvedCandidate.caretRect,
primaryQuality: resolvedCandidate.caretQuality,
primaryObservedCharWidth: resolvedCandidate.observedCharWidth,
Expand Down Expand Up @@ -270,61 +269,6 @@ struct FocusSnapshotResolver {
return ordered
}

/// Chooses the caret geometry to ship from the primary candidate and the optional deep-tree
/// result, following a fixed precedence (see the call site). Pulled out of `resolveSnapshot`
/// so that method stays under the cyclomatic-complexity limit; returns `nil` when neither
/// source produced a rect, which the caller maps to an unsupported snapshot.
///
/// Precedence: primary `.exact` → deep `.exact` (beats primary `.derived`, escaping Chrome's
/// union-rect trap) → primary `.derived` → deep (any) → primary (any, fallback).
private struct SelectedCaretGeometry {
let rect: CGRect
let source: String
let quality: CaretGeometryQuality
let observedCharWidth: CGFloat?
}

private static func selectCaretGeometry(
primaryRect: CGRect?,
primaryQuality: CaretGeometryQuality?,
primaryObservedCharWidth: CGFloat?,
deepResult: CaretGeometryResult?
) -> SelectedCaretGeometry? {
if let primary = primaryRect, primaryQuality == .exact {
return SelectedCaretGeometry(
rect: primary, source: "exact primary", quality: .exact,
observedCharWidth: primaryObservedCharWidth
)
}
if let deep = deepResult, deep.quality == .exact {
return SelectedCaretGeometry(
rect: deep.rect, source: "exact deep", quality: .exact,
observedCharWidth: deep.observedCharWidth
)
}
if let primary = primaryRect, primaryQuality == .derived {
return SelectedCaretGeometry(
rect: primary, source: "derived primary", quality: .derived,
observedCharWidth: primaryObservedCharWidth
)
}
if let deep = deepResult {
return SelectedCaretGeometry(
rect: deep.rect, source: "\(deep.quality.label) deep", quality: deep.quality,
observedCharWidth: deep.observedCharWidth
)
}
if let primary = primaryRect {
return SelectedCaretGeometry(
rect: primary,
source: "\(primaryQuality?.label ?? "unknown") primary-fallback",
quality: primaryQuality ?? .estimated,
observedCharWidth: primaryObservedCharWidth
)
}
return nil
}

/// Runs deep geometry search from the resolved editable candidate first, then falls back to
/// the raw focused node when those are different branches of the same local AX neighborhood.
private func resolveDeepGeometrySource(
Expand Down
89 changes: 89 additions & 0 deletions Cotabby/Support/CaretGeometrySelector.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import CoreGraphics
import Foundation

/// Pure caret-geometry trust policy used by `FocusSnapshotResolver`.
///
/// Two decisions live here, kept free of live Accessibility objects so they can be unit tested:
/// 1. `shouldSearchDeep` — whether the focused input's own caret geometry is too weak to trust,
/// so the resolver should run the expensive deep AX-tree walk for a better source.
/// 2. `select` — given the primary candidate's geometry and an optional deep-walk result, which
/// rect to actually ship, following a fixed precedence.
///
/// The regression these protect against is trusting a descendant rect over the focused input's own
/// usable rect (or vice versa). Chrome's AXTextArea answers BoundsForRange with a multi-line union
/// rect — labelled `.derived` but unusable for precise caret placement — while the leaf AXStaticText
/// holding the active line carries a real `.exact` rect that the deep walk can recover.
Comment on lines +12 to +15
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Class doc contradicts the implementation it describes

Lines 13–15 say Chrome's AXTextArea labels its multi-line union rect .derived, it is "unusable for precise caret placement," and "the deep walk can recover" a real .exact rect from the leaf AXStaticText. But shouldSearchDeep now returns false for .derived, and select prefers primary .derived over a deep .exact (see testPrimaryDerivedWinsOverDeepExact). The protection described in the header is exactly the behavior this PR removes. A Chrome user with a .derived primary union rect will get that unusable rect shipped; the deep walk that recovered the leaf's .exact no longer fires.

Either the doc is stale (Chrome's .derived geometry is now actually usable) and the comment should be updated or removed, or the code regresses Chrome caret placement for editors that still expose a wide union rect under .derived.

Fix in Codex Fix in Claude Code

enum CaretGeometrySelector {
/// The caret geometry chosen to ship, plus a human-readable source label for diagnostics.
struct Selected: Equatable {
let rect: CGRect
let source: String
let quality: CaretGeometryQuality
let observedCharWidth: CGFloat?
}

/// Whether the primary (focused-input) caret geometry is too weak to trust, so the resolver
/// should run the deep AX-tree walk for a more precise source.
///
/// `.exact` and `.derived` are trusted as-is; only `.estimated`, unknown quality, or a missing
/// rect justify the ~200-node walk. The walk pins a CPU core when run on every keystroke, so we
/// avoid it whenever the primary geometry is already good enough.
static func shouldSearchDeep(
primaryRect: CGRect?,
primaryQuality: CaretGeometryQuality?
) -> Bool {
guard primaryRect != nil else {
return true
}
switch primaryQuality {
case .exact, .derived:
return false
default:
return true
}
}

/// Chooses the caret geometry to ship from the primary candidate and the optional deep-tree
/// result. Returns `nil` when neither source produced a rect, which the caller maps to an
/// unsupported snapshot.
///
/// Precedence:
/// 1. primary `.exact` (single API call, perfect — no walk needed)
/// 2. primary `.derived` (trusted; `shouldSearchDeep` skips the walk entirely for it)
/// 3. deep (any) (only reached when primary is `.estimated`/unknown)
/// 4. primary (any, fallback)
static func select(
primaryRect: CGRect?,
primaryQuality: CaretGeometryQuality?,
primaryObservedCharWidth: CGFloat?,
deepResult: CaretGeometryResult?
) -> Selected? {
if let primary = primaryRect, primaryQuality == .exact {
return Selected(
rect: primary, source: "exact primary", quality: .exact,
observedCharWidth: primaryObservedCharWidth
)
}
if let primary = primaryRect, primaryQuality == .derived {
return Selected(
rect: primary, source: "derived primary", quality: .derived,
observedCharWidth: primaryObservedCharWidth
)
}
if let deep = deepResult {
return Selected(
rect: deep.rect, source: "\(deep.quality.label) deep", quality: deep.quality,
observedCharWidth: deep.observedCharWidth
)
}
if let primary = primaryRect {
return Selected(
rect: primary,
source: "\(primaryQuality?.label ?? "unknown") primary-fallback",
quality: primaryQuality ?? .estimated,
observedCharWidth: primaryObservedCharWidth
)
}
return nil
}
}