Repository navigation
Fix browser issues - #1029
Fix browser issues#1029
Conversation
|
To use Codex here, create a Codex account and connect to github. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR adds targeted recovery logic for off-window anchor reparenting in BrowserWindowPortal.swift. When a visible portal entry's anchor view moves off-window (superview exists but window is nil), the code schedules a transient recovery operation (anchorWindowMismatch) instead of immediate detachment, defers stabilization via retry, and clears the drop-zone overlay. A corresponding test verifies portal entries remain visible and bound during this scenario. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR adds a targeted guard in Key changes:
Issues found:
Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[synchronizeWebView called] --> B{anchorView.window\n=== window?}
B -- yes --> C[Normal layout & frame sync]
B -- no --> D{isOffWindowReparent?\nvisibleInUI &&\nanchorView.window == nil &&\nanchorView.superview != nil}
D -- yes --> E[scheduleTransientRecoveryRetryIfNeeded\nreason: anchorWindowMismatch]
E --> F{didScheduleTransientRecovery?}
F -- true --> G[setDropZoneOverlay nil\nreturn — portal stays VISIBLE]
F -- false\nbudget exhausted --> H[⚠️ setDropZoneOverlay nil\nreturn — portal STILL VISIBLE\nno further recovery scheduled]
D -- no --> I[scheduleTransientDetachRecovery\nreason: anchorWindowMismatch]
I --> J{retry scheduled?}
J -- yes --> K[hide container\nreturn]
J -- no\nbudget exhausted --> L[hide container\nreturn]
Last reviewed commit: 72c0f56 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if isOffWindowReparent { | ||
| let didScheduleTransientRecovery = scheduleTransientRecoveryRetryIfNeeded( | ||
| forWebViewId: webViewId, | ||
| entry: &entry, | ||
| webView: webView, | ||
| reason: "anchorWindowMismatch" | ||
| ) | ||
| #if DEBUG | ||
| if didScheduleTransientRecovery && !containerView.isHidden { | ||
| dlog( | ||
| "browser.portal.hidden.deferKeep web=\(browserPortalDebugToken(webView)) " + | ||
| "reason=anchorWindowMismatch.offWindow frame=\(browserPortalDebugFrame(containerView.frame))" | ||
| ) | ||
| } | ||
| #endif | ||
| containerView.setDropZoneOverlay(zone: nil) | ||
| return |
There was a problem hiding this comment.
Zombie portal when retry budget is exhausted
When scheduleTransientRecoveryRetryIfNeeded returns false (budget exhausted — i.e. the anchor has been off-window for more than transientRecoveryRetryBudget sync cycles), the code still returns early without hiding the container. No further deferred syncs are scheduled at that point, so the portal remains unconditionally visible with no remaining recovery path.
Compare this to the existing code immediately below: when the non-off-window anchor-window-mismatch path exhausts its budget, scheduleTransientDetachRecovery returns false and the code falls through to unconditionally hide and clean up the container.
In the off-window path the only escape from this state is an external call to synchronizeWebViewForAnchor (e.g. when the anchor is re-added to the window hierarchy). If that call never arrives — or arrives only after a long delay — the portal will remain visible indefinitely in a zombie state, showing stale content with no frame updates and only the drop-zone overlay cleared.
Consider hiding the container (and clearing the remaining overlays) when the budget is exhausted, analogous to the fallthrough behaviour in the non-off-window path:
if isOffWindowReparent {
let didScheduleTransientRecovery = scheduleTransientRecoveryRetryIfNeeded(
forWebViewId: webViewId,
entry: &entry,
webView: webView,
reason: "anchorWindowMismatch"
)
if didScheduleTransientRecovery {
#if DEBUG
if !containerView.isHidden {
dlog(
"browser.portal.hidden.deferKeep web=\(browserPortalDebugToken(webView)) " +
"reason=anchorWindowMismatch.offWindow frame=\(browserPortalDebugFrame(containerView.frame))"
)
}
#endif
containerView.setDropZoneOverlay(zone: nil)
return
}
// Budget exhausted — fall through to hide/cleanup below
}| #if DEBUG | ||
| if didScheduleTransientRecovery && !containerView.isHidden { | ||
| dlog( | ||
| "browser.portal.hidden.deferKeep web=\(browserPortalDebugToken(webView)) " + | ||
| "reason=anchorWindowMismatch.offWindow frame=\(browserPortalDebugFrame(containerView.frame))" | ||
| ) | ||
| } | ||
| #endif |
There was a problem hiding this comment.
Misleading debug log name
The log key browser.portal.hidden.deferKeep is emitted inside the guard !containerView.isHidden, meaning the container is currently visible and is being kept visible — it is not being hidden. The name hidden in the key will confuse log-analysis tooling or anyone grepping for hide events.
A name like browser.portal.visible.deferKeep or browser.portal.sync.deferKeep would better reflect what is actually happening at this point.
| #if DEBUG | |
| if didScheduleTransientRecovery && !containerView.isHidden { | |
| dlog( | |
| "browser.portal.hidden.deferKeep web=\(browserPortalDebugToken(webView)) " + | |
| "reason=anchorWindowMismatch.offWindow frame=\(browserPortalDebugFrame(containerView.frame))" | |
| ) | |
| } | |
| #endif | |
| dlog( | |
| "browser.portal.visible.deferKeep web=\(browserPortalDebugToken(webView)) " + | |
| "reason=anchorWindowMismatch.offWindow frame=\(browserPortalDebugFrame(containerView.frame))" | |
| ) |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 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/BrowserWindowPortal.swift">
<violation number="1" location="Sources/BrowserWindowPortal.swift:2303">
P2: When the transient recovery budget is exhausted (`scheduleTransientRecoveryRetryIfNeeded` returns `false`), this block still returns early, leaving the container visible at a stale frame with no recovery scheduled. Gate the early return on the scheduling result so it falls through to the existing hide logic when retries are exhausted.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| containerView.setDropZoneOverlay(zone: nil) | ||
| return |
There was a problem hiding this comment.
P2: When the transient recovery budget is exhausted (scheduleTransientRecoveryRetryIfNeeded returns false), this block still returns early, leaving the container visible at a stale frame with no recovery scheduled. Gate the early return on the scheduling result so it falls through to the existing hide logic when retries are exhausted.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/BrowserWindowPortal.swift, line 2303:
<comment>When the transient recovery budget is exhausted (`scheduleTransientRecoveryRetryIfNeeded` returns `false`), this block still returns early, leaving the container visible at a stale frame with no recovery scheduled. Gate the early return on the scheduling result so it falls through to the existing hide logic when retries are exhausted.</comment>
<file context>
@@ -2281,6 +2281,28 @@ final class WindowBrowserPortal: NSObject {
+ )
+ }
+#endif
+ containerView.setDropZoneOverlay(zone: nil)
+ return
+ }
</file context>
| containerView.setDropZoneOverlay(zone: nil) | |
| return | |
| containerView.setDropZoneOverlay(zone: nil) | |
| if didScheduleTransientRecovery { | |
| return | |
| } |
…nal-drag-glitch Fix browser issues
This branch was successfully deployed
1 participant
Summary
Testing
Demo Video
For UI or behavior changes, include a short demo video (GitHub upload, Loom, or other direct link).
Review Trigger (Copy/Paste as PR comment)
Checklist
Summary by cubic
Fixes a flicker where the browser portal detaches or hides when a visible webview’s anchor is temporarily reparented off-window during drag. Addresses Linear issue 1024 (terminal drag glitch) by keeping the slot visible until the anchor returns.
Written for commit 72c0f56. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests