Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions libs/widget-lib/src/cowSwapWidget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
if (typeof window === 'undefined') return noopHandler

// 1. Create a brand new iframe
const iframe = createIframe(currentParams)
let iframe = createIframe(currentParams)
const iframeOrigin = getIframeOrigin(iframe)
logWidget('Resolved trusted iframe origin', { iframeOrigin })
const windowListeners: WindowListener[] = []
Expand Down Expand Up @@ -160,12 +160,24 @@
// 10. Listen for Safe SDK messages from the iframe only when explicitly enabled by the host.
iframeSafeSdkBridge = createIframeSafeSdkBridge(enableSafeSdkBridge, window, iframeWindow, iframeOrigin)

const loadingContext = widgetIframeLoading(container, iframe, setup, destroy, props.onLoadingError)
const loadingContext = widgetIframeLoading(container, iframe, reloadIframe, props.onLoadingError)

cancelWidgetLoading = loadingContext.cancelWidgetLoading
const onWidgetReady = loadingContext.onWidgetReady
}

// Rebuild the iframe from scratch and re-run setup. Reusing the same sandboxed frame and only
// swapping its `src` did not reliably re-fetch the widget on retry, so a failed load could never
// recover. A brand new iframe gives a clean document that re-emits READY once it loads.
function reloadIframe(): void {
destroy()

iframe = createIframe(currentParams)
container.appendChild(iframe)

setup()
}

Comment on lines +163 to +180

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if the about:blank test still exists in the current branch
rg -n 'about:blank' libs/widget-lib/src/cowSwapWidget.test.ts
# Also check if the test captures handler.iframe before updateParams
rg -n -A5 'reloads the iframe with no popup' libs/widget-lib/src/cowSwapWidget.test.ts

Repository: cowprotocol/cowswap

Length of output: 477


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the widget implementation around the reported lines and the module-level reload helper.
sed -n '140,240p' libs/widget-lib/src/cowSwapWidget.ts
printf '\n---\n'
sed -n '300,360p' libs/widget-lib/src/cowSwapWidget.ts

printf '\n=== TEST CONTEXT ===\n'
sed -n '300,350p' libs/widget-lib/src/cowSwapWidget.test.ts

Repository: cowprotocol/cowswap

Length of output: 7842


updateParams is calling the local reloadIframe() helper, not the module-level sandbox swap.
The zero-arg inner function shadows reloadIframe(iframe, params), so the disableWindowOpen path now destroys and recreates the iframe instead of performing the about:blank transition. The existing test that captures handler.iframe before updateParams should either re-read the live iframe after the call or be updated to reflect the rebuild flow.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@libs/widget-lib/src/cowSwapWidget.ts` around lines 163 - 180, The inner
zero-argument reloadIframe function shadows the module-level
reloadIframe(iframe, params), causing updateParams to rebuild the iframe instead
of performing the about:blank sandbox transition when disableWindowOpen changes.
Rename or otherwise disambiguate the local helper and update updateParams to
call the intended module-level function; adjust the affected test to read
handler.iframe after updateParams or assert the rebuilt iframe behavior.

function destroy(skipIframeDestroy = false): void {
// Disconnect rpc provider and unsubscribe to events
iframeRpcProviderBridge?.disconnect()
Expand All @@ -192,14 +204,17 @@

// 11. Return the handler, so the widget, listeners, and provider can be updated
return {
iframe,
// `iframe` is rebuilt on retry, so expose it lazily to always hand back the live element.
get iframe() {
return iframe
},
updateParams: (newParams: CowSwapWidgetParams) => {
if (!iframeWindow) return
currentParams = resolveWidgetParams(newParams)

applyContainerStyles(container, currentParams, lastDynamicHeight)
if (requiresIframeReload(iframe, currentParams)) {
reloadIframe(iframe, currentParams)

Check failure on line 217 in libs/widget-lib/src/cowSwapWidget.ts

View workflow job for this annotation

GitHub Actions / Typecheck

Expected 0 arguments, but got 2.
} else {
updateParams(iframeWindow, iframeOrigin, currentParams, provider)
}
Expand Down
61 changes: 53 additions & 8 deletions libs/widget-lib/src/widgetIframeLoading.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,25 +54,70 @@ describe('widgetIframeLoading error UI styling', () => {

expect(document.head.querySelectorAll(`style[data-${ERROR_CLASS}]`).length).toBeLessThanOrEqual(1)
})

it('shows a loading state on the reload button while the retry is in progress', () => {
const { container, iframe } = setup()
failIframe(iframe)

const reloadBtn = container.querySelector<HTMLButtonElement>(`.${ERROR_CLASS} button`)
expect(reloadBtn?.disabled).toBe(false)

reloadBtn?.click()

// The panel stays mounted and the button is disabled (cursor:progress) while the retry runs.
expect(container.querySelector(`.${ERROR_CLASS}`)).not.toBeNull()
expect(reloadBtn?.disabled).toBe(true)
})

it('triggers a full reload when the reload button is clicked', () => {
const { container, iframe, reload } = setup()
failIframe(iframe)

container.querySelector<HTMLButtonElement>(`.${ERROR_CLASS} button`)?.click()

expect(reload).toHaveBeenCalledTimes(1)
})

it('removes the error panel and reveals the iframe once the widget is ready', () => {
const { container, iframe, state } = setup()
failIframe(iframe)
expect(container.querySelector(`.${ERROR_CLASS}`)).not.toBeNull()
expect(iframe.style.display).toBe('none')

state.onWidgetReady()

expect(container.querySelector(`.${ERROR_CLASS}`)).toBeNull()
expect(iframe.style.display).toBe('')
})

it('keeps a single error panel across repeated failures on the same widget', () => {
const { container, iframe } = setup()

failIframe(iframe)
failIframe(iframe)

expect(container.querySelectorAll(`.${ERROR_CLASS}`).length).toBe(1)
})
})

function setup(): { container: HTMLElement; iframe: HTMLIFrameElement; state: LoadingState } {
function setup(): {
container: HTMLElement
iframe: HTMLIFrameElement
state: LoadingState
reload: jest.Mock
} {
const container = document.createElement('div')
document.body.appendChild(container)

const iframe = document.createElement('iframe')
iframe.src = `${WIDGET_ORIGIN}/`
container.appendChild(iframe)

const state = widgetIframeLoading(
container,
iframe,
() => void 0,
() => void 0,
)
const reload = jest.fn()
const state = widgetIframeLoading(container, iframe, reload)
activeStates.push(state)

return { container, iframe, state }
return { container, iframe, state, reload }
}

function failIframe(iframe: HTMLIFrameElement): void {
Expand Down
36 changes: 21 additions & 15 deletions libs/widget-lib/src/widgetIframeLoading.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,36 +10,41 @@ interface WidgetIframeLoadingContext {
export function widgetIframeLoading(
container: HTMLElement,
iframe: HTMLIFrameElement,
setup: () => void,
destroy: (skipIframeDestroy?: boolean) => void,
reload: () => void,
onWidgetLoadingError?: () => void,
): WidgetIframeLoadingContext {
const originalSrc = iframe.src

let cancelled = false
let timeout: number | null = null

function removeLoadingErrorContainer(): void {
const existing = container.getElementsByClassName(WIDGET_LOADING_ERROR_CLASS)[0]
if (existing) container.removeChild(existing)
}

function onLoadingError(): void {
onWidgetLoadingError?.()

iframe.style.display = 'none'
ensureLoadingErrorStyles()

// Replace any previous error/loading panel so a repeated failure doesn't stack panels.
removeLoadingErrorContainer()

const loadingContainer = document.createElement('div')
const reloadBtn = document.createElement('button')
const errorText = document.createElement('p')

reloadBtn.innerText = 'Reload'
reloadBtn.addEventListener('click', () => {
container.removeChild(loadingContainer)
destroy(true)
iframe.src = 'about:blank'
iframe.style.display = ''

setTimeout(() => {
iframe.src = originalSrc
setup()
}, 100)
// Keep the panel mounted and switch it to a loading state so the retry is visibly in
// progress; it's cleared by onWidgetReady on success or replaced by onLoadingError on
// another failure. `reload` rebuilds the iframe from scratch so the widget actually
// re-fetches and re-emits READY — swapping `iframe.src` on the sandboxed frame in place
// did not reliably reload it.
reloadBtn.disabled = true
errorText.innerText = 'Loading…'

reload()
})

errorText.innerText = "Couldn't load the page. Please, try again later"
Expand Down Expand Up @@ -69,8 +74,9 @@ export function widgetIframeLoading(
onWidgetReady() {
if (timeout) clearTimeout(timeout)

const loadingContainer = document.getElementsByClassName(WIDGET_LOADING_ERROR_CLASS)[0]
if (loadingContainer) container.removeChild(loadingContainer)
// Reveal the iframe again — it is hidden while an error/retry panel is shown.
iframe.style.display = ''
removeLoadingErrorContainer()
},
cancelWidgetLoading() {
cancelled = true
Expand Down
Loading