fix(desktop): restore visible Google sign-in popups in previews - #13348
MatthewFeroz wants to merge 2 commits into
Conversation
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — The PR changes the default webview configuration for all desktop previews and changes Google OAuth popup ownership and FedCM behavior on an existing authentication path. These are focused changes with tests, but the product-default and authentication scope warrant human review. You can add or adjust custom eligibility rules. Learn more. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: pingdotgg/t3code/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughPreview guests now disable FedCm. Allowed sign-in popups use the live main window as their parent when one is available and not destroyed. ChangesPreview sign-in flow
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The preview sign-in changes are mergeable after normal checks. No supported issue remains that would block popup fallback or ownership. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
What Changed
Fix Google sign-in in desktop preview tabs: disable unsupported FedCM so Google uses its popup fallback, and make those popups non-modal children of the T3 window so clicking back on T3 cannot hide the account chooser behind it.
Why
Clicking Google sign-in on YNAB in the Windows Browser panel left the login page unchanged. The console reported
Prompt dismissedand[GSI_LOGGER]: FedCM get() rejects with NetworkError: Error retrieving a token.Electron advertises the FedCM API without providing the browser-owned identity dialog, causing Google to choose a flow that cannot show its prompt.This scopes the workaround to preview tabs. It preserves their sandbox and disabled Node integration, and leaves T3's own sign-in configuration unchanged.
Testing exposed a second issue: an already-open Google chooser could sit behind T3, making sign-in appear unresponsive until refresh opened another window. The reporter confirmed the hidden windows existed. Give popups a live host parent at creation so native window ordering keeps them above T3, without global always-on-top behavior or a modal dialog. Preserve context isolation, the opener, and the guest session; skip a destroyed parent.
Validation
vp test run src/preview/Manager.test.ts src/preview/WebviewPreferences.test.tsinapps/desktop: 100 passed.vp run typecheckinapps/desktop: passed (existing Effect suggestions).git diff --check: passed.The change is desktop preview configuration shared by hosted browser tabs; no provider or wire-contract changes. macOS, Linux, mobile, and remote-hosted preview flows were not manually exercised.
UI Changes
Before: Google sign-in immediately dismisses its FedCM prompt and stays on YNAB's login page.
After: Google sign-in opens its account chooser in the development app, and the chooser stays above T3 when the reporter clicks back on T3.
User-recorded Windows evidence, cropped with account details blurred and audio removed. The newest after recording includes the popup ownership follow-up.
Before video · After video
Checklist
Model: GPT-6 Astra. Harness: Codex in T3 Code.