Skip to content

Fix Google sign-in infinite loading in browser pane#1493

Merged
austinywang merged 2 commits intomainfrom
issue-1491-google-signin-infinite-loading
Mar 16, 2026
Merged

Fix Google sign-in infinite loading in browser pane#1493
austinywang merged 2 commits intomainfrom
issue-1491-google-signin-infinite-loading

Conversation

@austinywang
Copy link
Copy Markdown
Contributor

@austinywang austinywang commented Mar 16, 2026

Summary

  • add a regression seam/test for nil-target browser popup fallback routing
  • stop rerouting scripted .other nil-target navigations back into new tabs
  • preserve the WKUIDelegate.createWebViewWith popup path so Google OAuth keeps window.opener semantics

Testing

  • not run locally per repo policy
  • ./scripts/reload.sh --tag fix-google-auth1491 reached ** BUILD SUCCEEDED **

Fixes #1491


Summary by cubic

Fixes Google sign-in infinite loading by preserving popup opener semantics and not rerouting scripted .other nil-target navigations to new tabs. Fixes #1491.

  • Bug Fixes
    • Handle scripted popups via WKUIDelegate.createWebViewWith so OAuth window.opener stays intact.
    • Fall back to a new tab only for target=_blank link activations; added regression tests for this behavior.

Written for commit 4ef164b. Summary will update on new commits.

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of navigation when no target is specified, with smarter fallback behavior that distinguishes between user-initiated link clicks and scripted actions.
  • Tests

    • Added test coverage for nil-target navigation scenarios.

@vercel
Copy link
Copy Markdown

vercel bot commented Mar 16, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment Mar 16, 2026 1:47am

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 16, 2026

Caution

Review failed

Pull request was closed or merged during review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bc5f59a8-5e29-4cbf-91ad-3c7852f05bf1

📥 Commits

Reviewing files that changed from the base of the PR and between bd17886 and 4ef164b.

📒 Files selected for processing (2)
  • Sources/Panels/BrowserPanel.swift
  • cmuxTests/CmuxWebViewKeyEquivalentTests.swift

📝 Walkthrough

Walkthrough

A helper function was added to centralize the decision logic for whether nil-target navigation should fallback to opening in a new tab. This function is now used in two navigation decision paths: the WKNavigationAction policy handler and the UI delegate's createWebViewWith flow, with updated comments reflecting the new handling.

Changes

Cohort / File(s) Summary
Browser Navigation Logic
Sources/Panels/BrowserPanel.swift
Added helper function browserNavigationShouldFallbackNilTargetToNewTab() to determine nil-target fallback behavior based on navigation type. Updated WKNavigationAction policy and createWebViewWith flows to use this centralized decision logic.
Navigation Tests
cmuxTests/CmuxWebViewKeyEquivalentTests.swift
Added BrowserNilTargetFallbackDecisionTests class with two test cases validating that non-link navigation (.other) does not fallback to new tab, while link-activated navigation does.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Poem

🐰 A nil-target leap, now smart and true,
Link hops fallback, but scripts pass through—
Two paths align with logic neat,
Navigation's dance, swift and fleet! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly summarizes the main objective: fixing Google sign-in infinite loading. It directly relates to the primary issue being addressed (#1491).
Description check ✅ Passed The PR description covers the summary (what changed and why) and testing approach, but lacks demo video and the review trigger checklist is incomplete (videos and local testing excluded per repo policy).
Linked Issues check ✅ Passed The PR directly addresses #1491 by implementing the fix to preserve popup semantics and prevent rerouting scripted .other navigations, aligning with the requirement to restore Google OAuth flow.
Out of Scope Changes check ✅ Passed All changes are focused on fixing the nil-target navigation fallback logic and adding regression tests, which are directly scoped to the issue requirements. No unrelated changes detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch issue-1491-google-signin-infinite-loading
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@austinywang austinywang merged commit 2e4c482 into main Mar 16, 2026
6 of 13 checks passed
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 2 files

bn-l pushed a commit to bn-l/cmux that referenced this pull request Apr 3, 2026
* Add regression test for Google sign-in popup fallback (manaflow-ai#1491)

* Fix Google sign-in popup routing regression (manaflow-ai#1491)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Google sign-in infinitely loading in browser pane (regression)

1 participant