From 51bf9f15c810724fce91307fe178d2dd93846a42 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Mon, 16 Dec 2019 21:59:23 -0800 Subject: [PATCH] fix(scrollIntoView): make it work for nested frames case --- src/chromium/FrameManager.ts | 17 +++++++++++++++-- src/dom.ts | 12 ++++++++++++ src/firefox/FrameManager.ts | 4 ++++ src/page.ts | 1 + src/webkit/FrameManager.ts | 4 ++++ test/click.spec.js | 3 +-- test/utils.js | 4 ---- 7 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/chromium/FrameManager.ts b/src/chromium/FrameManager.ts index f8f5617694..d67ec540fe 100644 --- a/src/chromium/FrameManager.ts +++ b/src/chromium/FrameManager.ts @@ -445,10 +445,13 @@ export class FrameManager implements PageDelegate { } async getContentFrame(handle: dom.ElementHandle): Promise { + const doc = await handle.evaluateHandle(node => node.ownerDocument ? node.ownerDocument.documentElement : null); + if (!doc) + return null; const nodeInfo = await this._client.send('DOM.describeNode', { - objectId: toRemoteObject(handle).objectId + objectId: toRemoteObject(doc).objectId }); - if (typeof nodeInfo.node.frameId !== 'string') + if (!nodeInfo || typeof nodeInfo.node.frameId !== 'string') return null; return this._page._frameManager.frame(nodeInfo.node.frameId); } @@ -510,6 +513,16 @@ export class FrameManager implements PageDelegate { throw new Error('Unable to adopt element handle from a different document'); return to._createHandle(result.object).asElement()!; } + + async getFrameOwner(frame: frames.Frame): Promise { + if (!frame.parentFrame()) + return null; + const result = await this._client.send('DOM.getFrameOwner', { frameId: frame._id }); + if (!result) + return null; + const utilityContext = await frame.parentFrame()._utilityContext(); + return this.adoptBackendNodeId(result.backendNodeId, utilityContext); + } } function toRemoteObject(handle: dom.ElementHandle): Protocol.Runtime.RemoteObject { diff --git a/src/dom.ts b/src/dom.ts index e6cc789ad0..fecca5f42c 100644 --- a/src/dom.ts +++ b/src/dom.ts @@ -152,6 +152,18 @@ export class ElementHandle extends js.JSHandle { } async _scrollIntoViewIfNeeded() { + const path: frames.Frame[] = []; + for (let frame = await this.contentFrame(); frame; frame = frame.parentFrame()) + path.push(frame); + for (const frame of path.reverse()) { + const owner = await frame._page._delegate.getFrameOwner(frame); + if (owner) + await owner._scrollIntoViewIfNeededWithinFrame(); + } + this._scrollIntoViewIfNeededWithinFrame(); + } + + async _scrollIntoViewIfNeededWithinFrame() { const error = await this.evaluate(async (node: Node, pageJavascriptEnabled: boolean) => { if (!node.isConnected) return 'Node is detached from document'; diff --git a/src/firefox/FrameManager.ts b/src/firefox/FrameManager.ts index 6d5a357b0d..9e8e1d369a 100644 --- a/src/firefox/FrameManager.ts +++ b/src/firefox/FrameManager.ts @@ -406,6 +406,10 @@ export class FrameManager implements PageDelegate { assert(false, 'Multiple isolated worlds are not implemented'); return handle; } + + async getFrameOwner(frame: frames.Frame): Promise { + throw new Error('Not implemented'); + } } export function normalizeWaitUntil(waitUntil: frames.LifecycleEvent | frames.LifecycleEvent[]): frames.LifecycleEvent[] { diff --git a/src/page.ts b/src/page.ts index 5d2fc4cab9..4072cd3036 100644 --- a/src/page.ts +++ b/src/page.ts @@ -62,6 +62,7 @@ export interface PageDelegate { isElementHandle(remoteObject: any): boolean; adoptElementHandle(handle: dom.ElementHandle, to: dom.FrameExecutionContext): Promise>; getContentFrame(handle: dom.ElementHandle): Promise; + getFrameOwner(frame: frames.Frame): Promise; getContentQuads(handle: dom.ElementHandle): Promise; layoutViewport(): Promise<{ width: number, height: number }>; setInputFiles(handle: dom.ElementHandle, files: input.FilePayload[]): Promise; diff --git a/src/webkit/FrameManager.ts b/src/webkit/FrameManager.ts index f0da9baa98..c06796a4dc 100644 --- a/src/webkit/FrameManager.ts +++ b/src/webkit/FrameManager.ts @@ -487,6 +487,10 @@ export class FrameManager implements PageDelegate { }); return to._createHandle(result.object) as dom.ElementHandle; } + + async getFrameOwner(frame: frames.Frame): Promise { + throw new Error('Not implemented'); + } } function toRemoteObject(handle: dom.ElementHandle): Protocol.Runtime.RemoteObject { diff --git a/test/click.spec.js b/test/click.spec.js index 4e26fb0bc9..9458d54047 100644 --- a/test/click.spec.js +++ b/test/click.spec.js @@ -261,8 +261,7 @@ module.exports.addTests = function({testRunner, expect, playwright, FFOX, CHROME await button.click(); expect(await frame.evaluate(() => window.result)).toBe('Clicked'); }); - // @see https://github.com/GoogleChrome/puppeteer/issues/4110 - xit('should click the button with fixed position inside an iframe', async({page, server}) => { + it.skip(WEBKIT || FFOX)('should click the button with fixed position inside an iframe', async({page, server}) => { await page.goto(server.EMPTY_PAGE); await page.setViewport({width: 500, height: 500}); await page.setContent('
spacer
'); diff --git a/test/utils.js b/test/utils.js index defb4f3265..dd714271f6 100644 --- a/test/utils.js +++ b/test/utils.js @@ -105,10 +105,6 @@ const utils = module.exports = { frame.src = url; frame.id = frameId; document.body.appendChild(frame); - // TODO(einbinder) do this right - // Access a scriptable global object to ensure JS context is - // initialized. WebKit will create it lazily only as need be. - frame.contentWindow; await new Promise(x => frame.onload = x); return frame; }