diff --git a/apps/desktop/src/main/core/browser/Browser.ts b/apps/desktop/src/main/core/browser/Browser.ts index 182462ecd5..66cf3aa41f 100644 --- a/apps/desktop/src/main/core/browser/Browser.ts +++ b/apps/desktop/src/main/core/browser/Browser.ts @@ -4,7 +4,7 @@ import { join } from 'node:path'; import { APP_WINDOW_MIN_SIZE } from '@lobechat/desktop-bridge'; import type { MainBroadcastEventKey, MainBroadcastParams } from '@lobechat/electron-client-ipc'; import type { BrowserWindowConstructorOptions } from 'electron'; -import { BrowserWindow, ipcMain, screen, session as electronSession } from 'electron'; +import { BrowserWindow, ipcMain, screen, session as electronSession, shell } from 'electron'; import { preloadDir, resourcesDir } from '@/const/dir'; import { isMac } from '@/const/env'; @@ -163,6 +163,9 @@ export default class Browser { // Setup event listeners this.setupEventListeners(browserWindow); + + // Setup external link handler (prevents opening new windows in renderer) + this.setupWindowOpenHandler(browserWindow); } private initiateContentLoading(): void { @@ -187,6 +190,26 @@ export default class Browser { this.setupContextMenu(browserWindow); } + /** + * Setup window open handler to intercept external links + * Prevents opening new windows in renderer and uses system browser instead + */ + private setupWindowOpenHandler(browserWindow: BrowserWindow): void { + logger.debug(`[${this.identifier}] Setting up window open handler for external links`); + + browserWindow.webContents.setWindowOpenHandler(({ url }) => { + logger.info(`[${this.identifier}] Intercepted window open for URL: ${url}`); + + // Open external URL in system browser + shell.openExternal(url).catch((error) => { + logger.error(`[${this.identifier}] Failed to open external URL: ${url}`, error); + }); + + // Deny creating new window in renderer + return { action: 'deny' }; + }); + } + private setupWillPreventUnloadListener(browserWindow: BrowserWindow): void { logger.debug(`[${this.identifier}] Setting up 'will-prevent-unload' event listener.`); browserWindow.webContents.on('will-prevent-unload', (event) => { diff --git a/apps/desktop/src/main/core/browser/__tests__/Browser.test.ts b/apps/desktop/src/main/core/browser/__tests__/Browser.test.ts index b1153f9cbe..862d352444 100644 --- a/apps/desktop/src/main/core/browser/__tests__/Browser.test.ts +++ b/apps/desktop/src/main/core/browser/__tests__/Browser.test.ts @@ -41,6 +41,7 @@ const { mockBrowserWindow, mockNativeTheme, mockIpcMain, mockScreen, MockBrowser }, }, on: vi.fn(), + setWindowOpenHandler: vi.fn(), }, }; diff --git a/apps/desktop/src/preload/routeInterceptor.test.ts b/apps/desktop/src/preload/routeInterceptor.test.ts index ea169392d4..9cfbd71071 100644 --- a/apps/desktop/src/preload/routeInterceptor.test.ts +++ b/apps/desktop/src/preload/routeInterceptor.test.ts @@ -39,52 +39,6 @@ describe('setupRouteInterceptors', () => { document.body.innerHTML = ''; }); - describe('window.open interception', () => { - it('should intercept external URL and invoke openExternalLink', () => { - setupRouteInterceptors(); - - const externalUrl = 'https://google.com'; - const result = window.open(externalUrl, '_blank'); - - expect(invoke).toHaveBeenCalledWith('system.openExternalLink', externalUrl); - expect(result).toBeNull(); - }); - - it('should intercept URL object for external link', () => { - setupRouteInterceptors(); - - const externalUrl = new URL('https://github.com'); - const result = window.open(externalUrl, '_blank'); - - expect(invoke).toHaveBeenCalledWith('system.openExternalLink', 'https://github.com/'); - expect(result).toBeNull(); - }); - - it('should allow internal link to proceed with original window.open', () => { - setupRouteInterceptors(); - - const originalWindowOpen = window.open; - const internalUrl = 'http://localhost:3000/settings'; - - // We can't fully test the original behavior in happy-dom, but we can verify invoke is not called - window.open(internalUrl); - - expect(invoke).not.toHaveBeenCalledWith('system.openExternalLink', expect.anything()); - }); - - it('should handle relative URL that resolves as internal link', () => { - setupRouteInterceptors(); - - // In happy-dom, 'invalid-url' is resolved relative to window.location.href - // So it becomes 'http://localhost:3000/invalid-url' which is internal - const relativeUrl = 'invalid-url'; - window.open(relativeUrl); - - // Since it's internal, it won't call invoke for external link - expect(invoke).not.toHaveBeenCalledWith('system.openExternalLink', expect.anything()); - }); - }); - describe('link click interception', () => { it('should intercept external link clicks', async () => { setupRouteInterceptors(); @@ -189,184 +143,6 @@ describe('setupRouteInterceptors', () => { }); }); - describe('history.pushState interception', () => { - it('should intercept pushState for matched routes', () => { - setupRouteInterceptors(); - - const matchedRoute = { - description: 'Developer Tools', - enabled: true, - pathPrefix: '/desktop/devtools', - targetWindow: 'devtools', - }; - vi.mocked(findMatchingRoute).mockReturnValue(matchedRoute); - - const originalLength = history.length; - history.pushState({}, '', '/desktop/devtools'); - - expect(findMatchingRoute).toHaveBeenCalledWith('/desktop/devtools'); - expect(invoke).toHaveBeenCalledWith('windows.interceptRoute', { - path: '/desktop/devtools', - source: 'push-state', - url: 'http://localhost:3000/desktop/devtools', - }); - // Ensure navigation was prevented - expect(history.length).toBe(originalLength); - }); - - it('should not intercept if already on target page', () => { - setupRouteInterceptors(); - - vi.stubGlobal('location', { - href: 'http://localhost:3000/desktop/devtools/console', - origin: 'http://localhost:3000', - pathname: '/desktop/devtools/console', - }); - - const matchedRoute = { - description: 'Developer Tools', - enabled: true, - pathPrefix: '/desktop/devtools', - targetWindow: 'devtools', - }; - vi.mocked(findMatchingRoute).mockReturnValue(matchedRoute); - - history.pushState({}, '', '/desktop/devtools/network'); - - expect(consoleLogSpy).toHaveBeenCalledWith( - expect.stringContaining('Skip pushState interception'), - ); - }); - - it('should allow pushState for non-matched routes', () => { - setupRouteInterceptors(); - - vi.mocked(findMatchingRoute).mockReturnValue(undefined); - - history.pushState({}, '', '/chat/new'); - - expect(invoke).not.toHaveBeenCalledWith('windows.interceptRoute', expect.anything()); - }); - - it('should handle pushState errors gracefully', () => { - setupRouteInterceptors(); - - vi.mocked(findMatchingRoute).mockImplementation(() => { - throw new Error('Route matching error'); - }); - - history.pushState({}, '', '/some/path'); - - expect(consoleErrorSpy).toHaveBeenCalledWith( - expect.stringContaining('pushState interception error'), - expect.any(Error), - ); - }); - }); - - describe('history.replaceState interception', () => { - it('should intercept replaceState for matched routes', () => { - setupRouteInterceptors(); - - const matchedRoute = { - description: 'Developer Tools', - enabled: true, - pathPrefix: '/desktop/devtools', - targetWindow: 'devtools', - }; - vi.mocked(findMatchingRoute).mockReturnValue(matchedRoute); - - history.replaceState({}, '', '/desktop/devtools'); - - expect(findMatchingRoute).toHaveBeenCalledWith('/desktop/devtools'); - expect(invoke).toHaveBeenCalledWith('windows.interceptRoute', { - path: '/desktop/devtools', - source: 'replace-state', - url: 'http://localhost:3000/desktop/devtools', - }); - }); - - it('should not intercept if already on target page', () => { - setupRouteInterceptors(); - - vi.stubGlobal('location', { - href: 'http://localhost:3000/desktop/devtools/console', - origin: 'http://localhost:3000', - pathname: '/desktop/devtools/console', - }); - - const matchedRoute = { - description: 'Developer Tools', - enabled: true, - pathPrefix: '/desktop/devtools', - targetWindow: 'devtools', - }; - vi.mocked(findMatchingRoute).mockReturnValue(matchedRoute); - - history.replaceState({}, '', '/desktop/devtools/network'); - - expect(consoleLogSpy).toHaveBeenCalledWith( - expect.stringContaining('Skip replaceState interception'), - ); - }); - - it('should allow replaceState for non-matched routes', () => { - setupRouteInterceptors(); - - vi.mocked(findMatchingRoute).mockReturnValue(undefined); - - history.replaceState({}, '', '/chat/session-123'); - - expect(invoke).not.toHaveBeenCalledWith('windows.interceptRoute', expect.anything()); - }); - }); - - describe('error event interception', () => { - it('should prevent navigation errors for prevented paths', () => { - setupRouteInterceptors(); - - // First trigger a route interception to add path to preventedPaths - const matchedRoute = { - description: 'Developer Tools', - enabled: true, - pathPrefix: '/desktop/devtools', - targetWindow: 'devtools', - }; - vi.mocked(findMatchingRoute).mockReturnValue(matchedRoute); - history.pushState({}, '', '/desktop/devtools'); - - // Now trigger an error event with navigation in the message - const errorEvent = new ErrorEvent('error', { - bubbles: true, - cancelable: true, - message: 'navigation error occurred', - }); - const preventDefaultSpy = vi.spyOn(errorEvent, 'preventDefault'); - - window.dispatchEvent(errorEvent); - - expect(preventDefaultSpy).toHaveBeenCalled(); - expect(consoleLogSpy).toHaveBeenCalledWith( - expect.stringContaining('Captured possible routing error'), - ); - }); - - it('should not prevent non-navigation errors', () => { - setupRouteInterceptors(); - - const errorEvent = new ErrorEvent('error', { - bubbles: true, - cancelable: true, - message: 'some other error', - }); - const preventDefaultSpy = vi.spyOn(errorEvent, 'preventDefault'); - - window.dispatchEvent(errorEvent); - - expect(preventDefaultSpy).not.toHaveBeenCalled(); - }); - }); - describe('interceptRoute helper', () => { it('should handle successful route interception', async () => { vi.mocked(invoke).mockResolvedValue(undefined); @@ -381,13 +157,18 @@ describe('setupRouteInterceptors', () => { }; vi.mocked(findMatchingRoute).mockReturnValue(matchedRoute); - history.pushState({}, '', '/desktop/devtools'); + const link = document.createElement('a'); + link.href = 'http://localhost:3000/desktop/devtools'; + document.body.append(link); + + const clickEvent = new MouseEvent('click', { bubbles: true, cancelable: true }); + link.dispatchEvent(clickEvent); await new Promise((resolve) => setTimeout(resolve, 0)); expect(invoke).toHaveBeenCalledWith('windows.interceptRoute', { path: '/desktop/devtools', - source: 'push-state', + source: 'link-click', url: 'http://localhost:3000/desktop/devtools', }); }); @@ -406,12 +187,17 @@ describe('setupRouteInterceptors', () => { }; vi.mocked(findMatchingRoute).mockReturnValue(matchedRoute); - history.pushState({}, '', '/desktop/devtools'); + const link = document.createElement('a'); + link.href = 'http://localhost:3000/desktop/devtools'; + document.body.append(link); + + const clickEvent = new MouseEvent('click', { bubbles: true, cancelable: true }); + link.dispatchEvent(clickEvent); await new Promise((resolve) => setTimeout(resolve, 0)); expect(consoleErrorSpy).toHaveBeenCalledWith( - expect.stringContaining('Route interception (push-state) call failed'), + expect.stringContaining('Route interception (link-click) call failed'), error, ); }); diff --git a/apps/desktop/src/preload/routeInterceptor.ts b/apps/desktop/src/preload/routeInterceptor.ts index f7983247a9..922b6a6a35 100644 --- a/apps/desktop/src/preload/routeInterceptor.ts +++ b/apps/desktop/src/preload/routeInterceptor.ts @@ -2,11 +2,7 @@ import { findMatchingRoute } from '~common/routes'; import { invoke } from './invoke'; -const interceptRoute = async ( - path: string, - source: 'link-click' | 'push-state' | 'replace-state', - url: string, -) => { +const interceptRoute = async (path: string, source: 'link-click', url: string) => { console.log(`[preload] Intercepted ${source} and prevented default behavior:`, path); // Use electron-client-ipc's dispatch method @@ -16,43 +12,13 @@ const interceptRoute = async ( console.error(`[preload] Route interception (${source}) call failed`, e); } }; + /** * Route interceptor - Responsible for capturing and intercepting client-side route navigation */ export const setupRouteInterceptors = function () { console.log('[preload] Setting up route interceptors'); - // Store prevented paths to avoid pushState duplicate triggers - const preventedPaths = new Set(); - - // Override window.open method to intercept JavaScript calls - const originalWindowOpen = window.open; - window.open = function (url?: string | URL, target?: string, features?: string) { - if (url) { - try { - const urlString = typeof url === 'string' ? url : url.toString(); - const urlObj = new URL(urlString, window.location.href); - - // Check if it's an external link - if (urlObj.origin !== window.location.origin) { - console.log(`[preload] Intercepted window.open for external URL:`, urlString); - // Call main process to handle external link - invoke('system.openExternalLink', urlString); - return null; // Return null to indicate no window was opened - } - } catch (error) { - // Handle invalid URL or special protocol - console.error(`[preload] Intercepted window.open for special protocol:`, url); - console.error(error); - invoke('system.openExternalLink', typeof url === 'string' ? url : url.toString()); - return null; - } - } - - // For internal links, call original window.open - return originalWindowOpen.call(window, url, target, features); - }; - // Intercept all a tag click events - For Next.js Link component document.addEventListener( 'click', @@ -112,101 +78,5 @@ export const setupRouteInterceptors = function () { true, ); - // Intercept history API (for capturing Next.js useRouter().push/replace etc.) - const originalPushState = history.pushState; - const originalReplaceState = history.replaceState; - - // Override pushState - history.pushState = function () { - const url = arguments[2]; - if (typeof url === 'string') { - try { - // Only handle relative paths or current domain URLs - const parsedUrl = new URL(url, window.location.origin); - - // Use shared config to check if interception is needed - const matchedRoute = findMatchingRoute(parsedUrl.pathname); - - // Check if this navigation needs interception - if (matchedRoute) { - // Check if current page is already under target path, if so don't intercept - const currentPath = window.location.pathname; - const isAlreadyInTargetPage = currentPath.startsWith(matchedRoute.pathPrefix); - - // If already in target page, don't intercept, let default navigation continue - if (isAlreadyInTargetPage) { - console.log( - `[preload] Skip pushState interception for ${parsedUrl.pathname} because already in target page ${matchedRoute.pathPrefix}`, - ); - return Reflect.apply(originalPushState, this, arguments); - } - - // Add this path to prevented set - preventedPaths.add(parsedUrl.pathname); - - interceptRoute(parsedUrl.pathname, 'push-state', parsedUrl.href); - - // Don't execute original pushState operation, prevent navigation - // But return undefined to avoid errors - return; - } - } catch (err) { - console.error('[preload] pushState interception error:', err); - } - } - return Reflect.apply(originalPushState, this, arguments); - }; - - // Override replaceState - history.replaceState = function () { - const url = arguments[2]; - if (typeof url === 'string') { - try { - const parsedUrl = new URL(url, window.location.origin); - - // Use shared config to check if interception is needed - const matchedRoute = findMatchingRoute(parsedUrl.pathname); - - // Check if this navigation needs interception - if (matchedRoute) { - // Check if current page is already under target path, if so don't intercept - const currentPath = window.location.pathname; - const isAlreadyInTargetPage = currentPath.startsWith(matchedRoute.pathPrefix); - - // If already in target page, don't intercept, let default navigation continue - if (isAlreadyInTargetPage) { - console.log( - `[preload] Skip replaceState interception for ${parsedUrl.pathname} because already in target page ${matchedRoute.pathPrefix}`, - ); - return Reflect.apply(originalReplaceState, this, arguments); - } - - // Add to prevented set - preventedPaths.add(parsedUrl.pathname); - - interceptRoute(parsedUrl.pathname, 'replace-state', parsedUrl.href); - - // Prevent navigation - return; - } - } catch (err) { - console.error('[preload] replaceState interception error:', err); - } - } - return Reflect.apply(originalReplaceState, this, arguments); - }; - - // Listen and intercept routing errors - Sometimes Next.js tries to recover navigation on routing errors - window.addEventListener( - 'error', - function (e) { - if (e.message && e.message.includes('navigation') && preventedPaths.size > 0) { - console.log('[preload] Captured possible routing error, preventing default behavior'); - e.preventDefault(); - } - }, - true, - ); - console.log('[preload] Route interceptors setup completed'); };