fix(hub-ui): clean up inactive iframe docks (#364)

Author: dvcolomban

packages/hub-ui/src/client/components/dock/DockEdge.vue

@@ -7,7 +7,7 @@ import { computed, h, onMounted, ref, useTemplateRef } from 'vue'
 import { getEntryGroup } from '../../state/dock-settings'
 import { setEdgePositionDropdown, setFloatingTooltip, useDocksGroupPanel, useEdgePositionDropdown } from '../../state/floating-tooltip'
 import { useSettings } from '../../state/settings-defaults'
-import { useIframePanes } from '../../utils/useIframePanes'
+import { getEntryPaneKey, useIframePanes } from '../../utils/useIframePanes'
 import BrandMark from '../icons/BrandMark.vue'
 import ViewEntry from '../views/ViewEntry.vue'
 import { resolveDockEdge, resolveDockLayout } from './dock-layout'
@@ -27,7 +27,7 @@ const settings = useSettings(context)
 const layout = computed(() => resolveDockLayout(props.layout))
 
 const viewsContainer = useTemplateRef<HTMLElement>('viewsContainer')
-const panes = useIframePanes(viewsContainer, context.panel)
+const panes = useIframePanes(viewsContainer, context.panel, () => getEntryPaneKey(context.docks.selected))
 
 const isVertical = computed(() => store.position === 'left' || store.position === 'right')
 

packages/hub-ui/src/client/components/views/ViewIframe.vue

@@ -208,6 +208,7 @@ function openExternally() {
   catch {}
 }
 
+let mountedTarget: HTMLDivElement | null = null
 let onIframeLoad: (() => void) | undefined
 let stopLocationWatch: (() => void) | undefined
 
@@ -304,7 +305,8 @@ onMounted(() => {
 
   window.addEventListener('message', onWindowMessage)
 
-  pane.mount(viewFrame.value!)
+  mountedTarget = viewFrame.value
+  pane.mount(mountedTarget!)
   isLoading.value = false
   paneReady.value = true
   nextTick(() => {
@@ -314,20 +316,15 @@ onMounted(() => {
 
 onUnmounted(() => {
   window.removeEventListener('message', onWindowMessage)
-  // A shared frame outlives this view, so its page is left exactly as found;
-  // the incoming view starts its own watch.
-  stopLocationWatch?.()
-  stopLocationWatch = undefined
   const pane = props.panes.get(paneKey.value)
   if (pane && onIframeLoad)
     pane.iframe?.removeEventListener('load', onIframeLoad)
-  // Only unmount if this view still owns the pane. When switching between two
-  // docks sharing a `frameId`, the incoming view may re-mount the shared pane
-  // onto its own container before this outgoing view tears down; unmounting
-  // then would wrongly hide the just-revealed iframe. Guarding on the current
-  // target makes the handoff order-independent.
-  if (pane && pane.target === viewFrame.value)
+  /** Vue clears template refs before this hook; retain the target to check ownership across shared-iframe handoffs. */
+  if (pane && pane.target === mountedTarget)
     pane.unmount()
+  mountedTarget = null
+  stopLocationWatch?.()
+  stopLocationWatch = undefined
 })
 </script>
 

packages/hub/src/client/__tests__/frame-location.test.ts

@@ -72,7 +72,11 @@ function fakeFrame(initialHref: string, options: { navigation?: boolean, crossOr
           }
         : undefined,
       addEventListener: (type, listener) => void listeners.get(type)!.add(listener),
-      removeEventListener: (type, listener) => void listeners.get(type)!.delete(listener),
+      get removeEventListener() {
+        if (options.crossOrigin)
+          throw new DOMException('cross-origin', 'SecurityError')
+        return (type: 'popstate' | 'hashchange', listener: Listener) => void listeners.get(type)!.delete(listener)
+      },
     }
   }
 
@@ -122,6 +126,38 @@ function fakeFrame(initialHref: string, options: { navigation?: boolean, crossOr
 }
 
 describe('watchFrameLocation', () => {
+  it('resumes location tracking after navigating through a foreign origin', () => {
+    expect.assertions(5)
+    const options = { crossOrigin: false }
+    const frame = fakeFrame('http://localhost/app/', options)
+    const onChange = vi.fn()
+    const dispose = watchFrameLocation({ iframe: frame.iframe, onChange })
+
+    options.crossOrigin = true
+    expect(() => frame.load('http://example.test/')).not.toThrow()
+    expect(onChange).toHaveBeenCalledTimes(1)
+    options.crossOrigin = false
+    frame.load('http://localhost/returned')
+    expect(onChange).toHaveBeenLastCalledWith('http://localhost/returned')
+    frame.pushState('http://localhost/next')
+    expect(onChange).toHaveBeenLastCalledWith('http://localhost/next')
+    dispose()
+    expect(frame.isDetached()).toBe(true)
+  })
+
+  it('releases remaining subscriptions when the previous window is inaccessible', () => {
+    expect.assertions(4)
+    const options = { crossOrigin: false, navigation: true }
+    const frame = fakeFrame('http://localhost/app/', options)
+    const dispose = watchFrameLocation({ iframe: frame.iframe, onChange: vi.fn() })
+
+    expect(frame.isHistoryPristine()).toBe(false)
+    options.crossOrigin = true
+    expect(dispose).not.toThrow()
+    expect(frame.isHistoryPristine()).toBe(true)
+    expect(dispose).not.toThrow()
+  })
+
   it('reports pushState and replaceState by wrapping them, and restores them on dispose', () => {
     const frame = fakeFrame('http://localhost/app/')
     const onChange = vi.fn()

packages/hub/src/client/frame-location.ts

@@ -137,7 +137,14 @@ export function watchFrameLocation(options: WatchFrameLocationOptions): () => vo
     }
 
     detach = () => {
-      for (const off of listeners) off()
+      for (const unsubscribe of listeners) {
+        try {
+          unsubscribe()
+        }
+        catch {
+          /** Navigation can invalidate the old document's window or history; release the remaining subscriptions. */
+        }
+      }
     }
   }