From ec3a52f012da5bba9218f164e0270f330e67eba7 Mon Sep 17 00:00:00 2001
From: yyh <92089059+lyzno1@users.noreply.github.com>
Date: Wed, 10 Dec 2025 19:12:14 +0800
Subject: [PATCH] Fix immediate window open defaults and error handling
 (#29417)

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
---
 web/hooks/use-async-window-open.spec.ts | 73 ++++++++++++++++++++++++-
 web/hooks/use-async-window-open.ts      | 12 +++-
 2 files changed, 81 insertions(+), 4 deletions(-)

diff --git a/web/hooks/use-async-window-open.spec.ts b/web/hooks/use-async-window-open.spec.ts
index 63ec9185da..5c1410b2c1 100644
--- a/web/hooks/use-async-window-open.spec.ts
+++ b/web/hooks/use-async-window-open.spec.ts
@@ -12,8 +12,9 @@ describe('useAsyncWindowOpen', () => {
     window.open = originalOpen
   })
 
-  it('opens immediate url synchronously without calling async getter', async () => {
-    const openSpy = jest.fn()
+  it('opens immediate url synchronously, clears opener, without calling async getter', async () => {
+    const mockWindow: any = { opener: 'should-clear' }
+    const openSpy = jest.fn(() => mockWindow)
     window.open = openSpy
     const getUrl = jest.fn()
     const { result } = renderHook(() => useAsyncWindowOpen())
@@ -22,12 +23,54 @@ describe('useAsyncWindowOpen', () => {
       await result.current(getUrl, {
         immediateUrl: 'https://example.com',
         target: '_blank',
-        features: 'noopener,noreferrer',
+        features: undefined,
       })
     })
 
     expect(openSpy).toHaveBeenCalledWith('https://example.com', '_blank', 'noopener,noreferrer')
     expect(getUrl).not.toHaveBeenCalled()
+    expect(mockWindow.opener).toBeNull()
+  })
+
+  it('appends noopener,noreferrer when immediate open passes custom features', async () => {
+    const mockWindow: any = { opener: 'should-clear' }
+    const openSpy = jest.fn(() => mockWindow)
+    window.open = openSpy
+    const getUrl = jest.fn()
+    const { result } = renderHook(() => useAsyncWindowOpen())
+
+    await act(async () => {
+      await result.current(getUrl, {
+        immediateUrl: 'https://example.com',
+        target: '_blank',
+        features: 'width=500',
+      })
+    })
+
+    expect(openSpy).toHaveBeenCalledWith('https://example.com', '_blank', 'width=500,noopener,noreferrer')
+    expect(getUrl).not.toHaveBeenCalled()
+    expect(mockWindow.opener).toBeNull()
+  })
+
+  it('reports error when immediate window fails to open', async () => {
+    const openSpy = jest.fn(() => null)
+    window.open = openSpy
+    const getUrl = jest.fn()
+    const onError = jest.fn()
+    const { result } = renderHook(() => useAsyncWindowOpen())
+
+    await act(async () => {
+      await result.current(getUrl, {
+        immediateUrl: 'https://example.com',
+        target: '_blank',
+        onError,
+      })
+    })
+
+    expect(onError).toHaveBeenCalled()
+    const errArg = onError.mock.calls[0][0] as Error
+    expect(errArg.message).toBe('Failed to open new window')
+    expect(getUrl).not.toHaveBeenCalled()
   })
 
   it('sets opener to null and redirects when async url resolves', async () => {
@@ -75,6 +118,30 @@ describe('useAsyncWindowOpen', () => {
     expect(mockWindow.location.href).toBe('')
   })
 
+  it('preserves custom features as-is for async open', async () => {
+    const close = jest.fn()
+    const mockWindow: any = {
+      location: { href: '' },
+      close,
+      opener: 'should-be-cleared',
+    }
+    const openSpy = jest.fn(() => mockWindow)
+    window.open = openSpy
+    const { result } = renderHook(() => useAsyncWindowOpen())
+
+    await act(async () => {
+      await result.current(async () => 'https://example.com/path', {
+        target: '_blank',
+        features: 'width=500',
+      })
+    })
+
+    expect(openSpy).toHaveBeenCalledWith('about:blank', '_blank', 'width=500')
+    expect(mockWindow.opener).toBeNull()
+    expect(mockWindow.location.href).toBe('https://example.com/path')
+    expect(close).not.toHaveBeenCalled()
+  })
+
   it('closes placeholder and reports when no url is returned', async () => {
     const close = jest.fn()
     const mockWindow: any = {
diff --git a/web/hooks/use-async-window-open.ts b/web/hooks/use-async-window-open.ts
index e3d7910217..b640fe430c 100644
--- a/web/hooks/use-async-window-open.ts
+++ b/web/hooks/use-async-window-open.ts
@@ -17,8 +17,18 @@ export const useAsyncWindowOpen = () => useCallback(async (getUrl: GetUrl, optio
     onError,
   } = options ?? {}
 
+  const secureImmediateFeatures = features ? `${features},noopener,noreferrer` : 'noopener,noreferrer'
+
   if (immediateUrl) {
-    window.open(immediateUrl, target, features)
+    const newWindow = window.open(immediateUrl, target, secureImmediateFeatures)
+    if (!newWindow) {
+      onError?.(new Error('Failed to open new window'))
+      return
+    }
+    try {
+      newWindow.opener = null
+    }
+    catch { /* noop */ }
     return
   }