Skip to content

Add HighlightedTextManager and pull highlighted text logic into it #368

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
timlenardo merged 5 commits into from
Aug 5, 2025
Merged

Add HighlightedTextManager and pull highlighted text logic into it #368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1db5b26
Add HighlightedTextManager and pull highlighted text logic into it
timlenardo Jul 29, 2025
1959fd7
Add HighlightedTextManager and pull highlighted text logic into it
timlenardo Jul 29, 2025
610edd2
Apply suggestion from @greptile-apps[bot]
timlenardo Jul 29, 2025
e85110a
Review fixes
Niduank Jul 31, 2025
0502615
address nits
timlenardo Aug 5, 2025
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
2 changes: 2 additions & 0 deletions macos/Onit/Accessibility/Notifications/AccessibilityNotificationsDelegate.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import SwiftUI
func accessibilityManager(_ manager: AccessibilityNotificationsManager, didResizeWindow window: TrackedWindow)

func accessibilityManager(_ manager: AccessibilityNotificationsManager, didChangeWindowTitle window: TrackedWindow)
func accessibilityManager(_ manager: AccessibilityNotificationsManager, didChangeSelection element: AXUIElement)
func accessibilityManager(_ manager: AccessibilityNotificationsManager, didChangeFocusedUIElement element: AXUIElement)

var wantsNotificationsFromIgnoredProcesses: Bool { get }
var wantsNotificationsFromOnit: Bool { get }
Expand Down
2 changes: 1 addition & 1 deletion macos/Onit/Accessibility/Notifications/AccessibilityNotificationsManager+Config.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ extension AccessibilityNotificationsManager {
struct Config {

static let debounceInterval: TimeInterval = 0.3 // 300ms
static let textSelectionDebounceInterval: TimeInterval = 0.15 // 150ms

}
}
106 changes: 9 additions & 97 deletions macos/Onit/Accessibility/Notifications/AccessibilityNotificationsManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,8 @@ class AccessibilityNotificationsManager: ObservableObject {

private var currentSource: String?

private var lastHighlightingProcessedAt: Date?
private var lastCaretPositionChangeTimestamp: Date?

private var valueDebounceWorkItem: DispatchWorkItem?
private var selectionDebounceWorkItem: DispatchWorkItem?
private var parseDebounceWorkItem: DispatchWorkItem?

// Published property for selected text that QuickEditManager can observe
@Published var selectedText: String?

// MARK: - Private initializer

Expand Down Expand Up @@ -113,14 +106,9 @@ class AccessibilityNotificationsManager: ObservableObject {
}

currentSource = nil
lastHighlightingProcessedAt = nil
lastCaretPositionChangeTimestamp = nil
valueDebounceWorkItem?.cancel()
selectionDebounceWorkItem?.cancel()
parseDebounceWorkItem?.cancel()

selectedText = nil

ContextFetchingService.shared.reset()
}

Expand All @@ -136,10 +124,9 @@ class AccessibilityNotificationsManager: ObservableObject {

Task { @MainActor in
if let element = unsafeElement {
self.processSelectionChange(for: element)
self.handleSelectionChange(for: element)
} else {
// Handle text deselection - clear pendingInput
self.processSelectedText(nil)
HighlightedTextManager.shared.processSelectedText(nil)

@Niduank Niduank Jul 31, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@timlenardo I replaced the // wut by this call.
Without this call, unhighlight is not working on Notes, iTerm2.

I know that it breaks the separation of layers.
The other way to handle it is to make AXUIElement optional in handleSelectionChange, delegate etc ...
I started doing that but stopped when I reached this code in AccessibilityNotificationsManager:

private func notifyDelegates(for element: AXUIElement, _ notification: (AccessibilityNotificationsDelegate) -> Void) {
       guard let elementPid = element.pid() else {
           // If we can't determine PID, notify all delegates
           notifyDelegates(notification)
           return
       }

       let processType = determineProcessType(for: elementPid)

       for case let delegate as AccessibilityNotificationsDelegate in delegates.allObjects {
           switch processType {
           case .ownProcess:
               if delegate.wantsNotificationsFromOnit {
                   notification(delegate)
               }
           case .ignoreProcess:
               if delegate.wantsNotificationsFromIgnoredProcesses {
                   notification(delegate)
               }
           case .eligible:
               notification(delegate)
           }
       }
   }

@timlenardo timlenardo Aug 5, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I see the problem. My only other idea is:

  1. Add a 'selectedText' optional String to handleSelectionChange, pass it through the delegate's didChangeSelection method.
  2. Update HighlightedTextWorker to always return a valid AXUIElement, even if there's no selected text. This could be the window element if no selectedText elements are found.

I think this would be nicer, because a) we can completely dissociate HighlightedTextManger from AccessibilityNotificationManager b) we may avoid some race conditions by reading the selectedText() as soon as the notification comes through. Currently, it's up to the AXNotifDelegate to read the selectedText(), but if there's anything asynchronous in the callstack, the value may change. What do you think?

@lk340 lk340 Aug 5, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It seems like didChangeSelection is only used by the HighlightedTextManager (all other implementations are stubs), in which case, can't we just make it so that AXUIElement can be nil for :

  • AccessibilityNotificationsManager.handleSelectionChange()
  • AccessibilityNotificationsDelegate.accessibilityManager(_ manager..., didChangeSelection: ...)
  • HighlightedTextManager.handleSelectionChange()

And then, in HighlightedTextManager.handleSelectionChange(), we can add a guard clause that sets selectedText to nil when element is nil:


    func handleSelectionChange(for element: AXUIElement?) {
        guard let element = element else {
            PanelStateCoordinator.shared.state.pendingInput = nil
            PanelStateCoordinator.shared.state.trackedPendingInput = nil
            self.selectedText = nil
            return
        }

        guard HighlightedTextValidator.isValid(element: element) else { return }

        ...
    }

And then all we'd have to do in AccessibilityNotificationsManager.handleAppActivation() is replace


                Task { @MainActor in
                    if let element = unsafeElement {
                        self.handleSelectionChange(for: element)
                    } else {
                        HighlightedTextManager.shared.processSelectedText(nil)
                    }
                }

with


                Task { @MainActor in
                    self.handleSelectionChange(for: unsafeElement)
                }

And update the guard clause in notifyDelegate from:

`guard let elementPid = element.pid() else { ... }`

to


`guard let element = element, let elementPid = element.pid() else { ... }`

And then update the stubs to take in an optional AXUIElement for the didChangeSelection notification.

I don't think this should break anything, as HighlightedTextManager is the only one that's actually processing the kAXSelectedTextChangedNotification.

Is this what you were referring to @Niduank ?

@timlenardo timlenardo Aug 5, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@lk340 - the issue is in notifyDelegates in AccessibilityNotificationsManager, where we check the processID before sending the notification. If we have a nil AXUIElement, we won't know which process it's from, so we will always send the notification. That would be okay for now, since only Notes and iTerm2 use the HighlightedTextPoller. But it would break if want to add any app using the HighlightedTextPoller to the ignoredApps list. In this case, we'd send the notification to all delegates, even if the app should be ignored.

}
}
})
Expand Down Expand Up @@ -176,9 +163,8 @@ class AccessibilityNotificationsManager: ObservableObject {
self.handleSelectionChange(for: element)
case kAXValueChangedNotification:
self.handleValueChanged(for: element)
// self.handleCaretPositionChange(for: element)
case kAXFocusedUIElementChangedNotification:
self.handleCaretPositionChange(for: element)
self.handleFocusedUIElementChanged(for: element)
case kAXWindowMovedNotification:
self.handleWindowMoved(for: element, elementPid: elementPid)
case kAXWindowResizedNotification:
Expand Down Expand Up @@ -276,23 +262,15 @@ class AccessibilityNotificationsManager: ObservableObject {
}

private func handleSelectionChange(for element: AXUIElement) {
guard HighlightedTextValidator.isValid(element: element) else { return }

// Fix to work with PDF in Chrome
if let lastHighlightingProcessedAt = lastHighlightingProcessedAt, Date().timeIntervalSince(lastHighlightingProcessedAt) < 0.002 {
return
self.notifyDelegates(for: element) { delegate in
delegate.accessibilityManager(self, didChangeSelection: element)
}

lastHighlightingProcessedAt = Date()
selectionDebounceWorkItem?.cancel()
}

let workItem = DispatchWorkItem { [weak self] in
self?.processSelectionChange(for: element)
private func handleFocusedUIElementChanged(for element: AXUIElement) {
self.notifyDelegates(for: element) { delegate in
delegate.accessibilityManager(self, didChangeFocusedUIElement: element)
}

selectionDebounceWorkItem = workItem

DispatchQueue.main.asyncAfter(deadline: .now() + Config.textSelectionDebounceInterval, execute: workItem)
}

// MARK: Value Changed
Expand All @@ -309,73 +287,7 @@ class AccessibilityNotificationsManager: ObservableObject {
showDebug()
}

// MARK: Text Selection

private func processSelectionChange(for element: AXUIElement) {
// Ensure we're on the main thread
dispatchPrecondition(condition: .onQueue(.main))

let selectedText = element.selectedText()

if let selectedText = selectedText, HighlightedTextValidator.isValid(text: selectedText) {
Task {
_ = await HighlightedTextBoundsExtractor.shared.getBounds(for: element, selectedText: selectedText)

processSelectedText(selectedText)
}
} else {
// On every apps, when caret position changed, we receive AXSelectedTextChanged notification with nil value.
// This code is used to hide the QuickEdit hint for a real deselection
let now = Date()
let caretPositionChangeRecently = now.timeIntervalSince(lastCaretPositionChangeTimestamp ?? .distantPast) < 0.5
let isEditableField = element.role() == kAXTextFieldRole || element.role() == kAXTextAreaRole

if !caretPositionChangeRecently && !isEditableField {
processSelectedText(nil)
HighlightedTextBoundsExtractor.shared.reset()
QuickEditManager.shared.hideHint()
} else if isEditableField {
handleCaretPositionChange(for: element)
}
}

showDebug()
}

private func processSelectedText(_ text: String?) {
guard Defaults[.autoContextFromHighlights],
let selectedText = text,
HighlightedTextValidator.isValid(text: selectedText) else {

PanelStateCoordinator.shared.state.pendingInput = nil
PanelStateCoordinator.shared.state.trackedPendingInput = nil
self.selectedText = nil
return
}

// Update the published selectedText property
self.selectedText = selectedText

let input = Input(selectedText: selectedText, application: currentSource ?? "")

if Defaults[.autoAddHighlightedTextToContext] {
PanelStateCoordinator.shared.state.pendingInput = input
} else {
PanelStateCoordinator.shared.state.trackedPendingInput = input
}
}

// MARK: Caret Position Handling

private func handleCaretPositionChange(for element: AXUIElement) {
guard element.supportsCaretTracking() else { return }

selectionDebounceWorkItem?.cancel()
selectionDebounceWorkItem = nil
processSelectedText(nil)
lastCaretPositionChangeTimestamp = Date()
caretPositionManager.updateCaretPosition(for: element)
}

// MARK: Debug

Expand Down
5 changes: 4 additions & 1 deletion macos/Onit/Accessibility/Notifications/WindowChangeDelegate.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import Foundation
import AppKit
import ApplicationServices

// MARK: - Window Change Info

Expand Down Expand Up @@ -148,7 +149,9 @@ final class WindowChangeDelegate: AccessibilityNotificationsDelegate {
func accessibilityManager(_ manager: AccessibilityNotificationsManager, didDeminimizeWindow window: TrackedWindow) {}
func accessibilityManager(_ manager: AccessibilityNotificationsManager, didActivateIgnoredWindow window: TrackedWindow?) {}
func accessibilityManager(_ manager: AccessibilityNotificationsManager, didDestroyWindow window: TrackedWindow) {}

func accessibilityManager(_ manager: AccessibilityNotificationsManager, didChangeSelection element: AXUIElement) {}
func accessibilityManager(_ manager: AccessibilityNotificationsManager, didChangeFocusedUIElement element: AXUIElement) {}

var wantsNotificationsFromIgnoredProcesses: Bool { false }
var wantsNotificationsFromOnit: Bool { false }
}
4 changes: 4 additions & 0 deletions macos/Onit/PanelStateManager/Pinned/PanelStatePinnedManager+Delegates.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
//

import AppKit
import ApplicationServices

extension PanelStatePinnedManager: AccessibilityNotificationsDelegate {
// MARK: - DID ACTIVATE WINDOW
Expand Down Expand Up @@ -80,6 +81,9 @@ extension PanelStatePinnedManager: AccessibilityNotificationsDelegate {

var wantsNotificationsFromIgnoredProcesses: Bool { false }
var wantsNotificationsFromOnit: Bool { false }

func accessibilityManager(_ manager: AccessibilityNotificationsManager, didChangeSelection element: AXUIElement) {}
func accessibilityManager(_ manager: AccessibilityNotificationsManager, didChangeFocusedUIElement element: AXUIElement) {}
}

extension PanelStatePinnedManager: OnitPanelStateDelegate {
Expand Down
5 changes: 5 additions & 0 deletions macos/Onit/PanelStateManager/Tethered/PanelStateTetheredManager+Delegates.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

// MARK: - AccessibilityNotificationsDelegate

import ApplicationServices

extension PanelStateTetheredManager: AccessibilityNotificationsDelegate {
// MARK: - DID ACTIVATE WINDOW

Expand Down Expand Up @@ -102,6 +104,9 @@ extension PanelStateTetheredManager: AccessibilityNotificationsDelegate {
}
}

func accessibilityManager(_ manager: AccessibilityNotificationsManager, didChangeSelection element: AXUIElement) {}
func accessibilityManager(_ manager: AccessibilityNotificationsManager, didChangeFocusedUIElement element: AXUIElement) {}

var wantsNotificationsFromIgnoredProcesses: Bool { false }
var wantsNotificationsFromOnit: Bool { false }
}
Expand Down
12 changes: 12 additions & 0 deletions macos/Onit/StateManagers/HighlightedText/HighlightedTextConfig.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
//
// HighlightedTextConfig.swift
// Onit
//
// Created by Kévin Naudin on 28/04/2025.
//

import Foundation

struct HighlightedTextConfig {
static let textSelectionDebounceInterval: TimeInterval = 0.15 // 150ms

@lk340 lk340 Aug 5, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We should remove the vestigial textSelectionDebounceInterval from AccessibilityNotificationsManager+Config.swift.

}
0 ...htedText/HighlightedTextCoordinator.swift → ...htedText/HighlightedTextCoordinator.swift
View file
Open in desktop
File renamed without changes.
50 changes: 50 additions & 0 deletions macos/Onit/StateManagers/HighlightedText/HighlightedTextManager+Delegates.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
//
// HighlightedTextManager+Delegates.swift
// Onit
//
// Created by TimL on 28/04/2025.
//

import ApplicationServices

extension HighlightedTextManager: AccessibilityNotificationsDelegate {
// MARK: - AccessibilityNotificationsDelegate Implementation
func accessibilityManager(_ manager: AccessibilityNotificationsManager, didChangeSelection element: AXUIElement) {
handleSelectionChange(for: element)
}

func accessibilityManager(_ manager: AccessibilityNotificationsManager, didChangeFocusedUIElement element: AXUIElement) {
handleCaretPositionChange(for: element)
}

// Unused stubs
func accessibilityManager(_ manager: AccessibilityNotificationsManager, didActivateWindow window: TrackedWindow) {}
func accessibilityManager(_ manager: AccessibilityNotificationsManager, didActivateIgnoredWindow window: TrackedWindow?) {}
func accessibilityManager(_ manager: AccessibilityNotificationsManager, didMinimizeWindow window: TrackedWindow) {}
func accessibilityManager(_ manager: AccessibilityNotificationsManager, didDeminimizeWindow window: TrackedWindow) {}
func accessibilityManager(_ manager: AccessibilityNotificationsManager, didDestroyWindow window: TrackedWindow) {}
func accessibilityManager(_ manager: AccessibilityNotificationsManager, didMoveWindow window: TrackedWindow) {}
func accessibilityManager(_ manager: AccessibilityNotificationsManager, didResizeWindow window: TrackedWindow) {}
func accessibilityManager(_ manager: AccessibilityNotificationsManager, didChangeWindowTitle window: TrackedWindow) {}
}

extension HighlightedTextManager: AccessibilityObserversDelegate {

func accessibilityObserversManager(didActivateApplication appName: String?, processID: pid_t) {
setCurrentSource(appName)
}

func accessibilityObserversManager(didDeactivateApplication appName: String?, processID: pid_t) {
// Reset state when app deactivates
reset()
}

func accessibilityObserversManager(didActivateIgnoredApplication appName: String?, processID: pid_t) {}
func accessibilityObserversManager(didReceiveNotification notification: String,
element: AXUIElement,
elementPid: pid_t,
info: [String: Any]) {}
func accessibilityObserversManager(didDeactivateIgnoredApplication appName: String?, processID: pid_t) {}
func accessibilityObserversManager(didActivateOnit processID: pid_t) {}
func accessibilityObserversManager(didDeactivateOnit processID: pid_t) {}
}
Loading