Skip to content

fix(frontend): open customer portal in popup synchronously to bypass Safari blocker - #9242

Merged
rubenfiszel merged 2 commits into
mainfrom
fix-customer-portal-safari
May 19, 2026
Merged

fix(frontend): open customer portal in popup synchronously to bypass Safari blocker#9242
rubenfiszel merged 2 commits into
mainfrom
fix-customer-portal-safari

Conversation

@hugocasa

@hugocasa hugocasa commented May 19, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Safari blocks window.open() called after an await because the call is no longer tied to the user gesture, so clicking "Open customer portal" in instance settings silently did nothing on Safari.
  • Open a blank tab synchronously on click, then assign location.href once the portal URL resolves. If the synchronous open is blocked too, fall back to navigating the current tab.

Test plan

  • Click "Open customer portal" in instance settings on Safari — portal opens in a new tab
  • Same flow on Chrome/Firefox — still opens in a new tab as before
  • If the API call fails, the blank tab is closed and the error surfaces

🤖 Generated with Claude Code


Summary by cubic

Fixes Safari blocking the customer portal by creating the tab synchronously, then redirecting it once the URL is ready. Keeps the new-tab behavior on other browsers.

  • Bug Fixes

    • Open a blank tab immediately on click, then set its URL after the portal session resolves to preserve the user gesture on Safari.
    • If the popup is blocked, navigate the current tab; on API failure, close the blank tab and show the error.
  • Refactors

    • Wire dev_override feature flag in the backend via windmill-common/dev_override.

Written for commit fc77e63. Summary will update on new commits. Review in cubic

hugocasa and others added 2 commits May 19, 2026 16:59
…Safari blocker

Safari blocks window.open() called after an await because it loses the
user-gesture context. Open a blank tab synchronously on click, then
assign location.href once the portal URL resolves.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying windmill with  Cloudflare Pages  Cloudflare Pages

Latest commit: fc77e63
Status: ✅  Deploy successful!
Preview URL: https://80ddc2ce.windmill.pages.dev
Branch Preview URL: https://fix-customer-portal-safari.windmill.pages.dev

View logs

@hugocasa
hugocasa marked this pull request as ready for review May 19, 2026 15:17
@claude

claude Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor

Claude finished @hugocasa's task in 2m 15s —— View job


Good to merge

Review — PR #9242

Checked the diff against the PR base, the openCustomerPortal call site, and the dev_override feature wiring.

Frontend — InstanceSetting.svelte

The fix is the correct, idiomatic workaround for Safari's popup blocker:

  • window.open('', '_blank') is called synchronously inside the click handler (on:click={openCustomerPortal} at line 567), so it stays tied to the user gesture. ✅
  • newWindow.location.href = url is assigned after the await, which Safari permits because the tab was already opened during the gesture. ✅
  • Popup-blocked fallback (newWindow is nullwindow.location.href = url) is sensible. ✅
  • The new catch closes the orphaned blank tab on API failure and rethrows, so error surfacing is unchanged versus the previous (catch-less) code, which also propagated the rejection. This matches the sibling renewLicenseKey (line 108) pattern in the same file. ✅

No introduced bugs found. TypeScript-safe (window.openWindow | null is correctly guarded with if (newWindow) / newWindow?.close()).

Optional, non-blocking (pre-existing, not introduced here): neither the old window.open(url, '_blank') nor the new code passes noopener, so the opened tab keeps a window.opener back-reference. Risk is low since url is a backend-issued Stripe billing portal URL; the synchronous-blank-tab pattern is incompatible with noopener anyway (it would force window.open to return null). Mentioning only for awareness — no change needed.

Backend — Cargo.toml

dev_override = ["windmill-common/dev_override"] correctly references an existing feature (backend/windmill-common/Cargo.toml:24), which gates the WINDMILL_CUSTOMER_SERVICE_BASE_URL env override in windmill-common/src/ee.rs:445. This is a dev-only convenience that lets the customer-portal flow be pointed at a local service — directly relevant to testing this Safari fix. Bundling it with the frontend fix is slightly outside the PR's stated scope but justified and harmless (no-op unless the feature is explicitly enabled).

