fix: prevent mouse event passthrough on floating sidebar#236
Conversation
Greptile SummaryThis PR injects a transparent shield Key changes:
Confidence Score: 2/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant BrowserView
participant FloatingSidebarOverlay
participant TabManager
participant Tab
participant WKWebView
User->>FloatingSidebarOverlay: Mouse enters sidebar strip
FloatingSidebarOverlay->>BrowserView: showFloatingSidebar = true
BrowserView->>BrowserView: onChange(showFloatingSidebar)
BrowserView->>WKWebView: callAsyncJavaScript — inject shield div
User->>TabManager: Switch to uninitialized tab
TabManager->>BrowserView: onChange(activeTab)
BrowserView->>WKWebView: evaluateJavaScript — remove shield from oldTab
BrowserView->>WKWebView: callAsyncJavaScript — inject shield into placeholder WKWebView ⚠️
Note over BrowserView,WKWebView: Shield is in the placeholder WebView!
BrowserView->>Tab: asyncAfter(0.01s) restoreTransientState()
Tab->>WKWebView: Creates brand-new WKWebView (discards placeholder)
Note over Tab,WKWebView: Shield is lost — new WKWebView has no shield
User->>WKWebView: Navigate to new URL (same tab)
WKWebView->>WKWebView: Page loads, old DOM (with shield) is destroyed
Note over BrowserView,WKWebView: No re-injection triggered — sidebar still visible but unshielded
Last reviewed commit: 4934c44 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| .onChange(of: tabManager.activeTab) { oldTab, newTab in | ||
| if showFloatingSidebar { | ||
| oldTab?.webView.evaluateJavaScript(Self.removeShieldJS, completionHandler: nil) | ||
| injectSidebarMouseShield(visible: true) | ||
| } |
There was a problem hiding this comment.
Shield injected into webView that gets replaced by restoreTransientState
When the active tab switches to one where !tab.isWebViewReady, the injection and the tab restoration race:
injectSidebarMouseShield(visible: true)fires immediately, callingcallAsyncJavaScriptontab.webView— which at this point is the uninitialized placeholderWKWebViewwith no loaded document.- 10 ms later,
restoreTransientStatecreates a brand-newWKWebView, assigns it totab.webView, and loads the URL.
The injected shield ends up in the discarded placeholder webView; the real webView that the user sees never gets the shield. The floating sidebar therefore has no DOM-level mouse-event protection on any freshly-restored tab.
The fix is to move the shield injection inside the asyncAfter block, after restoreTransientState has run:
.onChange(of: tabManager.activeTab) { oldTab, newTab in
if showFloatingSidebar {
oldTab?.webView.evaluateJavaScript(Self.removeShieldJS, completionHandler: nil)
}
if let tab = newTab, !tab.isWebViewReady {
DispatchQueue.main.asyncAfter(deadline: .now() + 0.01) {
tab.restoreTransientState(
historyManager: historyManager,
downloadManager: downloadManager,
tabManager: tabManager,
isPrivate: privacyMode.isPrivate
)
if self.showFloatingSidebar {
self.injectSidebarMouseShield(visible: true)
}
}
} else if showFloatingSidebar {
injectSidebarMouseShield(visible: true)
}
}| .onChange(of: showFloatingSidebar) { _, visible in | ||
| injectSidebarMouseShield(visible: visible) | ||
| } |
There was a problem hiding this comment.
Shield lost on same-tab page navigation
The shield is re-injected in two places: when showFloatingSidebar toggles (here) and when tabManager.activeTab changes. However, if the user navigates to a new URL within the same tab while the floating sidebar is visible, the old DOM (including the shield div) is torn down by WebKit, and neither of these observers fires — so the sidebar has no shield for the remainder of that browsing session until the user closes and reopens it.
Consider hooking into WKNavigationDelegate.webView(_:didFinish:) (or a dedicated notification) to re-inject the shield after navigation commits when the sidebar is visible. For example, in WebViewNavigationDelegate, you could post a notification on didFinishNavigation and observe it here:
.onReceive(NotificationCenter.default.publisher(for: .webViewDidFinishNavigation)) { _ in
if showFloatingSidebar {
injectSidebarMouseShield(visible: true)
}
}| private func injectSidebarMouseShield(visible: Bool) { | ||
| guard let webView = tabManager.activeTab?.webView else { return } | ||
| if visible { | ||
| let side = sidebarManager.sidebarPosition == .primary ? "left" : "right" | ||
| let widthVW = clampedSidebarFraction * 100 | ||
| webView.callAsyncJavaScript( | ||
| """ | ||
| var e = document.getElementById('ora-sb-shield'); | ||
| if (e) e.remove(); | ||
| var d = document.createElement('div'); | ||
| d.id = 'ora-sb-shield'; | ||
| d.style.position = 'fixed'; | ||
| d.style.top = '0'; | ||
| d.style[side] = '0'; | ||
| d.style.width = widthVW + 'vw'; | ||
| d.style.height = '100vh'; | ||
| d.style.zIndex = '2147483647'; | ||
| d.style.pointerEvents = 'auto'; | ||
| d.style.cursor = 'default'; | ||
| document.documentElement.appendChild(d); | ||
| """, | ||
| arguments: ["side": side, "widthVW": widthVW], | ||
| in: nil, | ||
| in: .page, | ||
| completionHandler: nil | ||
| ) | ||
| } else { | ||
| webView.evaluateJavaScript(Self.removeShieldJS, completionHandler: nil) | ||
| } | ||
| } |
There was a problem hiding this comment.
OraWebView secondary defense layer described but not implemented
The PR description states:
"Introduces
OraWebView(WKWebView subclass) with amouseMovedoverride as a secondary defense layer"
However, no OraWebView class exists anywhere in the codebase — Tab.webView is still typed as WKWebView and initialized directly as one. If this secondary layer was intentionally left out, the PR description should be updated. If it was accidentally omitted, the protection is incomplete: the mouseMoved override was expected to serve as a fallback for cases where the DOM shield is not yet present (e.g., immediately after a tab switch).
1 participant
Summary
<div>into the web page when the floating sidebar is visible, blocking hover effects and cursor changes on web content behind the sidebarcallAsyncJavaScriptwith parameterized arguments to avoid JS injection risks