Add find-in-page (Cmd+F) for browser panels - #875
Merged
lawrencecchen merged 2 commits intoMar 5, 2026
Merged
Conversation
JavaScript-based find using TreeWalker + <mark> highlights with match counter, next/previous navigation, and drag-to-corner overlay matching the existing terminal find bar. - BrowserFindJavaScript: JS generation for search/next/prev/clear - BrowserSearchOverlay: SwiftUI overlay with IME-safe onSubmit - BrowserSearchState: Observable state (needle/selected/total) - TabManager routing: Cmd+F/G dispatches to browser when focused - Visibility filter: skips script/style/hidden/aria-hidden elements - Stale DOM guard: isConnected check in next/previous scripts - Navigation cleanup: clears find on didFinish and didFailNavigation
|
@y-agatsuma is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis change implements find-in-page functionality for the embedded browser panel. It introduces JavaScript-based search utilities (BrowserFindJavaScript), a SwiftUI search overlay (BrowserSearchOverlay), observable search state management (BrowserSearchState), and wires these into BrowserPanel and the global TabManager find flow with comprehensive test coverage. Changes
Sequence DiagramsequenceDiagram
actor User
participant TabManager
participant BrowserPanel
participant BrowserSearchOverlay
participant BrowserPanel as BP<br/>(JavaScript)
participant WKWebView
User->>TabManager: startSearch() / findNext() / findPrevious()
activate TabManager
TabManager->>BrowserPanel: startFind() / findNext() / findPrevious()
deactivate TabManager
activate BrowserPanel
BrowserPanel->>BrowserSearchOverlay: Create/Update searchState
BrowserPanel->>BP: Generate JavaScript (searchScript/nextScript)
deactivate BrowserPanel
activate BP
BP->>WKWebView: executeJavaScript()
activate WKWebView
WKWebView->>WKWebView: Highlight matches, track state
WKWebView-->>BP: Return JSON {total, current}
deactivate WKWebView
BP->>BrowserPanel: Parse result, update searchState
deactivate BP
activate BrowserSearchOverlay
BrowserSearchOverlay->>User: Display match count & highlights
deactivate BrowserSearchOverlay
User->>BrowserSearchOverlay: Click Next/Prev or press Cmd+G
activate BrowserSearchOverlay
BrowserSearchOverlay->>BrowserPanel: onNext() / onPrevious()
deactivate BrowserSearchOverlay
BrowserPanel->>BP: Execute nextScript() / previousScript()
BP->>WKWebView: executeJavaScript()
WKWebView-->>BP: Return updated position
BP->>BrowserPanel: Update searchState
BrowserSearchOverlay->>User: Reflect new position
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Greptile SummaryThis PR implements find-in-page functionality for browser panels, mirroring the existing terminal find feature. The implementation uses JavaScript TreeWalker to scan and highlight text matches with Key implementation details:
Architecture: Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant TabManager
participant BrowserPanel
participant WKWebView
participant BrowserSearchOverlay
User->>TabManager: Cmd+F (startSearch)
TabManager->>BrowserPanel: startFind()
BrowserPanel->>BrowserPanel: Create BrowserSearchState
BrowserPanel->>BrowserSearchOverlay: Display overlay
BrowserSearchOverlay->>User: Focus search field
User->>BrowserSearchOverlay: Type search query
BrowserSearchOverlay->>BrowserPanel: Update needle (debounced)
BrowserPanel->>BrowserPanel: Generate searchScript(query)
BrowserPanel->>WKWebView: evaluateJavaScript(searchScript)
WKWebView->>WKWebView: TreeWalker scan + mark elements
WKWebView-->>BrowserPanel: {total: N, current: 0}
BrowserPanel->>BrowserSearchOverlay: Update match count
User->>TabManager: Cmd+G (findNext)
TabManager->>BrowserPanel: findNext()
BrowserPanel->>WKWebView: evaluateJavaScript(nextScript)
WKWebView->>WKWebView: Update current class + scroll
WKWebView-->>BrowserPanel: {total: N, current: M}
BrowserPanel->>BrowserSearchOverlay: Update current match
User->>BrowserSearchOverlay: Esc (close)
BrowserSearchOverlay->>BrowserPanel: hideFind()
BrowserPanel->>WKWebView: evaluateJavaScript(clearScript)
WKWebView->>WKWebView: Remove mark elements
BrowserPanel->>BrowserPanel: Clear searchState
Last reviewed commit: 9488aeb |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/Find/BrowserFindJavaScript.swift`:
- Around line 38-42: The script assumes DOM roots exist; update searchScript to
guard before using document.body and document.head: check if document.body
exists before calling document.createTreeWalker (or fall back to
document.documentElement) and only create/use walker when a root is present
(referencing the walker and createTreeWalker usage and isVisible filter), and
check if document.head exists before calling document.head.appendChild(style)
(or fall back to appending to document.documentElement or doing an early return)
so neither TreeWalker creation nor style injection throws when roots are null.
In `@Sources/Find/BrowserSearchOverlay.swift`:
- Around line 45-55: Add unified debug logging by inserting calls to the free
function dlog("...") inside the new handlers wrapped in `#if` DEBUG / `#endif`
blocks; specifically, in the onExitCommand { onClose() } and onSubmit { ... }
handlers add dlog messages (e.g. dlog("BrowserSearchOverlay.onExitCommand") and
dlog("BrowserSearchOverlay.onSubmit
shift=\(NSEvent.modifierFlags.contains(.shift))")) before invoking onClose(),
onPrevious(), or onNext(). Do the same pattern for the other new key/focus/drag
handler functions referenced in this diff (use their function/closure names as
the message), ensuring every debug call is inside `#if` DEBUG / `#endif` and uses
the dlog free function.
- Around line 116-134: The drag gesture is currently applied after the infinite
.frame which causes drags to be recognized outside the visible bar — move the
.gesture(DragGesture()...) modifier so it is attached before the .frame(...) to
limit its hit area to the actual view; keep the existing logic that computes
centerPosition(for:corner:in:barSize:), creates newCenter and chooses newCorner
via closestCorner(to:in:), and updates corner and dragOffset inside
withAnimation. Also add debug logging wrapped in `#if` DEBUG / `#endif` in the
relevant handlers: call dlog(...) in the .onExitCommand handler, the .onSubmit
handler, the .onReceive that manages focus, and inside the DragGesture
.onChanged and .onEnded blocks to log value.translation and the computed
newCorner/newCenter to aid debugging. Ensure all added logs reference the same
identifying symbols (corner, dragOffset, centerPosition, closestCorner) so they
are easy to correlate.
In `@Sources/Panels/BrowserPanel.swift`:
- Around line 1456-1471: The current sink on searchState.$needle triggers
executeFindSearch asynchronously without cancelling prior work, so stale JS
responses can overwrite newer selected/total state; modify the implementation to
track and cancel or ignore prior in-flight find requests (e.g., add a stored
Task? findTask or a query token/UUID) inside the BrowserPanel and update
executeFindSearch to either return a cancellable Task or accept/return the query
token, cancel the previous Task before starting a new one (or compare tokens on
response and only apply results when the token matches the latest), and update
references to selected/total only for the latest query; apply the same pattern
for the other occurrences mentioned (lines ~2588-2616 and 2618-2629) referencing
executeFindSearch, searchNeedleCancellable, and the selected/total update logic.
- Around line 1455-1470: Replace any production NSLog that prints the raw search
needle with a privacy-safe debug-only log: remove the needle from the always-on
NSLog calls and instead wrap a dlog call in `#if` DEBUG / `#endif` that logs either
only the panel id or a redacted/masked version of the needle. Specifically,
update the NSLog in the search subscription (references:
searchNeedleCancellable, searchState.$needle, executeFindSearch) so it no longer
emits the raw `needle` to NSLog; if you still need to record it for debugging,
call dlog("Find: browser needle updated panel=%@ needle=%@", id.uuidString,
maskedNeedle) inside a DEBUG guard. Ensure the production path only logs
non-identifying context (e.g., panel id) and that executeFindSearch continues to
receive the full needle unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5b8fcf18-233f-4ba7-9988-d714c945fbd2
📒 Files selected for processing (8)
GhosttyTabs.xcodeproj/project.pbxprojSources/Find/BrowserFindJavaScript.swiftSources/Find/BrowserSearchOverlay.swiftSources/GhosttyTerminalView.swiftSources/Panels/BrowserPanel.swiftSources/Panels/BrowserPanelView.swiftSources/TabManager.swiftcmuxTests/BrowserFindJavaScriptTests.swift
Comment on lines
+38
to
+42
| const walker = document.createTreeWalker( | ||
| document.body, | ||
| NodeFilter.SHOW_TEXT, | ||
| { acceptNode(node) { return isVisible(node.parentElement) ? NodeFilter.FILTER_ACCEPT : NodeFilter.FILTER_REJECT; } } | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and get its structure
fd -t f "BrowserFindJavaScript.swift"Repository: manaflow-ai/cmux
Length of output: 100
🏁 Script executed:
# If found, read the file to inspect lines 38-42 and 91-99
cat -n Sources/Find/BrowserFindJavaScript.swift | head -120Repository: manaflow-ai/cmux
Length of output: 5733
🏁 Script executed:
# Check if the file contains any debug events (keys, mouse, focus, splits, tabs)
# These would need to be wrapped with dlog() and `#if` DEBUG blocks
rg -i 'console\.log|debug|key.*event|mouse.*event|focus.*event|split|tab' Sources/Find/BrowserFindJavaScript.swiftRepository: manaflow-ai/cmux
Length of output: 42
🏁 Script executed:
# Verify the complete context around lines 38-42 and 91-99 to see full guard conditions
sed -n '20,45p; 85,105p' Sources/Find/BrowserFindJavaScript.swiftRepository: manaflow-ai/cmux
Length of output: 2012
🏁 Script executed:
# Check if there are other unguarded DOM accesses in the file
rg -n 'document\.(body|head|getElementById|createElement|createTreeWalker)' Sources/Find/BrowserFindJavaScript.swiftRepository: manaflow-ai/cmux
Length of output: 509
🏁 Script executed:
# Verify the context around isVisible to understand the guard logic better
sed -n '28,37p' Sources/Find/BrowserFindJavaScript.swiftRepository: manaflow-ai/cmux
Length of output: 501
Guard against missing DOM roots before TreeWalker and style injection.
searchScript assumes document.body and document.head always exist. In non-HTML documents or during transient navigation states, these can be null, causing exceptions that leave find state broken.
Line 39: createTreeWalker(document.body, ...) executes without checking if document.body exists.
Line 98: document.head.appendChild(style) executes without checking if document.head exists.
Suggested fixes
const query = "\(escaped)";
if (!query) return JSON.stringify({total: 0, current: 0});
+ if (!document.body) return JSON.stringify({total: 0, current: 0});
if (!document.getElementById('__cmux-find-style')) {
const style = document.createElement('style');
style.id = '__cmux-find-style';
style.textContent = `...`;
- document.head.appendChild(style);
+ (document.head || document.documentElement).appendChild(style);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/Find/BrowserFindJavaScript.swift` around lines 38 - 42, The script
assumes DOM roots exist; update searchScript to guard before using document.body
and document.head: check if document.body exists before calling
document.createTreeWalker (or fall back to document.documentElement) and only
create/use walker when a root is present (referencing the walker and
createTreeWalker usage and isVisible filter), and check if document.head exists
before calling document.head.appendChild(style) (or fall back to appending to
document.documentElement or doing an early return) so neither TreeWalker
creation nor style injection throws when roots are null.
Comment on lines
+45
to
+55
| .onExitCommand { | ||
| onClose() | ||
| } | ||
| .onSubmit { | ||
| // onSubmit fires only after IME composition is committed. | ||
| if NSEvent.modifierFlags.contains(.shift) { | ||
| onPrevious() | ||
| } else { | ||
| onNext() | ||
| } | ||
| } |
There was a problem hiding this comment.
Add unified debug logging for new key/focus/drag handlers.
The new handlers currently miss dlog instrumentation in #if DEBUG blocks.
🛠️ Suggested patch
@@
.onExitCommand {
+#if DEBUG
+ dlog("browser.findbar.escape panel=\(panelId.uuidString.prefix(5))")
+#endif
onClose()
}
.onSubmit {
+ `#if` DEBUG
+ let isShiftSubmit = NSEvent.modifierFlags.contains(.shift)
+ dlog("browser.findbar.submit panel=\(panelId.uuidString.prefix(5)) shift=\(isShiftSubmit ? 1 : 0)")
+ `#endif`
// onSubmit fires only after IME composition is committed.
if NSEvent.modifierFlags.contains(.shift) {
@@
.onReceive(NotificationCenter.default.publisher(for: .browserSearchFocus)) { notification in
guard let notifiedPanelId = notification.object as? UUID,
notifiedPanelId == panelId else { return }
+#if DEBUG
+ dlog("browser.findbar.focusRequest panel=\(panelId.uuidString.prefix(5))")
+#endif
DispatchQueue.main.async {
isSearchFieldFocused = true
}
}
@@
.onEnded { value in
+#if DEBUG
+ dlog("browser.findbar.drag.end panel=\(panelId.uuidString.prefix(5)) dx=\(Int(value.translation.width)) dy=\(Int(value.translation.height))")
+#endif
let centerPos = centerPosition(for: corner, in: geo.size, barSize: barSize)As per coding guidelines, **/*.swift: All debug events (keys, mouse, focus, splits, tabs) must go to the unified debug log, with free function dlog("message") wrapping all call sites in #if DEBUG / #endif.
Also applies to: 100-106, 118-133
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/Find/BrowserSearchOverlay.swift` around lines 45 - 55, Add unified
debug logging by inserting calls to the free function dlog("...") inside the new
handlers wrapped in `#if` DEBUG / `#endif` blocks; specifically, in the
onExitCommand { onClose() } and onSubmit { ... } handlers add dlog messages
(e.g. dlog("BrowserSearchOverlay.onExitCommand") and
dlog("BrowserSearchOverlay.onSubmit
shift=\(NSEvent.modifierFlags.contains(.shift))")) before invoking onClose(),
onPrevious(), or onNext(). Do the same pattern for the other new key/focus/drag
handler functions referenced in this diff (use their function/closure names as
the message), ensuring every debug call is inside `#if` DEBUG / `#endif` and uses
the dlog free function.
Comment on lines
+116
to
+134
| .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: corner.alignment) | ||
| .gesture( | ||
| DragGesture() | ||
| .onChanged { value in | ||
| dragOffset = value.translation | ||
| } | ||
| .onEnded { value in | ||
| let centerPos = centerPosition(for: corner, in: geo.size, barSize: barSize) | ||
| let newCenter = CGPoint( | ||
| x: centerPos.x + value.translation.width, | ||
| y: centerPos.y + value.translation.height | ||
| ) | ||
| let newCorner = closestCorner(to: newCenter, in: geo.size) | ||
| withAnimation(.easeOut(duration: 0.2)) { | ||
| corner = newCorner | ||
| dragOffset = .zero | ||
| } | ||
| } | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# First, check if the file exists and its general structure
echo "=== File existence check ==="
ls -la Sources/Find/BrowserSearchOverlay.swift 2>/dev/null && echo "File found" || echo "File not found"
# Get file size
echo -e "\n=== File size ==="
wc -l Sources/Find/BrowserSearchOverlay.swift 2>/dev/null || echo "Cannot read file"Repository: manaflow-ai/cmux
Length of output: 251
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the file around lines 116-134 to verify the modifier order
echo "=== Code at lines 110-140 ==="
sed -n '110,140p' Sources/Find/BrowserSearchOverlay.swift 2>/dev/null || echo "Cannot read file"
# Also check for dlog usage in the file
echo -e "\n=== dlog usage in file ==="
rg -n 'dlog' Sources/Find/BrowserSearchOverlay.swift 2>/dev/null || echo "No dlog found"
# Check for event handlers and gesture modifiers
echo -e "\n=== Event handlers and gesture modifiers ==="
rg -n -i 'onExitCommand|onSubmit|browserSearchFocus|DragGesture|\.gesture' Sources/Find/BrowserSearchOverlay.swift 2>/dev/null || echo "Pattern not found"Repository: manaflow-ai/cmux
Length of output: 2028
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get the full file to understand overall context
echo "=== Full file content ==="
cat -n Sources/Find/BrowserSearchOverlay.swift 2>/dev/null || echo "Cannot read file"Repository: manaflow-ai/cmux
Length of output: 8186
Drag gesture is scoped too broadly and can steal panel interactions.
Because .gesture is applied after an infinite frame (line 116–117), dragging is recognized outside the visible bar area. Reorder modifiers so the gesture is attached before the infinite frame.
Additionally, key/focus/mouse event handlers must include debug logging per guidelines:
.onExitCommand(line 45),.onSubmit(line 48),.onReceivefor focus (line 100), and.gesturedrag events (line 118) are missingdlogcalls wrapped in#if DEBUG/#endif.
Suggested fix for gesture scope
.padding(padding)
.offset(dragOffset)
- .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: corner.alignment)
.gesture(
DragGesture()
@@
}
)
+ .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: corner.alignment)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/Find/BrowserSearchOverlay.swift` around lines 116 - 134, The drag
gesture is currently applied after the infinite .frame which causes drags to be
recognized outside the visible bar — move the .gesture(DragGesture()...)
modifier so it is attached before the .frame(...) to limit its hit area to the
actual view; keep the existing logic that computes
centerPosition(for:corner:in:barSize:), creates newCenter and chooses newCorner
via closestCorner(to:in:), and updates corner and dragOffset inside
withAnimation. Also add debug logging wrapped in `#if` DEBUG / `#endif` in the
relevant handlers: call dlog(...) in the .onExitCommand handler, the .onSubmit
handler, the .onReceive that manages focus, and inside the DragGesture
.onChanged and .onEnded blocks to log value.translation and the computed
newCorner/newCenter to aid debugging. Ensure all added logs reference the same
identifying symbols (corner, dragOffset, centerPosition, closestCorner) so they
are easy to correlate.
Comment on lines
+1455
to
+1470
| NSLog("Find: browser search state created panel=%@", id.uuidString) | ||
| searchNeedleCancellable = searchState.$needle | ||
| .removeDuplicates() | ||
| .map { needle -> AnyPublisher<String, Never> in | ||
| if needle.isEmpty || needle.count >= 3 { | ||
| return Just(needle).eraseToAnyPublisher() | ||
| } | ||
| return Just(needle) | ||
| .delay(for: .milliseconds(300), scheduler: DispatchQueue.main) | ||
| .eraseToAnyPublisher() | ||
| } | ||
| .switchToLatest() | ||
| .sink { [weak self] needle in | ||
| guard let self else { return } | ||
| NSLog("Find: browser needle updated panel=%@ needle=%@", self.id.uuidString, needle) | ||
| self.executeFindSearch(needle) |
There was a problem hiding this comment.
Avoid logging raw find queries in production.
Line 1469 logs the user’s search text, which is a privacy/compliance risk and should not be emitted via always-on NSLog.
🔧 Suggested fix
- NSLog("Find: browser search state created panel=%@", id.uuidString)
+ `#if` DEBUG
+ dlog("find.browser.state.created panel=\(id.uuidString.prefix(5))")
+ `#endif`
...
- NSLog("Find: browser needle updated panel=%@ needle=%@", self.id.uuidString, needle)
+ `#if` DEBUG
+ dlog("find.browser.needle.updated panel=\(self.id.uuidString.prefix(5)) chars=\(needle.count)")
+ `#endif`
...
- NSLog("Find: browser search state cleared panel=%@", id.uuidString)
+ `#if` DEBUG
+ dlog("find.browser.state.cleared panel=\(id.uuidString.prefix(5))")
+ `#endif`Also applies to: 1474-1474
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/Panels/BrowserPanel.swift` around lines 1455 - 1470, Replace any
production NSLog that prints the raw search needle with a privacy-safe
debug-only log: remove the needle from the always-on NSLog calls and instead
wrap a dlog call in `#if` DEBUG / `#endif` that logs either only the panel id or a
redacted/masked version of the needle. Specifically, update the NSLog in the
search subscription (references: searchNeedleCancellable, searchState.$needle,
executeFindSearch) so it no longer emits the raw `needle` to NSLog; if you still
need to record it for debugging, call dlog("Find: browser needle updated
panel=%@ needle=%@", id.uuidString, maskedNeedle) inside a DEBUG guard. Ensure
the production path only logs non-identifying context (e.g., panel id) and that
executeFindSearch continues to receive the full needle unchanged.
Comment on lines
+1456
to
+1471
| searchNeedleCancellable = searchState.$needle | ||
| .removeDuplicates() | ||
| .map { needle -> AnyPublisher<String, Never> in | ||
| if needle.isEmpty || needle.count >= 3 { | ||
| return Just(needle).eraseToAnyPublisher() | ||
| } | ||
| return Just(needle) | ||
| .delay(for: .milliseconds(300), scheduler: DispatchQueue.main) | ||
| .eraseToAnyPublisher() | ||
| } | ||
| .switchToLatest() | ||
| .sink { [weak self] needle in | ||
| guard let self else { return } | ||
| NSLog("Find: browser needle updated panel=%@ needle=%@", self.id.uuidString, needle) | ||
| self.executeFindSearch(needle) | ||
| } |
There was a problem hiding this comment.
Guard against stale async find results overwriting newer state.
Current find execution spawns uncancelled tasks, so older JS responses can win the race and update selected/total for an outdated query.
🔧 Suggested fix
- private var searchNeedleCancellable: AnyCancellable?
+ private var searchNeedleCancellable: AnyCancellable?
+ private var searchExecutionTask: Task<Void, Never>?
...
} else if oldValue != nil {
searchNeedleCancellable = nil
+ searchExecutionTask?.cancel()
+ searchExecutionTask = nil
NSLog("Find: browser search state cleared panel=%@", id.uuidString)
executeFindClear()
}
...
private func executeFindSearch(_ needle: String) {
guard !needle.isEmpty else {
executeFindClear()
searchState?.selected = nil
searchState?.total = nil
return
}
- Task { `@MainActor` [weak self] in
+ searchExecutionTask?.cancel()
+ searchExecutionTask = Task { `@MainActor` [weak self] in
guard let self else { return }
let js = BrowserFindJavaScript.searchScript(query: needle)
do {
let result = try await self.webView.evaluateJavaScript(js)
+ guard !Task.isCancelled, self.searchState?.needle == needle else { return }
self.parseFindResult(result)
} catch {
NSLog("Find: browser JS search error: %@", error.localizedDescription)
}
}
}
private func executeFindClear() {
- Task { `@MainActor` [weak self] in
+ searchExecutionTask?.cancel()
+ searchExecutionTask = Task { `@MainActor` [weak self] in
guard let self else { return }
do {
_ = try await self.webView.evaluateJavaScript(BrowserFindJavaScript.clearScript())
} catch {
NSLog("Find: browser JS clear error: %@", error.localizedDescription)
}
}
}Also applies to: 2588-2616, 2618-2629
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/Panels/BrowserPanel.swift` around lines 1456 - 1471, The current sink
on searchState.$needle triggers executeFindSearch asynchronously without
cancelling prior work, so stale JS responses can overwrite newer selected/total
state; modify the implementation to track and cancel or ignore prior in-flight
find requests (e.g., add a stored Task? findTask or a query token/UUID) inside
the BrowserPanel and update executeFindSearch to either return a cancellable
Task or accept/return the query token, cancel the previous Task before starting
a new one (or compare tokens on response and only apply results when the token
matches the latest), and update references to selected/total only for the latest
query; apply the same pattern for the other occurrences mentioned (lines
~2588-2616 and 2618-2629) referencing executeFindSearch,
searchNeedleCancellable, and the selected/total update logic.
4 tasks done
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/Find/BrowserSearchOverlay.swift">
<violation number="1" location="Sources/Find/BrowserSearchOverlay.swift:4">
P3: BrowserSearchOverlay is largely a copy of SurfaceSearchOverlay. Consider extracting a shared generic overlay to avoid duplicated UI/drag logic that will need to be updated in two places.</violation>
</file>
<file name="Sources/Find/BrowserFindJavaScript.swift">
<violation number="1" location="Sources/Find/BrowserFindJavaScript.swift:24">
P2: Add a null check for `document.body` to prevent a `TypeError` crash when searching in non-HTML or incomplete documents.</violation>
<violation number="2" location="Sources/Find/BrowserFindJavaScript.swift:30">
P1: SVG elements are not correctly skipped because their `tagName` is lowercase (`"svg"`). Convert `el.tagName` to uppercase before checking the `SKIP_TAGS` set.</violation>
<violation number="3" location="Sources/Find/BrowserFindJavaScript.swift:54">
P0: String length misalignment caused by `.toLowerCase()` will permanently corrupt text content. Use `RegExp` with the case-insensitive flag instead.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const parts = []; | ||
| let lastEnd = 0; | ||
| while (true) { | ||
| const idx = lowerText.indexOf(lowerQuery, startIndex); |
There was a problem hiding this comment.
P0: String length misalignment caused by .toLowerCase() will permanently corrupt text content. Use RegExp with the case-insensitive flag instead.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/Find/BrowserFindJavaScript.swift, line 54:
<comment>String length misalignment caused by `.toLowerCase()` will permanently corrupt text content. Use `RegExp` with the case-insensitive flag instead.</comment>
<file context>
@@ -0,0 +1,207 @@
+ const parts = [];
+ let lastEnd = 0;
+ while (true) {
+ const idx = lowerText.indexOf(lowerQuery, startIndex);
+ if (idx === -1) break;
+ parts.push({ start: idx, end: idx + query.length });
</file context>
| const SKIP_TAGS = new Set(['SCRIPT','STYLE','NOSCRIPT','TEMPLATE','IFRAME','SVG']); | ||
| const isVisible = (el) => { | ||
| while (el && el !== document.body) { | ||
| if (SKIP_TAGS.has(el.tagName)) return false; |
There was a problem hiding this comment.
P1: SVG elements are not correctly skipped because their tagName is lowercase ("svg"). Convert el.tagName to uppercase before checking the SKIP_TAGS set.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/Find/BrowserFindJavaScript.swift, line 30:
<comment>SVG elements are not correctly skipped because their `tagName` is lowercase (`"svg"`). Convert `el.tagName` to uppercase before checking the `SKIP_TAGS` set.</comment>
<file context>
@@ -0,0 +1,207 @@
+ const SKIP_TAGS = new Set(['SCRIPT','STYLE','NOSCRIPT','TEMPLATE','IFRAME','SVG']);
+ const isVisible = (el) => {
+ while (el && el !== document.body) {
+ if (SKIP_TAGS.has(el.tagName)) return false;
+ if (el.getAttribute('aria-hidden') === 'true') return false;
+ const st = getComputedStyle(el);
</file context>
| \(clearBody) | ||
|
|
||
| const query = "\(escaped)"; | ||
| if (!query) return JSON.stringify({total: 0, current: 0}); |
There was a problem hiding this comment.
P2: Add a null check for document.body to prevent a TypeError crash when searching in non-HTML or incomplete documents.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/Find/BrowserFindJavaScript.swift, line 24:
<comment>Add a null check for `document.body` to prevent a `TypeError` crash when searching in non-HTML or incomplete documents.</comment>
<file context>
@@ -0,0 +1,207 @@
+ \(clearBody)
+
+ const query = "\(escaped)";
+ if (!query) return JSON.stringify({total: 0, current: 0});
+
+ const lowerQuery = query.toLowerCase();
</file context>
| import Bonsplit | ||
| import SwiftUI | ||
|
|
||
| struct BrowserSearchOverlay: View { |
There was a problem hiding this comment.
P3: BrowserSearchOverlay is largely a copy of SurfaceSearchOverlay. Consider extracting a shared generic overlay to avoid duplicated UI/drag logic that will need to be updated in two places.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/Find/BrowserSearchOverlay.swift, line 4:
<comment>BrowserSearchOverlay is largely a copy of SurfaceSearchOverlay. Consider extracting a shared generic overlay to avoid duplicated UI/drag logic that will need to be updated in two places.</comment>
<file context>
@@ -0,0 +1,183 @@
+import Bonsplit
+import SwiftUI
+
+struct BrowserSearchOverlay: View {
+ let panelId: UUID
+ @ObservedObject var searchState: BrowserSearchState
</file context>
Contributor
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Contributor
|
Thank you for the contribution! Will merge for now, but there's a slight issue, notice the letter "A" in "Avoid": Screen.Recording.2026-03-04.at.4.13.10.PM.movAdditionally the cmd+f popover is slightly covering the browser omnibar. Would appreciate follow-up PRs for those! |
This was referenced Mar 5, 2026
This was referenced Mar 14, 2026
This was referenced Mar 30, 2026
bn-l
pushed a commit
to bn-l/cmux
that referenced
this pull request
Apr 3, 2026
…ow-ai#875) JavaScript-based find using TreeWalker + <mark> highlights with match counter, next/previous navigation, and drag-to-corner overlay matching the existing terminal find bar. - BrowserFindJavaScript: JS generation for search/next/prev/clear - BrowserSearchOverlay: SwiftUI overlay with IME-safe onSubmit - BrowserSearchState: Observable state (needle/selected/total) - TabManager routing: Cmd+F/G dispatches to browser when focused - Visibility filter: skips script/style/hidden/aria-hidden elements - Stale DOM guard: isConnected check in next/previous scripts - Navigation cleanup: clears find on didFinish and didFailNavigation Co-authored-by: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com>
This was referenced May 8, 2026
4 of 6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #837
<mark>highlightsImplementation
SurfaceSearchOverlaydesign, with IME-safeonSubmitinstead ofonKeyPress(.return)startSearch/findNext/findPrevious/hideFinddispatch to browser when focuseddidFinishanddidFailNavigationTest plan
cmuxTests/BrowserFindJavaScriptTests)Note
Terminal find bar has the same Japanese IME issue (
onKeyPress(.return)intercepts IME confirm). Will address in a separate PR.Summary by cubic
Adds find-in-page (Cmd+F) to browser panels with in-page highlights and a draggable find bar. Implements Linear #837 and routes search commands to the focused browser panel while keeping terminal find behavior unchanged.
Written for commit 5f37ef8. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
New Features
Tests