fix(frontend): plan page dark-mode loader and checkout popup - #2900
fix(frontend): plan page dark-mode loader and checkout popup#2900riderx wants to merge 2 commits into
Conversation
Inherit spinner color for dark-mode plan buttons and open Stripe checkout via a confirm link so browsers do not block the tab after async session creation. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughWeb Stripe checkout now uses a confirmation flow across web platforms. Blocked popups show a manual checkout link. Dialog handling and E2E coverage were updated for the new-tab flow. ChangesStripe checkout flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Plans
participant openBlank
participant Browser
participant DialogV2
Plans->>openBlank: Open Stripe checkout URL
openBlank->>Browser: Call window.open
Browser-->>openBlank: Return popup result
openBlank->>DialogV2: Show manual link when blocked
DialogV2->>Browser: Open checkout link in a new tab
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_7fb8b852-c486-4fbd-9945-848987c09f8d) |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Merging this PR will not alter performance
Comparing 
Footnotes
|
Propagate iOS action-sheet cancel from openBlank and align the subscription checkout E2E with the confirm-link web flow. Co-authored-by: Cursor <cursoragent@cursor.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_aae79b41-24a0-411a-8712-24009d1719b1) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/services/stripe.ts`:
- Around line 62-64: Update the popup-opening flow around the opened result from
globalThis.open to set opened.opener to null before returning true; preserve the
existing null-return fallback unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0fe8e1e1-4d43-4f9d-9e1b-c246893fda06
📒 Files selected for processing (5)
messages/en.jsonplaywright/e2e/subscription-checkout.spec.tssrc/components/DialogV2.vuesrc/pages/settings/organization/Plans.vuesrc/services/stripe.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Cap-go/capacitor-updater(manual)
| const opened = globalThis.open(link, '_blank') | ||
| if (opened) | ||
| return true |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- src/services/stripe.ts ---'
cat -n src/services/stripe.ts
printf '%s\n' '--- window.open usages ---'
rg -n -C 3 'globalThis\.open|window\.open|noopener|opener' src tests .github 2>/dev/null || true
printf '%s\n' '--- relevant package/runtime metadata ---'
rg -n -C 2 '"(lint|test|typecheck|build)"|vue|vite|vitest|playwright' package.json bun.lockb package-lock.json yarn.lock pnpm-lock.yaml 2>/dev/null || trueRepository: Cap-go/capgo.app
Length of output: 36115
🌐 Web query:
MDN Window open target _blank implicit noopener window.opener browser compatibility
💡 Result:
In modern web browsers, setting target="_blank" on , , or
elements implicitly behaves as if rel="noopener" has been specified [1]. This means the opened browsing context will not have access to the opening window via the window.opener property, which will return null [2][1]. This implicit behavior was standardized to improve security by preventing the opened page from tampering with the original page via the window.opener reference [1][3]. If you explicitly require window.opener access, you must now use rel="opener" on the link [1][3]. Browser Compatibility: This implicit behavior is widely supported in modern browsers [4]. Support for target="_blank" implying rel="noopener" generally aligns with the adoption of this standard in major browser engines (Chromium 88+, Firefox 79+, and Safari 12.1+) [4]. Older browsers do not provide this implicit security, which is why developers previously had to manually add rel="noopener" to secure links [3][5]. Note: This implicit behavior applies to HTML anchor/form elements [1]. When using the window.open JavaScript method, the noopener behavior is not automatic and must still be explicitly requested by including "noopener" in the windowFeatures parameter [6][7].Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Attributes/rel/noopener
- 2: https://developer.mozilla.org/en-US/docs/Web/API/Window/opener
- 3: https://mathiasbynens.github.io/rel-noopener/
- 4: https://caniuse.com/mdn-html_elements_a_implicit_noopener
- 5: https://caniuse.com/rel-noopener
- 6: https://developer.mozilla.org/en-US/docs/Web/API/Window/open.
- 7: https://developer.mozilla.org/en-US/docs/Web/API/Window/open
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- browser-target and compatibility configuration ---'
rg -n -i -C 2 'browserslist|browser|target|support|compatib' package.json vite.config.* .browserslistrc tsconfig*.json README.md docs 2>/dev/null || true
printf '%s\n' '--- tests and history for openBlank ---'
rg -n -C 5 'openBlank|presentBlockedPopupFallback|window\.opener' tests src scripts 2>/dev/null || true
git diff --stat
git log --oneline -8 -- src/services/stripe.tsRepository: Cap-go/capgo.app
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- browser support configuration ---'
for f in package.json vite.config.ts vite.config.js vite.config.mts .browserslistrc; do
if [ -f "$f" ]; then
printf '\n[%s]\n' "$f"
rg -n -i 'browserslist|target|browser|support|compatib' "$f" || true
fi
done
printf '%s\n' '--- focused openBlank history ---'
git log --oneline -12 -- src/services/stripe.ts
printf '%s\n' '--- focused tests/usages ---'
rg -n -C 4 'openBlank|presentActionSheetOpen|window\.opener' tests src/services src/pages/app/'[app]'.channel.'[channel]'.vue 2>/dev/null || true
printf '%s\n' '--- current diff summary ---'
git diff --statRepository: Cap-go/capgo.app
Length of output: 3274
🏁 Script executed (no clone):
node - <<'JS'
'use strict'
// Read-only behavioral model of the proposed operation.
// It checks that the popup-blocked branch still depends on the return value
// and that a successfully returned WindowProxy gets opener cleared.
function openBlankModel(open) {
const opened = open('https://checkout.example', '_blank')
if (opened) {
opened.opener = null
return true
}
return 'fallback'
}
const popup = { opener: { location: 'https://app.example' } }
console.log('opened:', openBlankModel(() => popup))
console.log('opener:', popup.opener)
console.log('blocked:', openBlankModel(() => null))
JS
Length of output: 169
🌐 Web query:
MDN Window.open noopener feature return value WindowProxy opener null
💡 Result:
When the noopener feature is used in the window.open() method, the method returns null instead of a WindowProxy object [1][2]. Additionally, the newly opened window will have its window.opener property set to null [3][4]. This behavior ensures that the new browsing context has no access to the originating window, which is a key security feature to prevent unauthorized access or tampering [4][2]. This behavior is consistent with the HTML specification, which dictates that window.open() returns null when the noopener option is provided, effectively preventing the caller from obtaining a reference to the opened window [5][6][7].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/API/Window/open
- 2: https://web.dev/articles/detached-window-memory-leaks
- 3: https://developer.mozilla.org/en-US/docs/Web/API/Window/opener
- 4: https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Attributes/rel/noopener
- 5: https://html.spec.whatwg.org/dev/nav-history-apis.html
- 6: https://stackoverflow.com/questions/77812102/cross-origin-opener-policy-and-window-open-returning-null-in-a-cross-origin-ifra
- 7: Window opening steps return WindowProxy despite COOP whatwg/html#5759
Clear opened.opener before returning success. Set opened.opener = null when the popup opens. Keep the null-return fallback intact.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/services/stripe.ts` around lines 62 - 64, Update the popup-opening flow
around the opened result from globalThis.open to set opened.opener to null
before returning true; preserve the existing null-return fallback unchanged.
|
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Confidence score: 4/5
- In
playwright/e2e/subscription-checkout.spec.ts, the test usespage.gotoon the Confirm link href instead of exercising the realtarget="_blank"click flow, so the new popup-opening path (and popup-blocker fallback) can regress without being caught in CI; update the spec to click the link and assert popup/new-page behavior directly.
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="playwright/e2e/subscription-checkout.spec.ts">
<violation number="1" location="playwright/e2e/subscription-checkout.spec.ts:205">
P2: Reading the Confirm link's href and navigating with page.goto bypasses the actual target=_blank popup this PR is meant to harden, so the new `_blank` open path and popup-blocker fallback are never e2e-covered. Consider clicking the link and asserting the resulting popup (e.g. `const [popup] = await Promise.all([page.waitForEvent('popup'), confirmLink.click()])`, then assert `popup.url()` matches the Stripe origin) so a regression in DialogV2's native-blank handling is caught.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| expect(checkoutUrl!).toMatch(new RegExp(`${escapedStripeOrigin}/checkout/`)) | ||
|
|
||
| await page.goto(checkoutUrl) | ||
| await page.goto(checkoutUrl!) |
There was a problem hiding this comment.
P2: Reading the Confirm link's href and navigating with page.goto bypasses the actual target=_blank popup this PR is meant to harden, so the new _blank open path and popup-blocker fallback are never e2e-covered. Consider clicking the link and asserting the resulting popup (e.g. const [popup] = await Promise.all([page.waitForEvent('popup'), confirmLink.click()]), then assert popup.url() matches the Stripe origin) so a regression in DialogV2's native-blank handling is caught.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At playwright/e2e/subscription-checkout.spec.ts, line 205:
<comment>Reading the Confirm link's href and navigating with page.goto bypasses the actual target=_blank popup this PR is meant to harden, so the new `_blank` open path and popup-blocker fallback are never e2e-covered. Consider clicking the link and asserting the resulting popup (e.g. `const [popup] = await Promise.all([page.waitForEvent('popup'), confirmLink.click()])`, then assert `popup.url()` matches the Stripe origin) so a regression in DialogV2's native-blank handling is caught.</comment>
<file context>
@@ -206,14 +193,16 @@ test.describe('Subscription Checkout', () => {
+ expect(checkoutUrl!).toMatch(new RegExp(`${escapedStripeOrigin}/checkout/`))
- await page.goto(checkoutUrl)
+ await page.goto(checkoutUrl!)
await expect(page).toHaveURL(new RegExp(`${escapedStripeOrigin}/checkout/`))
await page.getByRole('button', { name: /^Pay / }).click()
</file context>
1 participant
Summary (AI generated)
_blanklink after Stripe session creation, avoiding popup blockerstarget="_blank"anchors open natively instead ofpreventDefault+window.openopenBlankfalls back to a confirm link when the browser blocks the popupMotivation (AI generated)
Dark mode made the subscribe loading spinner invisible (
text-whiteondark:bg-white). Checkout also opened withwindow.openafter an async API call, which browsers often block.Business Impact (AI generated)
Users can see checkout loading in dark mode and complete plan upgrades without a blocked Stripe tab.
Test Plan (AI generated)
/settings/organization/plansin dark mode and click a plan CTA — spinner is visible on black and white buttonswindow.open, confirm the fallback dialog can open the URLGenerated with AI
Made with Cursor
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit
New Features
Bug Fixes