Make per-site disable live: capture the focused page URL#529
Conversation
#527 landed the matching core; this wires it end to end behind a default-off `cotabbyPerDomainDisableEnabled` flag so a domain on the user's list actually suppresses autocomplete. - `AXHelper.webURL(near:)` is a fail-safe, read-only walk up a bounded number of ancestors for `kAXURLAttribute` (browsers expose it on the web area, not the field). It returns nil on any miss, so a failed read degrades to "no per-site rule applies" rather than misbehaving. - `FocusSnapshotResolver` reads it into a new `FocusedInputSnapshot.focusedURLString` *only* when the feature is enabled, so the default focus-capture path performs no extra Accessibility round-trips. - `SuggestionAvailabilityEvaluator` reads that URL straight off the snapshot it already receives; the disabled-domains list comes from `PerDomainDisableSettings` (a `defaults`-backed array, no UI yet) and is threaded through the availability call sites alongside the existing per-app list. Default-off and inert by construction: with the flag off (or an empty list, or a non-browser focus) behavior is byte-for-byte unchanged, which the existing availability tests confirm. Promoting it needs on-device validation that the URL read resolves across Chrome (OOPIF), Safari, and Arc, and adds no measurable focus-capture latency — and a settings UI for the list.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| static func webURL(near element: AXUIElement, maxClimb: Int = 6) -> String? { | ||
| var current = element | ||
| for _ in 0...maxClimb { | ||
| if let url = urlString(on: current) { | ||
| return url | ||
| } | ||
| guard let parent = parentElement(of: current) else { break } | ||
| current = parent | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
0...maxClimb makes maxClimb + 1 iterations (e.g., 7 when maxClimb = 6). On the last iteration, after the URL check fails, the function still calls parentElement(of: current) and assigns the result to current, but that value is never used — the loop ends. This is one wasted AX round-trip per call when no URL is found at the deepest level. Changing the range to 0..<maxClimb and adding a final check outside the loop eliminates the extra call while preserving the same traversal depth.
| static func webURL(near element: AXUIElement, maxClimb: Int = 6) -> String? { | |
| var current = element | |
| for _ in 0...maxClimb { | |
| if let url = urlString(on: current) { | |
| return url | |
| } | |
| guard let parent = parentElement(of: current) else { break } | |
| current = parent | |
| } | |
| return nil | |
| } | |
| static func webURL(near element: AXUIElement, maxClimb: Int = 6) -> String? { | |
| var current = element | |
| for _ in 0..<maxClimb { | |
| if let url = urlString(on: current) { | |
| return url | |
| } | |
| guard let parent = parentElement(of: current) else { return nil } | |
| current = parent | |
| } | |
| return urlString(on: current) | |
| } |
| SuggestionAvailabilityEvaluator.shouldCaptureVisualContext( | ||
| globallyEnabled: settingsSnapshot.isGloballyEnabled, | ||
| disabledAppBundleIdentifiers: settingsSnapshot.disabledAppBundleIdentifiers, | ||
| disabledDomains: PerDomainDisableSettings.disabledDomains(), | ||
| inputMonitoringGranted: permissionManager.inputMonitoringGranted, |
There was a problem hiding this comment.
PerDomainDisableSettings.disabledDomains() is called at every evaluator call site independently, which means a single focus-change event (e.g., handleFocusSnapshotChange) triggers multiple UserDefaults.stringArray(forKey:) reads in sequence. Within that one function call alone there are two consecutive reads (one for shouldCaptureVisualContext, one for disabledReason). The same pattern appears across SuggestionCoordinator+Lifecycle.swift and SuggestionCoordinator+Prediction.swift. Caching the result in a local let at the top of each event-handler function (or reading it once in the coordinator) would remove the redundant lookups.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Summary
#527 landed the pure matching core for per-site disable but left it inert. This wires it end to end behind a default-off
cotabbyPerDomainDisableEnabledflag, so a domain on the user's list actually suppresses autocomplete the way a disabled app does.AXHelper.webURL(near:)— a fail-safe, read-only walk up a bounded number of ancestors readingkAXURLAttribute(browsers expose it on the web area / window, not the focused field). Returns nil on any miss, tolerates the value arriving asURL/NSURL/String, and never mutates AX state.FocusedInputSnapshot.focusedURLString— a new optional field, populated byFocusSnapshotResolveronly when the feature is enabled, so the default focus-capture path performs zero extra Accessibility round-trips.SuggestionAvailabilityEvaluatorreads the URL straight off the snapshot it already receives; the disabled-domains list comes fromPerDomainDisableSettings(adefaults-backed array, hidden-flag pattern, no UI yet), threaded through all availability call sites alongside the existing per-app list.Validation
To exercise on device:
defaults write com.jacobfu.tabby cotabbyPerDomainDisableEnabled -bool YESanddefaults write com.jacobfu.tabby cotabbyDisabledDomains -array bank.com.Linked issues
None. Completes the per-site disable feature started in #527.
Risk / rollout notes
focusedURLStringfield defaults to nil, so the twoFocusedInputSnapshotconstruction sites are unaffected.defaults-backed array is the dogfood surface).project.pbxprojregenerated by XcodeGen for the two new files.Greptile Summary
This PR wires the per-site disable feature end-to-end: a new
AXHelper.webURL(near:)walks the AX ancestor tree to find the browser's page URL, which is stored in a newfocusedURLStringfield onFocusedInputSnapshotand checked bySuggestionAvailabilityEvaluatoragainst aUserDefaults-backed domain list. The entire path is gated behind a default-offcotabbyPerDomainDisableEnabledflag, leaving existing behavior byte-for-byte unchanged.AXHelper.webURL(near:)climbs up tomaxClimbancestors usingkAXURLAttribute, returning nil on any miss;FocusSnapshotResolveronly invokes it when the feature flag is on.PerDomainDisableSettingsis a thin, injectableUserDefaultsreader for the flag and domain list;SuggestionAvailabilityEvaluatoradds adisabledDomainsparameter threaded through all call sites in the three coordinator files.Confidence Score: 4/5
Safe to merge; the feature is default-off and the gating path has no effect on existing behavior when the flag is unset.
The AX ancestor walk in
webURLuses a closed range (0...maxClimb) that makes one extraparentElement()call on the final iteration when no URL is found — a wasted AX round-trip per focus event when the feature is enabled. The coordinator files also callPerDomainDisableSettings.disabledDomains()independently at each evaluator site rather than reading once per event handler. Neither issue affects correctness, but the AX call is worth tidying before the feature exits dogfood.Cotabby/Support/AXHelper.swift — the
webURLloop boundary; Cotabby/App/Coordinators/SuggestionCoordinator+Input.swift — redundantdisabledDomains()reads per event.Important Files Changed
webURL(near:maxClimb:)+urlString(on:)helpers; closed-range loop makes one extraparentElement()AX call on the final iteration when no URL is found.disabledDomainsparameter to all three evaluator entry points and wires the per-site gate tofocusSnapshot.context?.focusedURLString; logic is correct and the gate stays inert when URL is nil.focusedURLStringfield toFocusedInputSnapshotwith a nil default; existing call sites are unaffected.focusedURLStringonly whenPerDomainDisableSettings.isEnabled()is true, keeping the default focus-capture path free of extra AX round-trips.disabledDomainsthrough five evaluator call sites;PerDomainDisableSettings.disabledDomains()is called separately at each site rather than once per event handler.focusedURLStringthrough the snapshot; adds per-site disable and inert-by-default test cases.Sequence Diagram
sequenceDiagram participant Coord as SuggestionCoordinator participant Resolver as FocusSnapshotResolver participant Settings as PerDomainDisableSettings participant AX as AXHelper participant Eval as SuggestionAvailabilityEvaluator participant Domain as BrowserDomain Coord->>Resolver: resolve(focusedElement) Resolver->>Settings: isEnabled() alt feature enabled Settings-->>Resolver: true Resolver->>AX: webURL(near: focusedElement) AX->>AX: urlString(on: element) x up to maxClimb levels AX-->>Resolver: url string or nil else feature disabled Settings-->>Resolver: false Note over Resolver: focusedURLString = nil (no AX call) end Resolver-->>Coord: FocusSnapshot (focusedURLString set or nil) Coord->>Settings: disabledDomains() Settings-->>Coord: "Set<String>" Coord->>Eval: disabledReason(disabledDomains:focusSnapshot:...) Eval->>Eval: focusSnapshot.context?.focusedURLString alt URL present Eval->>Domain: host(fromURLString:) Domain-->>Eval: host string Eval->>Domain: isHostDisabled(host, disabledDomains) Domain-->>Eval: true/false alt host disabled Eval-->>Coord: Cotabby is disabled on host. else host not disabled Eval-->>Coord: nil (enabled) end else URL nil (feature off or non-browser) Eval-->>Coord: nil (enabled) endReviews (1): Last reviewed commit: "Make per-site disable live: capture the ..." | Re-trigger Greptile