From 66cdb02dd9dc18ef762f70f66e4c31b4f5a52755 Mon Sep 17 00:00:00 2001 From: "Tommy D. Rossi" Date: Fri, 6 Feb 2026 10:32:39 +0100 Subject: [PATCH] refactor: simplify iframe handling - use frame: Frame instead of iframe: Locator --- docs/framer-iframe-snapshot-guide.md | 4 +- playwriter/src/aria-snapshot.test.ts | 6 +- playwriter/src/aria-snapshot.ts | 159 +++++---------------------- playwriter/src/executor.ts | 29 +++-- playwriter/src/skill.md | 3 +- 5 files changed, 52 insertions(+), 149 deletions(-) diff --git a/docs/framer-iframe-snapshot-guide.md b/docs/framer-iframe-snapshot-guide.md index ff33566..ede734b 100644 --- a/docs/framer-iframe-snapshot-guide.md +++ b/docs/framer-iframe-snapshot-guide.md @@ -47,12 +47,12 @@ playwriter -s 1 -e "const iframe = page.locator(\"iframe[src*='plugins.framercdn - Run the accessibility snapshot on that iframe: ```bash -playwriter -s 1 -e "const iframe = page.locator(\"iframe[src*='plugins.framercdn.com']\"); console.log(await accessibilitySnapshot({ page, iframe }));" +playwriter -s 1 -e "const frame = await page.locator(\"iframe[src*='plugins.framercdn.com']\").contentFrame(); console.log(await accessibilitySnapshot({ page, frame }));" ``` - Validate the snapshot contains MCP UI text (confirms the panel is actually loaded): ```bash -playwriter -s 1 -e "const iframe = page.locator(\"iframe[src*='plugins.framercdn.com']\"); console.log(await accessibilitySnapshot({ page, iframe, search: /Control Framer with MCP|Login With Google/ }));" +playwriter -s 1 -e "const frame = await page.locator(\"iframe[src*='plugins.framercdn.com']\").contentFrame(); console.log(await accessibilitySnapshot({ page, frame, search: /Control Framer with MCP|Login With Google/ }));" ``` ## Expected iframe URL diff --git a/playwriter/src/aria-snapshot.test.ts b/playwriter/src/aria-snapshot.test.ts index 79f6981..3c20077 100644 --- a/playwriter/src/aria-snapshot.test.ts +++ b/playwriter/src/aria-snapshot.test.ts @@ -178,9 +178,13 @@ describe('aria-snapshot', () => { await page.locator('[data-testid="external-iframe"]').waitFor({ timeout: 5000 }) await page.frameLocator('[data-testid="external-iframe"]').locator('[data-testid="iframe-button"]').waitFor({ timeout: 5000 }) + // Convert iframe Locator to Frame + const iframeHandle = await page.locator('[data-testid="external-iframe"]').elementHandle() + const iframeFrame = await iframeHandle!.contentFrame() + const { snapshot } = await getAriaSnapshot({ page, - iframe: page.locator('[data-testid="external-iframe"]'), + frame: iframeFrame!, wsUrl: getCdpUrl({ port: TEST_PORT }), }) diff --git a/playwriter/src/aria-snapshot.ts b/playwriter/src/aria-snapshot.ts index edfefd0..d0f44cd 100644 --- a/playwriter/src/aria-snapshot.ts +++ b/playwriter/src/aria-snapshot.ts @@ -1,7 +1,7 @@ // Accessibility snapshot pipeline: build raw AX tree, filter to a // tree (interactive-only, labels/contexts, wrapper hoisting, ignored // indent preservation), then render lines and locators. -import type { Page, Locator, ElementHandle } from '@xmorse/playwright-core' +import type { Page, Locator, ElementHandle, Frame } from '@xmorse/playwright-core' import crypto from 'node:crypto' import fs from 'node:fs' import path from 'node:path' @@ -206,105 +206,6 @@ function toAttributeMap(attributes?: string[]): Map { return result } -function getAttributeValue(attributes: string[] | undefined, name: string): string | null { - if (!attributes) { - return null - } - const index = attributes.indexOf(name) - if (index === -1) { - return null - } - return attributes[index + 1] ?? null -} - -function normalizeUrlForCompare(url: string): string { - try { - const parsed = new URL(url) - return `${parsed.origin}${parsed.pathname}` - } catch { - return url - } -} - -// Resolve a locator -> iframe target session. -// CDP flow: -// - DOM.getDocument + DOM.querySelector + DOM.describeNode: find the iframe element and read its src + frameId. -// - Target.getTargets: list all CDP targets (including OOPIF iframe targets). -// - Target.attachToTarget: attach to the matching iframe target to obtain the child sessionId. -// If no iframe target is found, we fall back to the iframe element's frameId for same-process iframes. -async function resolveIframeTarget({ - session, - iframe, -}: { - session: ICDPSession - iframe: Locator -}): Promise<{ sessionId: string | null; targetId: string | null; frameId: Protocol.Page.FrameId | null }> { - const attrName = 'data-pw-iframe-scope' - const attrValue = crypto.randomUUID() - - await iframe.evaluate((element, data) => { - element.setAttribute(data.name, data.value) - }, { name: attrName, value: attrValue }) - - try { - const { root } = await session.send('DOM.getDocument') as Protocol.DOM.GetDocumentResponse - const { nodeId } = await session.send('DOM.querySelector', { - nodeId: root.nodeId, - selector: `[${attrName}="${attrValue}"]`, - }) as Protocol.DOM.QuerySelectorResponse - - if (!nodeId) { - throw new Error('Iframe locator did not resolve to a DOM node') - } - - const { node } = await session.send('DOM.describeNode', { nodeId }) as Protocol.DOM.DescribeNodeResponse - const iframeSrc = getAttributeValue(node.attributes, 'src') - if (!iframeSrc) { - throw new Error('Iframe element has no src attribute') - } - const frameId = node.frameId ?? null - - const { targetInfos } = await session.send('Target.getTargets') as Protocol.Target.GetTargetsResponse - const iframeTargets = targetInfos.filter((target) => target.type === 'iframe') - const normalizedSrc = normalizeUrlForCompare(iframeSrc) - - const exactMatch = iframeTargets.find((target) => target.url === iframeSrc) - const normalizedMatch = iframeTargets.find((target) => normalizeUrlForCompare(target.url) === normalizedSrc) - const originMatch = (() => { - try { - const { origin } = new URL(iframeSrc) - return iframeTargets.find((target) => target.url.startsWith(origin)) - } catch { - return undefined - } - })() - - const target = exactMatch || normalizedMatch || originMatch - if (!target) { - if (!frameId) { - throw new Error(`No iframe target found for src: ${iframeSrc}`) - } - return { sessionId: null, targetId: null, frameId } - } - - const { sessionId } = await session.send('Target.attachToTarget', { - targetId: target.targetId, - flatten: true, - }) as Protocol.Target.AttachToTargetResponse - - return { sessionId, targetId: target.targetId, frameId } - } finally { - try { - await iframe.evaluate((element, data) => { - element.removeAttribute(data.name) - }, { name: attrName }) - } catch (error) { - throw new Error('Failed to cleanup iframe scope marker', { cause: error }) - } - } -} - - function getStableRefFromAttributes(attributes: Map): { value: string; attr: string } | null { const id = attributes.get('id') if (id) { @@ -845,9 +746,9 @@ function isSubstringOfAny(needle: string, haystack: Set): boolean { * await page.locator(selector).click() * ``` */ -export async function getAriaSnapshot({ page, iframe, locator, refFilter, wsUrl, interactiveOnly = false, cdp }: { +export async function getAriaSnapshot({ page, frame, locator, refFilter, wsUrl, interactiveOnly = false, cdp }: { page: Page - iframe?: Locator + frame?: Frame locator?: Locator refFilter?: (info: { role: string; name: string }) => boolean wsUrl?: string @@ -855,28 +756,30 @@ export async function getAriaSnapshot({ page, iframe, locator, refFilter, wsUrl, cdp?: ICDPSession }): Promise { const session = cdp || await getCDPSessionForPage({ page, wsUrl }) - let iframeSessionId: string | null = null - let iframeTargetId: string | null = null - let iframeFrameId: Protocol.Page.FrameId | null = null - if (iframe) { - const resolved = await resolveIframeTarget({ session, iframe }) - iframeSessionId = resolved.sessionId - iframeTargetId = resolved.targetId - iframeFrameId = resolved.frameId - if (!iframeSessionId && !iframeFrameId) { - throw new Error('Failed to resolve iframe target or frameId') + + // For cross-origin iframes (OOPIFs), we need to attach to the iframe's target + // to get a separate CDP session. Same-origin iframes can use frameId directly. + let oopifSessionId: string | null = null + const frameId = frame?.frameId() ?? null + + if (frameId) { + const { targetInfos } = await session.send('Target.getTargets') as Protocol.Target.GetTargetsResponse + const frameUrl = frame!.url() + const iframeTarget = targetInfos.find((t) => { + return t.type === 'iframe' && t.url === frameUrl + }) + if (iframeTarget) { + const { sessionId } = await session.send('Target.attachToTarget', { + targetId: iframeTarget.targetId, + flatten: true, + }) as Protocol.Target.AttachToTargetResponse + oopifSessionId = sessionId + await session.send('Runtime.runIfWaitingForDebugger', undefined, oopifSessionId) } } - if (iframeSessionId) { - try { - await session.send('Runtime.runIfWaitingForDebugger', undefined, iframeSessionId) - } catch (error) { - throw new Error('Failed to resume iframe target session', { cause: error }) - } - } - await session.send('DOM.enable', undefined, iframeSessionId) - await session.send('Accessibility.enable', undefined, iframeSessionId) + await session.send('DOM.enable', undefined, oopifSessionId) + await session.send('Accessibility.enable', undefined, oopifSessionId) const scopeAttr = 'data-pw-scope' const scopeValue = crypto.randomUUID() let scopeApplied = false @@ -890,7 +793,7 @@ export async function getAriaSnapshot({ page, iframe, locator, refFilter, wsUrl, scopeApplied = true } - const { nodes: domNodes } = await session.send('DOM.getFlattenedDocument', { depth: -1, pierce: true }, iframeSessionId) as Protocol.DOM.GetFlattenedDocumentResponse + const { nodes: domNodes } = await session.send('DOM.getFlattenedDocument', { depth: -1, pierce: true }, oopifSessionId) as Protocol.DOM.GetFlattenedDocumentResponse const { domById, domByBackendId, childrenByParent } = buildDomIndex(domNodes) let scopeRootNodeId: Protocol.DOM.NodeId | null = null @@ -909,8 +812,8 @@ export async function getAriaSnapshot({ page, iframe, locator, refFilter, wsUrl, ? buildBackendIdSet(scopeRootNodeId, childrenByParent, domById) : null - const axParams = !iframeSessionId && iframeFrameId ? { frameId: iframeFrameId } : undefined - const { nodes: axNodes } = await session.send('Accessibility.getFullAXTree', axParams, iframeSessionId) as Protocol.Accessibility.GetFullAXTreeResponse + const axParams = !oopifSessionId && frameId ? { frameId } : undefined + const { nodes: axNodes } = await session.send('Accessibility.getFullAXTree', axParams, oopifSessionId) as Protocol.Accessibility.GetFullAXTreeResponse const axById = new Map() for (const node of axNodes) { @@ -1148,12 +1051,8 @@ export async function getAriaSnapshot({ page, iframe, locator, refFilter, wsUrl, element.removeAttribute(attr) }, scopeAttr) } - if (iframeSessionId) { - try { - await session.send('Target.detachFromTarget', { sessionId: iframeSessionId }) - } catch (error) { - throw new Error(`Failed to detach iframe target ${iframeTargetId ?? 'unknown'}`, { cause: error }) - } + if (oopifSessionId) { + await session.send('Target.detachFromTarget', { sessionId: oopifSessionId }).catch(() => {}) } if (!cdp) { await session.detach() diff --git a/playwriter/src/executor.ts b/playwriter/src/executor.ts index 138a73c..45781d9 100644 --- a/playwriter/src/executor.ts +++ b/playwriter/src/executor.ts @@ -293,7 +293,8 @@ export class PlaywrightExecutor { } private setupPageConsoleListener(page: Page) { - const targetId = (page as any)._guid as string | undefined + // Use targetId() if available, fallback to internal _guid for CDP connections + const targetId = page.targetId() || (page as any)._guid as string | undefined if (!targetId) { return } @@ -532,7 +533,8 @@ export class PlaywrightExecutor { const accessibilitySnapshot = async (options: { page?: Page - iframe?: Locator + /** Optional frame to scope the snapshot (e.g. from iframe.contentFrame()) */ + frame?: Frame /** Optional locator to scope the snapshot to a subtree */ locator?: Locator search?: string | RegExp @@ -542,7 +544,7 @@ export class PlaywrightExecutor { /** Only include interactive elements (default: true) */ interactiveOnly?: boolean }) => { - const { page: targetPage, iframe, locator, search, showDiffSinceLastCall = true, interactiveOnly = false } = options + const { page: targetPage, frame, locator, search, showDiffSinceLastCall = true, interactiveOnly = false } = options const resolvedPage = targetPage || page if (!resolvedPage) { throw new Error('accessibilitySnapshot requires a page') @@ -551,7 +553,7 @@ export class PlaywrightExecutor { // Use new in-page implementation via getAriaSnapshot const { snapshot: rawSnapshot, refs, getSelectorForRef } = await getAriaSnapshot({ page: resolvedPage, - iframe, + frame, locator, wsUrl: getCdpUrl(this.cdpConfig), interactiveOnly, @@ -567,7 +569,7 @@ export class PlaywrightExecutor { } this.lastRefToLocator.set(resolvedPage, refToLocator) - const shouldCacheSnapshot = !iframe + const shouldCacheSnapshot = !frame const previousSnapshot = shouldCacheSnapshot ? this.lastSnapshots.get(resolvedPage) : undefined if (shouldCacheSnapshot) { this.lastSnapshots.set(resolvedPage, snapshotStr) @@ -656,18 +658,16 @@ export class PlaywrightExecutor { }) } - const getPageTargetId = async (p: Page): Promise => { - const guid = (p as any)._guid - if (guid) return guid - throw new Error('Could not get page identifier: _guid not available') - } - const getLatestLogs = async (options?: { page?: Page; count?: number; search?: string | RegExp }) => { const { page: filterPage, count, search } = options || {} let allLogs: string[] = [] if (filterPage) { - const targetId = await getPageTargetId(filterPage) + // Use targetId() if available, fallback to internal _guid for CDP connections + const targetId = filterPage.targetId() || (filterPage as any)._guid as string | undefined + if (!targetId) { + throw new Error('Could not get page targetId') + } const pageLogs = this.browserLogs.get(targetId) || [] allLogs = [...pageLogs] } else { @@ -781,9 +781,8 @@ export class PlaywrightExecutor { ) => { return async (options: T = {} as T) => { const targetPage = options.page || page - // Get sessionId from cached CDP session to identify which tab to record - const cdp = await getCDPSession({ page: targetPage }) - const sessionId = options.sessionId || cdp.getSessionId() || undefined + // Use Playwright's exposed sessionId directly + const sessionId = options.sessionId || targetPage.sessionId() || undefined return fn({ page: targetPage, sessionId, relayPort, ...options }) } } diff --git a/playwriter/src/skill.md b/playwriter/src/skill.md index 935044b..1e7ec60 100644 --- a/playwriter/src/skill.md +++ b/playwriter/src/skill.md @@ -72,7 +72,8 @@ playwriter -s 1 -e "await page.screenshot({ path: 'screenshot.png', scale: 'css' playwriter -s 1 -e "await accessibilitySnapshot({ page })" # Get accessibility snapshot for a specific iframe -await accessibilitySnapshot({ iframe: page.locator('iframe[src*="iframe.html"]') }) +const frame = await page.locator('iframe').contentFrame() +await accessibilitySnapshot({ frame }) ``` **Multiline code:**