Skip to content

Add Safari DevTools diagnostic logging #1161

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

Closed
lawrencecchen wants to merge 2 commits into from
Closed

Add Safari DevTools diagnostic logging #1161

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7977484
Add Safari DevTools diagnostic logging
lawrencecchen Mar 10, 2026
6dbf582
Ignore repeated browser DevTools shortcuts
lawrencecchen Mar 10, 2026
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
12 changes: 12 additions & 0 deletions Sources/AppDelegate.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7689,6 +7689,12 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
// - Option+Command+I => Show/Toggle Web Inspector
// - Option+Command+C => Show JavaScript Console
if matchShortcut(event: event, shortcut: KeyboardShortcutSettings.shortcut(for: .toggleBrowserDeveloperTools)) {
if event.isARepeat {
#if DEBUG
logDeveloperToolsShortcutSnapshot(phase: "toggle.repeatIgnored", event: event, didHandle: true)
#endif
return true
}
Comment on lines +7692 to +7697

@coderabbitai coderabbitai Bot Mar 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Don't log ignored repeats as handled.

These branches intentionally skip the action, but the new trace records handled=1. In the surrounding post/tick logs, handled means the browser action actually ran, so this will make repeat-suppression look like a successful inspector/console open and muddy the diagnostics.

🔧 Proposed fix 
             if event.isARepeat {
 `#if` DEBUG
-                logDeveloperToolsShortcutSnapshot(phase: "toggle.repeatIgnored", event: event, didHandle: true)
+                logDeveloperToolsShortcutSnapshot(phase: "toggle.repeatIgnored", event: event)
 `#endif`
                 return true
             }
@@
             if event.isARepeat {
 `#if` DEBUG
-                logDeveloperToolsShortcutSnapshot(phase: "console.repeatIgnored", event: event, didHandle: true)
+                logDeveloperToolsShortcutSnapshot(phase: "console.repeatIgnored", event: event)
 `#endif`
                 return true
             }

Also applies to: 7713-7718

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@Sources/AppDelegate.swift` around lines 7692 - 7697, The logs for
repeat-suppressed shortcuts are incorrectly marking the action as handled;
update the calls to logDeveloperToolsShortcutSnapshot (the branches checking
event.isARepeat) to record didHandle: false (or remove the handled flag) so
repeats are logged as ignored rather than "handled"; make the same change for
both occurrences (the toggle.repeatIgnored branch and the other
repeat-suppression branch around the second occurrence) so diagnostic traces
correctly reflect that the action did not run.

#if DEBUG
logDeveloperToolsShortcutSnapshot(phase: "toggle.pre", event: event)
#endif
Expand All @@ -7704,6 +7710,12 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
}

if matchShortcut(event: event, shortcut: KeyboardShortcutSettings.shortcut(for: .showBrowserJavaScriptConsole)) {
if event.isARepeat {
#if DEBUG
logDeveloperToolsShortcutSnapshot(phase: "console.repeatIgnored", event: event, didHandle: true)
#endif
return true
}
#if DEBUG
logDeveloperToolsShortcutSnapshot(phase: "console.pre", event: event)
#endif
Expand Down
93 changes: 87 additions & 6 deletions Sources/BrowserWindowPortal.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2096,8 +2096,24 @@ final class WindowBrowserPortal: NSObject {
/// Used when a bind is deferred (host not yet in window) so stale portal syncs
/// do not keep an old anchor visible.
func updateEntryVisibility(forWebViewId webViewId: ObjectIdentifier, visibleInUI: Bool, zPriority: Int) {
guard var entry = entriesByWebViewId[webViewId] else { return }
guard var entry = entriesByWebViewId[webViewId] else {
#if DEBUG
dlog(
"browser.portal.visibility.skip webId=\(String(describing: webViewId)) " +
"reason=missing_entry visible=\(visibleInUI ? 1 : 0) z=\(zPriority)"
)
#endif
return
}
guard entry.visibleInUI != visibleInUI || entry.zPriority != zPriority else { return }
#if DEBUG
dlog(
"browser.portal.visibility web=\(browserPortalDebugToken(entry.webView)) " +
"container=\(browserPortalDebugToken(entry.containerView)) " +
"visible=\(visibleInUI ? 1 : 0) prevVisible=\(entry.visibleInUI ? 1 : 0) " +
"z=\(zPriority) prevZ=\(entry.zPriority)"
)
#endif
entry.visibleInUI = visibleInUI
entry.zPriority = zPriority
entriesByWebViewId[webViewId] = entry
Expand All @@ -2110,7 +2126,22 @@ final class WindowBrowserPortal: NSObject {
}

func hideWebView(withId webViewId: ObjectIdentifier, source: String = "externalHide") {
guard var entry = entriesByWebViewId[webViewId] else { return }
guard var entry = entriesByWebViewId[webViewId] else {
#if DEBUG
dlog(
"browser.portal.hide.skip webId=\(String(describing: webViewId)) " +
"reason=missing_entry source=\(source)"
)
#endif
return
}
#if DEBUG
dlog(
"browser.portal.hide web=\(browserPortalDebugToken(entry.webView)) " +
"container=\(browserPortalDebugToken(entry.containerView)) " +
"source=\(source) visible=\(entry.visibleInUI ? 1 : 0)"
)
#endif
entry.visibleInUI = false
entry.zPriority = 0
entriesByWebViewId[webViewId] = entry
Expand Down Expand Up @@ -2164,8 +2195,25 @@ final class WindowBrowserPortal: NSObject {
let webView = entry.webView,
let containerView = entry.containerView,
!containerView.isHidden else {
#if DEBUG
let entry = entriesByWebViewId[webViewId]
dlog(
"browser.portal.refresh.skip webId=\(String(describing: webViewId)) " +
"reason=missing_refresh_target request=\(reason) " +
"hasEntry=\(entry == nil ? 0 : 1) " +
"hasWeb=\(entry?.webView == nil ? 0 : 1) " +
"hasContainer=\(entry?.containerView == nil ? 0 : 1) " +
"containerHidden=\(entry?.containerView?.isHidden == true ? 1 : 0)"
)
#endif
return
}
#if DEBUG
dlog(
"browser.portal.refresh.begin web=\(browserPortalDebugToken(webView)) " +
"container=\(browserPortalDebugToken(containerView)) reason=\(reason)"
)
#endif
refreshHostedWebViewPresentation(
webView,
in: containerView,
Expand Down Expand Up @@ -2960,7 +3008,16 @@ enum BrowserWindowPortalRegistry {
}

static func bind(webView: WKWebView, to anchorView: NSView, visibleInUI: Bool, zPriority: Int = 0) {
guard let window = anchorView.window else { return }
guard let window = anchorView.window else {
#if DEBUG
dlog(
"browser.portal.bind.skip web=\(browserPortalDebugToken(webView)) " +
"anchor=\(browserPortalDebugToken(anchorView)) reason=no_window " +
"visible=\(visibleInUI ? 1 : 0) z=\(zPriority)"
)
#endif
return
}

let windowId = ObjectIdentifier(window)
let webViewId = ObjectIdentifier(webView)
Expand All @@ -2987,7 +3044,15 @@ enum BrowserWindowPortalRegistry {
static func updateEntryVisibility(for webView: WKWebView, visibleInUI: Bool, zPriority: Int) {
let webViewId = ObjectIdentifier(webView)
guard let windowId = webViewToWindowId[webViewId],
let portal = portalsByWindowId[windowId] else { return }
let portal = portalsByWindowId[windowId] else {
#if DEBUG
dlog(
"browser.portal.visibility.skip web=\(browserPortalDebugToken(webView)) " +
"reason=missing_window_mapping visible=\(visibleInUI ? 1 : 0) z=\(zPriority)"
)
#endif
return
}
portal.updateEntryVisibility(forWebViewId: webViewId, visibleInUI: visibleInUI, zPriority: zPriority)
}

Expand All @@ -3003,7 +3068,15 @@ enum BrowserWindowPortalRegistry {
static func hide(webView: WKWebView, source: String = "externalHide") {
let webViewId = ObjectIdentifier(webView)
guard let windowId = webViewToWindowId[webViewId],
let portal = portalsByWindowId[windowId] else { return }
let portal = portalsByWindowId[windowId] else {
#if DEBUG
dlog(
"browser.portal.hide.skip web=\(browserPortalDebugToken(webView)) " +
"reason=missing_window_mapping source=\(source)"
)
#endif
return
}
portal.hideWebView(withId: webViewId, source: source)
}

Expand Down Expand Up @@ -3053,7 +3126,15 @@ enum BrowserWindowPortalRegistry {
static func refresh(webView: WKWebView, reason: String) {
let webViewId = ObjectIdentifier(webView)
guard let windowId = webViewToWindowId[webViewId],
let portal = portalsByWindowId[windowId] else { return }
let portal = portalsByWindowId[windowId] else {
#if DEBUG
dlog(
"browser.portal.refresh.skip web=\(browserPortalDebugToken(webView)) " +
"reason=missing_window_mapping request=\(reason)"
)
#endif
return
}
portal.forceRefreshWebView(withId: webViewId, reason: reason)
}

Expand Down
Loading
Loading