Skip to content

fix(proxy): make configure automatically work in mobile safari - #115561

Closed
Silthus wants to merge 1 commit into
PostHog:masterfrom
Silthus:upstream/domain-connect-mobile-safari
Closed

Silthus wants to merge 1 commit into
PostHog:masterfrom
Silthus:upstream/domain-connect-mobile-safari

Conversation

@Silthus

@Silthus Silthus commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

Problem

  • Tapping "Configure automatically" on the managed reverse proxy or email domain setup screen does nothing in mobile Safari.
  • The button fetched the signed Domain Connect URL first, then called window.open after the await.
  • Safari no longer counts that as a user gesture, so its popup blocker drops the tab without notice (background). Desktop Chrome tolerates the delay, which hid the bug.

Supersedes #112664 (closed), rebased on current master. Fork handoff: Silthus/posthog#342.

Note

🔵 Merge risk: low · two-way door (merge-risk rubric)

  • Only the click handler of one button changes. A regression shows as a blank tab or a same-tab redirect to the DNS provider.
  • Jest covers the open, the error cleanup and the blocked-popup fallback.

Changes

  • The tap opens the provider tab at once, so Safari allows it. The tab loads the provider's approval page when the URL arrives.
  • The blank tab closes if the URL request fails, and the existing error toast shows.
  • If the browser refuses to open any tab, the current tab goes to the provider. Before, that case was a dead button too.
  • The new tab keeps no reference back to PostHog (opener is null).
  • Rejected alternative: same-tab navigation for everyone. It loses the email setup modal, which lives only in memory, on the way back from the provider.
  • No visual change.

How did you test this code?

Test rationale: no logic test existed for domainConnectLogic. The first case in the new domainConnectLogic.test.ts fails if window.open moves back behind the await, which is this bug. The other two cover the error cleanup and the no-tab fallback.

  • Ran the new Jest file locally on the rebased branch: 3 tests pass.
  • The original PR also drove the settings page in Playwright WebKit with iPhone emulation. Linux WebKit does not apply the iOS popup blocker, so that run could not reproduce the bug itself.
  • Not checked: a real iOS device or a real provider apply URL.

Release status

  • No feature flag controls this change

Docs update

None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: T3 Code with Claude Code, Claude Opus 5.5 (claude-opus-5-5). Cross-family review: Codex, GPT-6.1 Sol (gpt-6.1-sol), high reasoning effort, read-only.

  • Rebuilt from the closed PR's diff on master 60187c17de7. The patch applied without changes.
  • Skills: writing-pr-descriptions, writing-tests, merge-risk.
  • Review: no findings.
  • Duplicate search found no open PR for this fix.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@trunk-io

trunk-io Bot commented Oct 12, 2026

Copy link
Copy Markdown

Merging to master in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@scheduled-actions-posthog

Copy link
Copy Markdown
Contributor

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, please remove the stale label – otherwise this will be closed in another week. If you want to permanently keep it open, use the waiting label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant