Skip to content

Commit dc953dd

Browse files
committed
fix(hub-ui): clean up inactive iframe docks
1 parent 6ba7c59 commit dc953dd

4 files changed

Lines changed: 55 additions & 15 deletions

File tree

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { computed, h, onMounted, ref, useTemplateRef } from 'vue'
77
import { getEntryGroup } from '../../state/dock-settings'
88
import { setEdgePositionDropdown, setFloatingTooltip, useDocksGroupPanel, useEdgePositionDropdown } from '../../state/floating-tooltip'
99
import { useSettings } from '../../state/settings-defaults'
10-
import { useIframePanes } from '../../utils/useIframePanes'
10+
import { getEntryPaneKey, useIframePanes } from '../../utils/useIframePanes'
1111
import BrandMark from '../icons/BrandMark.vue'
1212
import ViewEntry from '../views/ViewEntry.vue'
1313
import { resolveDockEdge, resolveDockLayout } from './dock-layout'
@@ -27,7 +27,7 @@ const settings = useSettings(context)
2727
const layout = computed(() => resolveDockLayout(props.layout))
2828
2929
const viewsContainer = useTemplateRef<HTMLElement>('viewsContainer')
30-
const panes = useIframePanes(viewsContainer, context.panel)
30+
const panes = useIframePanes(viewsContainer, context.panel, () => getEntryPaneKey(context.docks.selected))
3131
3232
const isVertical = computed(() => store.position === 'left' || store.position === 'right')
3333

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

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,7 @@ function openExternally() {
208208
catch {}
209209
}
210210
211+
let mountedTarget: HTMLDivElement | null = null
211212
let onIframeLoad: (() => void) | undefined
212213
let stopLocationWatch: (() => void) | undefined
213214
@@ -304,7 +305,8 @@ onMounted(() => {
304305
305306
window.addEventListener('message', onWindowMessage)
306307
307-
pane.mount(viewFrame.value!)
308+
mountedTarget = viewFrame.value
309+
pane.mount(mountedTarget!)
308310
isLoading.value = false
309311
paneReady.value = true
310312
nextTick(() => {
@@ -314,20 +316,15 @@ onMounted(() => {
314316
315317
onUnmounted(() => {
316318
window.removeEventListener('message', onWindowMessage)
317-
// A shared frame outlives this view, so its page is left exactly as found;
318-
// the incoming view starts its own watch.
319-
stopLocationWatch?.()
320-
stopLocationWatch = undefined
321319
const pane = props.panes.get(paneKey.value)
322320
if (pane && onIframeLoad)
323321
pane.iframe?.removeEventListener('load', onIframeLoad)
324-
// Only unmount if this view still owns the pane. When switching between two
325-
// docks sharing a `frameId`, the incoming view may re-mount the shared pane
326-
// onto its own container before this outgoing view tears down; unmounting
327-
// then would wrongly hide the just-revealed iframe. Guarding on the current
328-
// target makes the handoff order-independent.
329-
if (pane && pane.target === viewFrame.value)
322+
/** Vue clears template refs before this hook; retain the target to check ownership across shared-iframe handoffs. */
323+
if (pane && pane.target === mountedTarget)
330324
pane.unmount()
325+
mountedTarget = null
326+
stopLocationWatch?.()
327+
stopLocationWatch = undefined
331328
})
332329
</script>
333330

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

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,11 @@ function fakeFrame(initialHref: string, options: { navigation?: boolean, crossOr
7272
}
7373
: undefined,
7474
addEventListener: (type, listener) => void listeners.get(type)!.add(listener),
75-
removeEventListener: (type, listener) => void listeners.get(type)!.delete(listener),
75+
get removeEventListener() {
76+
if (options.crossOrigin)
77+
throw new DOMException('cross-origin', 'SecurityError')
78+
return (type: 'popstate' | 'hashchange', listener: Listener) => void listeners.get(type)!.delete(listener)
79+
},
7680
}
7781
}
7882

@@ -122,6 +126,38 @@ function fakeFrame(initialHref: string, options: { navigation?: boolean, crossOr
122126
}
123127

124128
describe('watchFrameLocation', () => {
129+
it('resumes location tracking after navigating through a foreign origin', () => {
130+
expect.assertions(5)
131+
const options = { crossOrigin: false }
132+
const frame = fakeFrame('http://localhost/app/', options)
133+
const onChange = vi.fn()
134+
const dispose = watchFrameLocation({ iframe: frame.iframe, onChange })
135+
136+
options.crossOrigin = true
137+
expect(() => frame.load('http://example.test/')).not.toThrow()
138+
expect(onChange).toHaveBeenCalledTimes(1)
139+
options.crossOrigin = false
140+
frame.load('http://localhost/returned')
141+
expect(onChange).toHaveBeenLastCalledWith('http://localhost/returned')
142+
frame.pushState('http://localhost/next')
143+
expect(onChange).toHaveBeenLastCalledWith('http://localhost/next')
144+
dispose()
145+
expect(frame.isDetached()).toBe(true)
146+
})
147+
148+
it('releases remaining subscriptions when the previous window is inaccessible', () => {
149+
expect.assertions(4)
150+
const options = { crossOrigin: false, navigation: true }
151+
const frame = fakeFrame('http://localhost/app/', options)
152+
const dispose = watchFrameLocation({ iframe: frame.iframe, onChange: vi.fn() })
153+
154+
expect(frame.isHistoryPristine()).toBe(false)
155+
options.crossOrigin = true
156+
expect(dispose).not.toThrow()
157+
expect(frame.isHistoryPristine()).toBe(true)
158+
expect(dispose).not.toThrow()
159+
})
160+
125161
it('reports pushState and replaceState by wrapping them, and restores them on dispose', () => {
126162
const frame = fakeFrame('http://localhost/app/')
127163
const onChange = vi.fn()

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,14 @@ export function watchFrameLocation(options: WatchFrameLocationOptions): () => vo
137137
}
138138

139139
detach = () => {
140-
for (const off of listeners) off()
140+
for (const unsubscribe of listeners) {
141+
try {
142+
unsubscribe()
143+
}
144+
catch {
145+
/** Navigation can invalidate the old document's window or history; release the remaining subscriptions. */
146+
}
147+
}
141148
}
142149
}
143150

0 commit comments

Comments
 (0)