fix(invoices): stop popup blockers from silently eating the PDF preview tab (#1191)

* fix(invoices): stop popup blockers from silently eating the PDF preview tab

A window.open() after an await runs outside the click's transient user
activation (~5s, less in Safari), so the preview tab was popup-blocked
exactly when generation was slow (cold start + logo re-encode). The
request succeeded, nothing opened, no error: the button looked locked
(support: carina@cbysea.se).

- lib/browser/deferred-tab.ts: open the tab synchronously in the click,
  navigate it when the result arrives, close it on failure (the pattern
  AGIPanel already used for its signing tab), with unit tests.
- InvoiceEditor: preview uses the deferred tab + popup-blocked toast;
  revoke the blob URL instead of leaking it; guard the review dialog
  against an unresolved customer (silent no-op click); 5s timeout on the
  pre-review next-number fetch; spinner + disable while the submit
  handler is in flight in create mode.
- Same pre-open fix in TransactionAttachmentIndicator,
  JournalEntryAttachments (failures now toast instead of vanishing),
  DocumentViewButton, and the Arcim reconnect OAuth popup (its 'trusted
  gesture' comment was wrong after the await).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(invoices): review triage: close blocked preview tab, precise popup hint, test convention

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
2026-07-25 13:43:04 +02:00
committed by GitHub
co-authored by Claude Fable 5
parent b5e3c476a5
commit f07a34c51b
10 changed files with 297 additions and 25 deletions
+1
View File
@@ -375,3 +375,4 @@ One line per decision: `[YYYY-MM-DD] <decision>: <why>`. Appended by agents and
[2026-07-25] Currency-honesty batch (#1177-#1187): aggregates over mixed-currency rows group per currency instead of converting (supplier stats, bank-file totals): read paths must not depend on live FX fetches, and a per-currency line is honest where a converted single number would hide the mix. Where a stored SEK conversion exists but is NULL (invoices.total_sek after a failed rate fetch), aggregates skip-and-flag (unconvertedCount + one visible note) rather than fall back to the raw foreign amount.
[2026-07-25] Editing a draft ROT/RUT invoice keeps the stored encrypted personnummer when the field is left empty and deduction lines remain (#1186): the plaintext is not client-rehydratable by design, so empty-means-keep is the only edit semantics that neither blocks the edit nor wipes the ciphertext; typed value replaces, removing all deduction lines clears.
[2026-07-25] Article delete was broken globally by a phantom invoice_items.company_id filter (42703 -> ARTICLE_DELETE_FAILED) that mocked route tests cannot catch; fixed in #1188 with a source-pin test. Lesson: supabase-mock tests validate flow, never schema: any new filtered column needs a schema-level check or pg-real coverage.
[2026-07-25] Popup-after-await fix uses a pre-opened tab (AGIPanel pattern) via lib/browser/deferred-tab, not an anchor-download fallback: pre-opening about:blank keeps the user gesture and works for blob and signed URLs alike; the helper severs window.opener, except the Arcim OAuth popup which keeps it for postMessage.
+18 -1
View File
@@ -4,6 +4,7 @@ import { useState } from 'react'
import { ExternalLink, Loader2 } from 'lucide-react'
import { Button } from '@/components/ui/button'
import { useToast } from '@/components/ui/use-toast'
import { openDeferredTab } from '@/lib/browser/deferred-tab'
interface DocumentViewButtonProps {
documentId: string
@@ -35,10 +36,14 @@ export function DocumentViewButton({ documentId, label = 'Visa dokument', classN
return
}
setLoading(true)
// Pre-open inside the click's user activation: a window.open after the
// await is popup-blocked when the signed-URL fetch is slow.
const tab = openDeferredTab('Laddar...')
try {
const res = await fetch(`/api/documents/${documentId}`)
const json = await res.json().catch(() => ({}))
if (!res.ok || !json?.data?.download_url) {
tab.close()
toast({
title: 'Kunde inte öppna dokumentet',
description: json?.error || 'Försök igen om en stund.',
@@ -46,7 +51,19 @@ export function DocumentViewButton({ documentId, label = 'Visa dokument', classN
})
return
}
window.open(json.data.download_url as string, '_blank', 'noopener,noreferrer')
if (!tab.navigate(json.data.download_url as string)) {
tab.close()
toast({
title: 'Kunde inte öppna dokumentet',
description: tab.blocked
? 'Tillåt popupfönster för Accounted i webbläsaren och försök igen.'
: 'Försök igen om en stund.',
variant: 'destructive',
})
}
} catch {
tab.close()
toast({ title: 'Kunde inte öppna dokumentet', variant: 'destructive' })
} finally {
setLoading(false)
}
@@ -13,6 +13,7 @@ import {
DialogFooter,
} from '@/components/ui/dialog'
import { useToast } from '@/components/ui/use-toast'
import { openDeferredTab } from '@/lib/browser/deferred-tab'
import {
FileText,
ImageIcon,
@@ -70,6 +71,7 @@ export default function JournalEntryAttachments({
onCountChange,
}: JournalEntryAttachmentsProps) {
const t = useTranslations('journal_attachments')
const tCommon = useTranslations('common')
const { toast } = useToast()
const [documents, setDocuments] = useState<DocumentRecord[]>([])
const [loading, setLoading] = useState(true)
@@ -154,14 +156,23 @@ export default function JournalEntryAttachments({
}, [uploadFiles, fetchDocuments])
const handleDownload = async (docId: string) => {
// Pre-open inside the click's user activation: a window.open after the
// await is popup-blocked when the signed-URL fetch is slow.
const tab = openDeferredTab(tCommon('loading'))
try {
const res = await fetch(`/api/documents/${docId}`)
const { data } = await res.json()
if (data?.download_url) {
window.open(data.download_url, '_blank')
if (!data?.download_url || !tab.navigate(data.download_url)) {
tab.close()
toast({
title: t('download_failed'),
description: tab.blocked ? tCommon('popup_blocked_description') : undefined,
variant: 'destructive',
})
}
} catch {
// Non-critical: silently ignore
tab.close()
toast({ title: t('download_failed'), variant: 'destructive' })
}
}
@@ -1897,6 +1897,17 @@ export default function ArcimMigrationWorkspace(_props: WorkspaceComponentProps)
setIsLoading(true)
setSelectedProvider(provider)
// Pre-open the OAuth popup inside the click's user activation: opening it
// after the fetch below is popup-blocked when the response is slow (the
// activation expires after ~5s). Kept open only for OAuth providers; the
// token path and every failure path close it again. The opener reference
// stays intact: the provider popup posts back via postMessage.
const w = 600
const h = 700
const left = window.screenX + (window.outerWidth - w) / 2
const top = window.screenY + (window.outerHeight - h) / 2
const popup = window.open('', 'arcim-oauth', `width=${w},height=${h},left=${left},top=${top}`)
try {
const res = await fetch('/api/extensions/ext/arcim-migration/connect', {
method: 'POST',
@@ -1913,19 +1924,24 @@ export default function ArcimMigrationWorkspace(_props: WorkspaceComponentProps)
setAuthType(data.authType)
if (data.authType === 'oauth' && data.authUrl) {
// Open immediately: this runs inside the button's click handler, so
// the popup is a trusted user gesture and won't be blocked.
const w = 600
const h = 700
const left = window.screenX + (window.outerWidth - w) / 2
const top = window.screenY + (window.outerHeight - h) / 2
window.open(data.authUrl, 'arcim-oauth', `width=${w},height=${h},left=${left},top=${top}`)
if (popup && !popup.closed) {
popup.location.href = data.authUrl
} else {
// The pre-opened popup was blocked or closed; retrying here is a
// long shot (the activation may be gone) but strictly better than
// dropping the flow.
window.open(data.authUrl, 'arcim-oauth', `width=${w},height=${h},left=${left},top=${top}`)
}
setAuthUrl(data.authUrl)
} else if (data.authType === 'token') {
// Re-enter credentials for token-based providers
setStep('connect')
} else {
popup?.close()
if (data.authType === 'token') {
// Re-enter credentials for token-based providers
setStep('connect')
}
}
} catch (err) {
popup?.close()
setError(err instanceof Error ? getUserErrorMessage(err) : 'Kunde inte återansluta')
setAuthExpired(true)
} finally {
+47 -9
View File
@@ -41,6 +41,7 @@ import { useCanWrite } from '@/lib/hooks/use-can-write'
import { ConfirmationDialog } from '@/components/ui/confirmation-dialog'
import { InvoiceReviewContent } from '@/components/invoices/InvoiceReviewContent'
import { getErrorMessage } from '@/lib/errors/get-error-message'
import { openDeferredTab } from '@/lib/browser/deferred-tab'
import { useUnsavedChanges } from '@/lib/hooks/use-unsaved-changes'
import CustomerForm from '@/components/customers/CustomerForm'
import { BankDetailsSetupDialog } from '@/components/invoices/BankDetailsSetupDialog'
@@ -129,6 +130,7 @@ export default function InvoiceEditor(props: InvoiceEditorProps = { mode: 'creat
const t = useTranslations('invoice_editor')
const ts = useTranslations('self_billing')
const ta = useTranslations('accruals')
const tCommon = useTranslations('common')
// Toggle between a normal customer invoice (default) and registering a
// self-billing invoice we received (mottagen självfaktura, ML 17 kap 15§).
// Self-billing is never available when editing an existing draft.
@@ -344,7 +346,7 @@ export default function InvoiceEditor(props: InvoiceEditorProps = { mode: 'creat
setValue,
setError,
getValues,
formState: { errors, isDirty, dirtyFields },
formState: { errors, isDirty, dirtyFields, isSubmitting: isFormSubmitting },
} = useForm<FormData>({
resolver: zodResolver(schema),
// Edit mode pre-fills from the existing draft (header + every line incl.
@@ -1020,12 +1022,29 @@ export default function InvoiceEditor(props: InvoiceEditorProps = { mode: 'creat
await handleSelfBilledSubmit(data)
return
}
// The review dialog only mounts once the picked customer resolves against
// the loaded customers list. Without this guard a click while the list is
// still loading (or failed to load) set showReview on an unmounted dialog:
// the button then silently did nothing (support: cbysea.se).
if (!selectedCustomer) {
toast({
title: t('review_customer_missing_title'),
description: t('review_customer_missing_description'),
variant: 'destructive',
})
return
}
setPendingData(data)
// Re-fetch the preview right before review so the displayed number
// reflects any concurrent invoice creations. Skip for delivery notes.
// Bounded: this blocks the review dialog from opening, and a hung fetch
// must not be able to freeze the flow (the catch below eats the abort).
if (data.document_type !== 'delivery_note') {
try {
const r = await fetch(`/api/invoices/next-number?document_type=${encodeURIComponent(data.document_type)}`)
const r = await fetch(
`/api/invoices/next-number?document_type=${encodeURIComponent(data.document_type)}`,
{ signal: AbortSignal.timeout(5000) },
)
if (r.ok) {
const json = await r.json()
setNumberPreview(json?.data?.preview ?? null)
@@ -1300,6 +1319,12 @@ export default function InvoiceEditor(props: InvoiceEditorProps = { mode: 'creat
if (!pendingData) return
setIsPreviewing(true)
// Open the tab synchronously inside the click's user activation. A
// window.open after the awaits below is popup-blocked whenever generation
// outlives the activation window (~5s): exactly the slow cold-start case,
// where the preview then silently did nothing (support: cbysea.se).
const tab = openDeferredTab(t('preview_pdf_generating'))
try {
const response = await fetch('/api/invoices/preview-pdf', {
method: 'POST',
@@ -1326,8 +1351,21 @@ export default function InvoiceEditor(props: InvoiceEditorProps = { mode: 'creat
const blob = await response.blob()
const url = window.URL.createObjectURL(blob)
window.open(url, '_blank')
if (!tab.navigate(url)) {
tab.close()
window.URL.revokeObjectURL(url)
toast({
title: t('preview_pdf_failed'),
description: tCommon('popup_blocked_description'),
variant: 'destructive',
})
return
}
// The blob URL must outlive the tab's load; revoke on a generous delay
// instead of leaking it for the page's lifetime.
window.setTimeout(() => window.URL.revokeObjectURL(url), 60_000)
} catch (error) {
tab.close()
toast({
title: t('preview_pdf_failed'),
description: getErrorMessage(error, { context: 'invoice' }),
@@ -2393,11 +2431,11 @@ export default function InvoiceEditor(props: InvoiceEditorProps = { mode: 'creat
type="submit"
className="w-full"
size="lg"
disabled={isSubmitting || isSavingDraft || !canWrite}
disabled={isSubmitting || isSavingDraft || isFormSubmitting || !canWrite}
title={!canWrite ? t('viewer_disabled_tooltip') : undefined}
>
{!canWrite && <Lock className="mr-2 h-4 w-4 inline" />}
{isEditMode && isSubmitting && <Loader2 className="mr-2 h-4 w-4 animate-spin" />}
{isFormSubmitting && !isSavingDraft && <Loader2 className="mr-2 h-4 w-4 animate-spin" />}
{isEditMode ? t('save_changes') : isSelfBilled ? ts('register') : t('review_and_create')}
</Button>
{!isEditMode && !isSelfBilled && watchDocumentType === 'invoice' && (
@@ -2406,7 +2444,7 @@ export default function InvoiceEditor(props: InvoiceEditorProps = { mode: 'creat
variant="outline"
className="w-full"
size="lg"
disabled={isSubmitting || isSavingDraft || !canWrite}
disabled={isSubmitting || isSavingDraft || isFormSubmitting || !canWrite}
title={!canWrite ? t('viewer_disabled_tooltip') : t('save_as_draft_tooltip')}
onClick={handleSubmit(saveDraftData)}
>
@@ -2435,7 +2473,7 @@ export default function InvoiceEditor(props: InvoiceEditorProps = { mode: 'creat
<Button
type="button"
variant="outline"
disabled={isSubmitting || isSavingDraft || !canWrite}
disabled={isSubmitting || isSavingDraft || isFormSubmitting || !canWrite}
onClick={handleSubmit(saveDraftData)}
>
{isSavingDraft ? <Loader2 className="h-4 w-4 animate-spin" /> : t('save_as_draft_short')}
@@ -2443,11 +2481,11 @@ export default function InvoiceEditor(props: InvoiceEditorProps = { mode: 'creat
)}
<Button
type="submit"
disabled={isSubmitting || isSavingDraft || !canWrite}
disabled={isSubmitting || isSavingDraft || isFormSubmitting || !canWrite}
title={!canWrite ? t('viewer_disabled_tooltip') : undefined}
>
{!canWrite && <Lock className="mr-2 h-4 w-4 inline" />}
{isEditMode && isSubmitting && <Loader2 className="mr-2 h-4 w-4 animate-spin" />}
{isFormSubmitting && !isSavingDraft && <Loader2 className="mr-2 h-4 w-4 animate-spin" />}
{isEditMode ? t('save_changes') : isSelfBilled ? ts('register') : t('review_and_create')}
</Button>
</div>
@@ -7,6 +7,7 @@ import { Paperclip, Loader2 } from 'lucide-react'
import { Badge } from '@/components/ui/badge'
import { cn } from '@/lib/utils'
import { useToast } from '@/components/ui/use-toast'
import { openDeferredTab } from '@/lib/browser/deferred-tab'
interface Props {
documentId: string | null | undefined
@@ -47,6 +48,7 @@ export function TransactionAttachmentIndicator({
className,
}: Props) {
const t = useTranslations('tx_underlag')
const tCommon = useTranslations('common')
const { toast } = useToast()
const [isLoading, setIsLoading] = useState(false)
@@ -55,16 +57,28 @@ export function TransactionAttachmentIndicator({
e.preventDefault()
if (isLoading || !documentId) return
setIsLoading(true)
// Pre-open inside the click's user activation: a window.open after the
// await is popup-blocked when the signed-URL fetch is slow.
const tab = openDeferredTab(tCommon('loading'))
try {
const res = await fetch(`/api/documents/${documentId}`)
if (!res.ok) {
tab.close()
toast({ title: t('open_failed'), variant: 'destructive' })
return
}
const { data } = await res.json()
if (data?.download_url) {
window.open(data.download_url, '_blank', 'noopener,noreferrer')
if (!data?.download_url || !tab.navigate(data.download_url)) {
tab.close()
toast({
title: t('open_failed'),
description: tab.blocked ? tCommon('popup_blocked_description') : undefined,
variant: 'destructive',
})
}
} catch {
tab.close()
toast({ title: t('open_failed'), variant: 'destructive' })
} finally {
setIsLoading(false)
}
@@ -0,0 +1,96 @@
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
import { openDeferredTab } from '@/lib/browser/deferred-tab'
function makeFakeTab() {
return {
closed: false,
opener: {} as unknown,
close: vi.fn(),
location: { href: '' },
document: {
title: '',
body: { appendChild: vi.fn() },
createElement: vi.fn(() => ({ textContent: '', style: { cssText: '' } })),
},
}
}
beforeEach(() => {
vi.clearAllMocks()
vi.unstubAllGlobals()
})
afterEach(() => {
vi.unstubAllGlobals()
})
describe('openDeferredTab', () => {
it('is SSR-safe: reports blocked when window is undefined', () => {
const tab = openDeferredTab()
expect(tab.blocked).toBe(true)
expect(tab.navigate('https://example.com')).toBe(false)
expect(() => tab.close()).not.toThrow()
})
it('reports blocked and refuses navigation when window.open returns null', () => {
vi.stubGlobal('window', { open: vi.fn(() => null) })
const tab = openDeferredTab('Laddar...')
expect(tab.blocked).toBe(true)
expect(tab.navigate('https://example.com')).toBe(false)
expect(() => tab.close()).not.toThrow()
})
it('opens synchronously, severs opener, and navigates the pending tab', () => {
const fake = makeFakeTab()
const open = vi.fn(() => fake)
vi.stubGlobal('window', { open })
const tab = openDeferredTab('Genererar...')
expect(open).toHaveBeenCalledWith('', '_blank')
expect(tab.blocked).toBe(false)
expect(fake.opener).toBeNull()
expect(fake.document.title).toBe('Genererar...')
expect(fake.document.body.appendChild).toHaveBeenCalledTimes(1)
expect(tab.navigate('blob:https://app/abc')).toBe(true)
expect(fake.location.href).toBe('blob:https://app/abc')
})
it('skips the placeholder when none is given', () => {
const fake = makeFakeTab()
vi.stubGlobal('window', { open: vi.fn(() => fake) })
openDeferredTab()
expect(fake.document.body.appendChild).not.toHaveBeenCalled()
expect(fake.document.title).toBe('')
})
it('fails navigation once the user closed the pending tab', () => {
const fake = makeFakeTab()
vi.stubGlobal('window', { open: vi.fn(() => fake) })
const tab = openDeferredTab()
fake.closed = true
expect(tab.navigate('https://example.com')).toBe(false)
})
it('close() closes an open tab and is a no-op afterwards', () => {
const fake = makeFakeTab()
vi.stubGlobal('window', { open: vi.fn(() => fake) })
const tab = openDeferredTab()
tab.close()
expect(fake.close).toHaveBeenCalledTimes(1)
fake.closed = true
tab.close()
expect(fake.close).toHaveBeenCalledTimes(1)
})
it('survives a placeholder document that throws', () => {
const fake = makeFakeTab()
fake.document.createElement = vi.fn(() => {
throw new Error('detached')
})
vi.stubGlobal('window', { open: vi.fn(() => fake) })
const tab = openDeferredTab('Laddar...')
expect(tab.blocked).toBe(false)
expect(tab.navigate('https://example.com')).toBe(true)
})
})
+69
View File
@@ -0,0 +1,69 @@
/**
* Pre-open a browser tab inside a click handler, then point it at a URL once
* an async result arrives.
*
* Why: window.open() called after an `await` runs outside the click's
* transient user activation. Browsers expire that activation after ~5 seconds
* (Safari sooner), so popup blocking kicks in exactly when the request is
* slow: a cold serverless start made "Förhandsgranska PDF" silently do
* nothing (support: cbysea.se). Opening the tab synchronously keeps the
* trusted gesture; the tab is then navigated to the real URL, or closed again
* if the request fails. Same pattern as AGIPanel's signing tab.
*/
export interface DeferredTab {
/** True when even the synchronous open was blocked (strict popup blocker). */
blocked: boolean
/** Point the pending tab at the final URL. False if the tab is gone. */
navigate(url: string): boolean
/** Close the pending tab; call on request failure so no blank tab lingers. */
close(): void
}
/**
* Must be called synchronously from a user-gesture handler (click), before
* any `await`, or the open is popup-blocked in the exact slow cases this
* helper exists for.
*/
export function openDeferredTab(placeholder?: string): DeferredTab {
const tab = typeof window !== 'undefined' ? window.open('', '_blank') : null
if (tab) {
try {
// Sever the reverse channel up front: the final page never gets a
// window.opener, preserving the noopener semantics the direct
// window.open(url, '_blank', 'noopener') call sites had.
tab.opener = null
if (placeholder) {
tab.document.title = placeholder
const note = tab.document.createElement('p')
note.textContent = placeholder
note.style.cssText =
'font-family: system-ui, sans-serif; color: #666; padding: 24px;'
tab.document.body.appendChild(note)
}
} catch {
// The placeholder is cosmetic; never let it break the open.
}
}
return {
blocked: tab === null,
navigate(url: string): boolean {
if (!tab || tab.closed) return false
try {
tab.location.href = url
return true
} catch {
return false
}
},
close() {
try {
if (tab && !tab.closed) tab.close()
} catch {
// Already gone; nothing to clean up.
}
},
}
}
+5
View File
@@ -20,6 +20,8 @@
"load_more": "Load more",
"retry": "Try again",
"load_error": "Could not load data",
"popup_blocked_title": "The browser blocked the tab",
"popup_blocked_description": "Allow pop-ups for Accounted in your browser and try again.",
"confirm": "Confirm",
"yes": "Yes",
"no": "No",
@@ -2881,6 +2883,8 @@
"preview_pdf": "Preview PDF",
"preview_pdf_generating": "Generating...",
"preview_pdf_failed": "Could not generate PDF",
"review_customer_missing_title": "Customer details could not be loaded",
"review_customer_missing_description": "Reload the page and try again. Contact support if the problem persists.",
"create_invoice_failed_title": "Could not create invoice",
"doc_created_title": "{docLabel} created",
"doc_created_description": "{docLabel} {number} has been created",
@@ -3925,6 +3929,7 @@
"remove_blocked_cancel_cta": "Close",
"replace_uploading": "Replacing...",
"remove_failed": "Could not remove the document.",
"download_failed": "Could not open the document",
"replace_failed": "Could not upload new version.",
"choose_from_inbox": "Choose from inbox",
"picker_title": "Choose a document from the inbox",
+5
View File
@@ -20,6 +20,8 @@
"load_more": "Ladda fler",
"retry": "Försök igen",
"load_error": "Kunde inte ladda data",
"popup_blocked_title": "Webbläsaren blockerade fliken",
"popup_blocked_description": "Tillåt popupfönster för Accounted i webbläsaren och försök igen.",
"confirm": "Bekräfta",
"yes": "Ja",
"no": "Nej",
@@ -2881,6 +2883,8 @@
"preview_pdf": "Förhandsgranska PDF",
"preview_pdf_generating": "Genererar...",
"preview_pdf_failed": "Kunde inte generera PDF",
"review_customer_missing_title": "Kunduppgifterna kunde inte laddas",
"review_customer_missing_description": "Ladda om sidan och försök igen. Kontakta support om det inte hjälper.",
"create_invoice_failed_title": "Kunde inte skapa faktura",
"doc_created_title": "{docLabel} skapad",
"doc_created_description": "{docLabel} {number} har skapats",
@@ -3925,6 +3929,7 @@
"remove_blocked_cancel_cta": "Stäng",
"replace_uploading": "Ersätter...",
"remove_failed": "Kunde inte ta bort underlaget.",
"download_failed": "Kunde inte öppna dokumentet",
"replace_failed": "Kunde inte ladda upp ny version.",
"choose_from_inbox": "Välj från inkorgen",
"picker_title": "Välj underlag från inkorgen",