Merge pull request #222 from manaflow-ai/feat-pr117-review-followup

Follow up browser DevTools and portal review comments from PR 117

Sources/BrowserWindowPortal.swift

@@ -317,6 +317,16 @@ final class WindowBrowserPortal: NSObject {
         entry.containerView?.removeFromSuperview()
     }
 
+    /// Update the visibleInUI/zPriority state on an existing entry without rebinding.
+    /// 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 }
+        entry.visibleInUI = visibleInUI
+        entry.zPriority = zPriority
+        entriesByWebViewId[webViewId] = entry
+    }
+
     func bind(webView: WKWebView, to anchorView: NSView, visibleInUI: Bool, zPriority: Int = 0) {
         guard ensureInstalled() else { return }
 
@@ -853,6 +863,15 @@ enum BrowserWindowPortalRegistry {
         portal.synchronizeWebViewForAnchor(anchorView)
     }
 
+    /// Update visibleInUI/zPriority on an existing portal entry without rebinding.
+    /// Called when a bind is deferred because the new host is temporarily off-window.
+    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 }
+        portal.updateEntryVisibility(forWebViewId: webViewId, visibleInUI: visibleInUI, zPriority: zPriority)
+    }
+
     static func detach(webView: WKWebView) {
         let webViewId = ObjectIdentifier(webView)
         guard let windowId = webViewToWindowId.removeValue(forKey: webViewId) else { return }

Sources/Panels/BrowserPanel.swift

@@ -1589,14 +1589,21 @@ extension BrowserPanel {
         )
 #endif
         guard let inspector = webView.cmuxInspectorObject() else { return false }
-        let visible = inspector.cmuxCallBool(selector: NSSelectorFromString("isVisible")) ?? false
+        let isVisibleSelector = NSSelectorFromString("isVisible")
+        let visible = inspector.cmuxCallBool(selector: isVisibleSelector) ?? false
         let targetVisible = !visible
         let selector = NSSelectorFromString(targetVisible ? "show" : "close")
         guard inspector.responds(to: selector) else { return false }
         inspector.cmuxCallVoid(selector: selector)
         preferredDeveloperToolsVisible = targetVisible
         if targetVisible {
-            developerToolsRestoreRetryAttempt = 0
+            let visibleAfterToggle = inspector.cmuxCallBool(selector: isVisibleSelector) ?? false
+            if visibleAfterToggle {
+                cancelDeveloperToolsRestoreRetry()
+            } else {
+                developerToolsRestoreRetryAttempt = 0
+                scheduleDeveloperToolsRestoreRetry()
+            }
         } else {
             cancelDeveloperToolsRestoreRetry()
             forceDeveloperToolsRefreshOnNextAttach = false

Sources/Panels/BrowserPanelView.swift

@@ -2773,6 +2773,15 @@ struct WebViewRepresentable: NSViewRepresentable {
                 coordinator.lastPortalHostId = hostId
             }
             BrowserWindowPortalRegistry.synchronizeForAnchor(host)
+        } else {
+            // Bind is deferred until host moves into a window. Keep the current
+            // portal entry's desired state in sync so stale callbacks cannot keep
+            // the previous anchor visible while this host is temporarily off-window.
+            BrowserWindowPortalRegistry.updateEntryVisibility(
+                for: webView,
+                visibleInUI: coordinator.desiredPortalVisibleInUI,
+                zPriority: coordinator.desiredPortalZPriority
+            )
         }
 
         panel.restoreDeveloperToolsAfterAttachIfNeeded()

tests/test_browser_devtools_portal_regressions.py

@@ -0,0 +1,92 @@
+#!/usr/bin/env python3
+"""Static regression checks for browser DevTools/portal review fixes.
+
+Guards two follow-up fixes:
+1) DevTools toggle path must retry restore when inspector show is transiently ignored.
+2) Browser portal visibility must propagate even if host is temporarily off-window.
+"""
+
+from __future__ import annotations
+
+import re
+import subprocess
+from pathlib import Path
+
+
+def repo_root() -> Path:
+    result = subprocess.run(
+        ["git", "rev-parse", "--show-toplevel"],
+        capture_output=True,
+        text=True,
+    )
+    if result.returncode == 0:
+        return Path(result.stdout.strip())
+    return Path(__file__).resolve().parents[1]
+
+
+def extract_block(source: str, signature: str) -> str:
+    start = source.find(signature)
+    if start < 0:
+        raise ValueError(f"Missing signature: {signature}")
+    brace_start = source.find("{", start)
+    if brace_start < 0:
+        raise ValueError(f"Missing opening brace for: {signature}")
+    depth = 0
+    for idx in range(brace_start, len(source)):
+        char = source[idx]
+        if char == "{":
+            depth += 1
+        elif char == "}":
+            depth -= 1
+            if depth == 0:
+                return source[brace_start : idx + 1]
+    raise ValueError(f"Unbalanced braces for: {signature}")
+
+
+def main() -> int:
+    root = repo_root()
+    failures: list[str] = []
+
+    panel_path = root / "Sources" / "Panels" / "BrowserPanel.swift"
+    panel_source = panel_path.read_text(encoding="utf-8")
+    toggle_block = extract_block(panel_source, "func toggleDeveloperTools() -> Bool")
+    if "visibleAfterToggle" not in toggle_block:
+        failures.append("toggleDeveloperTools() no longer re-checks inspector visibility")
+    if "scheduleDeveloperToolsRestoreRetry()" not in toggle_block:
+        failures.append("toggleDeveloperTools() no longer schedules a DevTools restore retry")
+
+    view_path = root / "Sources" / "Panels" / "BrowserPanelView.swift"
+    view_source = view_path.read_text(encoding="utf-8")
+    portal_update_block = extract_block(view_source, "private func updateUsingWindowPortal(")
+    if "BrowserWindowPortalRegistry.updateEntryVisibility(" not in portal_update_block:
+        failures.append("BrowserPanelView.updateUsingWindowPortal() is missing deferred portal visibility propagation")
+    if "zPriority: coordinator.desiredPortalZPriority" not in portal_update_block:
+        failures.append("BrowserPanelView deferred portal update no longer propagates zPriority")
+
+    portal_path = root / "Sources" / "BrowserWindowPortal.swift"
+    portal_source = portal_path.read_text(encoding="utf-8")
+    if not re.search(
+        r"func\s+updateEntryVisibility\s*\(\s*forWebViewId\s+webViewId:\s*ObjectIdentifier,\s*visibleInUI:\s*Bool,\s*zPriority:\s*Int\s*\)",
+        portal_source,
+        flags=re.MULTILINE,
+    ):
+        failures.append("WindowBrowserPortal is missing updateEntryVisibility(forWebViewId:visibleInUI:zPriority:)")
+    if not re.search(
+        r"static\s+func\s+updateEntryVisibility\s*\(\s*for\s+webView:\s*WKWebView,\s*visibleInUI:\s*Bool,\s*zPriority:\s*Int\s*\)",
+        portal_source,
+        flags=re.MULTILINE,
+    ):
+        failures.append("BrowserWindowPortalRegistry is missing updateEntryVisibility(for:visibleInUI:zPriority:)")
+
+    if failures:
+        print("FAIL: browser devtools/portal regression guards failed")
+        for item in failures:
+            print(f" - {item}")
+        return 1
+
+    print("PASS: browser devtools/portal regression guards are in place")
+    return 0
+
+
+if __name__ == "__main__":
+    raise SystemExit(main())