AGENTS.md compliance

Compliant. No raw HTML elements added, follows the established sibling pattern, change kept focused.

Test coverage

  • Frontend: This is a DOM/window interaction inside a Svelte component, not a pure-logic utility — per REVIEW.md, the codebase does not test Svelte components, so no automated tests are expected here.
  • Backend: Cargo.toml feature wiring only — no automated tests expected.

Manual verification still worth doing before merge: click "Open customer portal" in instance settings on Safari and confirm a new tab opens to the portal; repeat on Chrome/Firefox to confirm unchanged new-tab behavior; force an API failure and confirm the blank tab is closed and the error toast surfaces; and exercise the popup-blocked path (browser set to block popups) to confirm the current tab navigates to the portal as the fallback.
· Branch preview

@github-actions

Copy link
Copy Markdown
Contributor

Codex Review

Good to merge

No issues found. Checked for bugs, security issues, and AGENTS.md compliance.

Test coverage

Frontend: no automated component tests expected for this Svelte interaction change. The PR’s manual test plan is appropriate.

Backend: only Cargo feature forwarding changed; no Rust logic or API surface changed, so no automated backend tests are expected.

Manual verification still needed: in instance settings, click “Open customer portal” on Safari and confirm the portal opens in a new tab. Repeat on Chrome/Firefox, and verify an API failure closes the blank tab and surfaces the error.

@github-actions

Copy link
Copy Markdown
Contributor

Pi Review

Good to merge

Review — PR #9242

I verified the diff, the surrounding context in InstanceSetting.svelte, the dev_override feature in windmill-common, and the previous Claude review.

Frontend — InstanceSetting.svelte

The fix correctly addresses Safari's popup-blocker behavior:

  • window.open('', '_blank') is invoked synchronously at line 126, before the await at line 129, keeping the call tied to the user gesture. ✅
  • newWindow.location.href = url redirects the already-opened tab after the portal URL resolves — Safari permits this because the tab was opened during the gesture. ✅
  • The null guard on newWindow (line 131) covers the case where the popup is blocked, falling back to navigating the current tab via window.location.href. ✅
  • The catch block at line 138 closes the orphaned blank tab (newWindow?.close()) on API failure and rethrows — error surfacing remains identical to the previous (catch-less) code. ✅
  • newWindow returns Window | null from window.open; both if (newWindow) and newWindow?.close() correctly narrow/handle the type. ✅

No bugs, no security concerns.

Backend — Cargo.toml

dev_override = ["windmill-common/dev_override"] wires an existing feature (windmill-common/Cargo.toml:24) that gates an env-var override (WINDMILL_CUSTOMER_SERVICE_BASE_URL) for the customer portal URL in windmill-common/src/ee.rs:445. This is a dev-only convenience (opt-in, no production impact) relevant for testing the portal flow locally — directly useful for verifying this Safari fix. Harmless and well-scoped.

AGENTS.md compliance

Compliant. No banned patterns, change is focused, no raw HTML, follows established code patterns in the same file.

Test coverage

  • Frontend: This is a DOM/window interaction within a Svelte component — the codebase does not test Svelte components at this layer, so no automated tests are expected.
  • Backend: Feature-flag wiring only — no automated tests expected.

Manual verification still worth doing before merge: click "Open customer portal" in instance settings on Safari and confirm a new tab opens to the portal; repeat on Chrome/Firefox to confirm unchanged new-tab behavior; force an API failure and confirm the blank tab is closed and the error toast surfaces; exercise the popup-blocked path (browser set to block popups) to confirm the current tab navigates to the portal as the fallback.

@rubenfiszel
rubenfiszel merged commit f51b51a into main May 19, 2026
29 checks passed
@rubenfiszel
rubenfiszel deleted the fix-customer-portal-safari branch May 19, 2026 15:26
@github-actions github-actions Bot locked and limited conversation to collaborators May 19, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants