From 690b1c2218f6ee83dcf02070c47753beaab54792 Mon Sep 17 00:00:00 2001 From: jrandolf <101637635+jrandolf@users.noreply.github.com> Date: Fri, 1 Sep 2023 17:13:29 +0200 Subject: [PATCH] chore: use `bindIsolatedHandle` (#10810) --- docs/api/puppeteer.elementhandle.aselement.md | 17 -- docs/api/puppeteer.elementhandle.md | 1 - .../puppeteer-core/src/api/ElementHandle.ts | 248 ++++++++++++------ .../src/common/ElementHandle.ts | 8 + .../src/common/IsolatedWorld.ts | 4 + .../src/common/bidi/ElementHandle.ts | 9 +- .../puppeteer-core/src/common/bidi/Sandbox.ts | 1 - .../src/common/bidi/Serializer.ts | 3 +- test/TestExpectations.json | 12 + test/src/ariaqueryhandler.spec.ts | 2 +- 10 files changed, 191 insertions(+), 114 deletions(-) delete mode 100644 docs/api/puppeteer.elementhandle.aselement.md diff --git a/docs/api/puppeteer.elementhandle.aselement.md b/docs/api/puppeteer.elementhandle.aselement.md deleted file mode 100644 index 17a9d8a4575..00000000000 --- a/docs/api/puppeteer.elementhandle.aselement.md +++ /dev/null @@ -1,17 +0,0 @@ ---- -sidebar_label: ElementHandle.asElement ---- - -# ElementHandle.asElement() method - -#### Signature: - -```typescript -class ElementHandle { - asElement(): ElementHandle; -} -``` - -**Returns:** - -[ElementHandle](./puppeteer.elementhandle.md)<ElementType> diff --git a/docs/api/puppeteer.elementhandle.md b/docs/api/puppeteer.elementhandle.md index e30a42bf300..495ec5142e8 100644 --- a/docs/api/puppeteer.elementhandle.md +++ b/docs/api/puppeteer.elementhandle.md @@ -54,7 +54,6 @@ The constructor for this class is marked as internal. Third-party code should no | [$$eval(selector, pageFunction, args)](./puppeteer.elementhandle.__eval.md) | |

Runs the given function on an array of elements matching the given selector in the current element.

If the given function returns a promise, then this method will wait till the promise resolves.

| | [$eval(selector, pageFunction, args)](./puppeteer.elementhandle._eval.md) | |

Runs the given function on the first element matching the given selector in the current element.

If the given function returns a promise, then this method will wait till the promise resolves.

| | [$x(expression)](./puppeteer.elementhandle._x.md) | | | -| [asElement()](./puppeteer.elementhandle.aselement.md) | | | | [autofill(data)](./puppeteer.elementhandle.autofill.md) | | If the element is a form input, you can use [ElementHandle.autofill()](./puppeteer.elementhandle.autofill.md) to test if the form is compatible with the browser's autofill implementation. Throws an error if the form cannot be autofilled. | | [boundingBox()](./puppeteer.elementhandle.boundingbox.md) | | This method returns the bounding box of the element (relative to the main frame), or null if the element is not visible. | | [boxModel()](./puppeteer.elementhandle.boxmodel.md) | | This method returns boxes of the element, or null if the element is not visible. | diff --git a/packages/puppeteer-core/src/api/ElementHandle.ts b/packages/puppeteer-core/src/api/ElementHandle.ts index 3ffdf0c1fe7..5e9ccd16043 100644 --- a/packages/puppeteer-core/src/api/ElementHandle.ts +++ b/packages/puppeteer-core/src/api/ElementHandle.ts @@ -139,6 +139,58 @@ export interface Point { export abstract class ElementHandle< ElementType extends Node = Element, > extends JSHandle { + /** + * A given method will have it's `this` replaced with an isolated version of + * `this` when decorated with this decorator. + * + * All changes of isolated `this` are reflected on the actual `this`. + * + * @internal + */ + static bindIsolatedHandle>( + target: (this: This, ...args: any[]) => Promise, + _: unknown + ): typeof target { + return async function (...args) { + // If the handle is already isolated, then we don't need to adopt it + // again. + if (this.realm === this.frame.isolatedRealm()) { + return await target.call(this, ...args); + } + using adoptedThis = await this.frame.isolatedRealm().adoptHandle(this); + const result = await target.call(adoptedThis, ...args); + // If the function returns `adoptedThis`, then we return `this`. + if (result === adoptedThis) { + return this; + } + // If the function returns a handle, transfer it into the current realm. + if (result instanceof JSHandle) { + return await this.realm.transferHandle(result); + } + // If the function returns an array of handlers, transfer them into the + // current realm. + if (Array.isArray(result)) { + await Promise.all( + result.map(async (item, index, result) => { + if (item instanceof JSHandle) { + result[index] = await this.realm.transferHandle(item); + } + }) + ); + } + if (result instanceof Map) { + await Promise.all( + [...result.entries()].map(async ([key, value]) => { + if (value instanceof JSHandle) { + result.set(key, await this.realm.transferHandle(value)); + } + }) + ); + } + return result; + }; + } + /** * @internal */ @@ -176,6 +228,10 @@ export abstract class ElementHandle< * @internal */ override async getProperty(propertyName: string): Promise>; + /** + * @internal + */ + @ElementHandle.bindIsolatedHandle override async getProperty( propertyName: HandleOr ): Promise> { @@ -185,6 +241,7 @@ export abstract class ElementHandle< /** * @internal */ + @ElementHandle.bindIsolatedHandle override async getProperties(): Promise> { return await this.handle.getProperties(); } @@ -224,6 +281,7 @@ export abstract class ElementHandle< /** * @internal */ + @ElementHandle.bindIsolatedHandle override async jsonValue(): Promise { return await this.handle.jsonValue(); } @@ -249,6 +307,9 @@ export abstract class ElementHandle< return this.handle.dispose(); } + /** + * @internal + */ override asElement(): ElementHandle { return this; } @@ -265,6 +326,7 @@ export abstract class ElementHandle< * @returns A {@link ElementHandle | element handle} to the first element * matching the given selector. Otherwise, `null`. */ + @ElementHandle.bindIsolatedHandle async $( selector: Selector ): Promise> | null> { @@ -283,6 +345,7 @@ export abstract class ElementHandle< * @returns An array of {@link ElementHandle | element handles} that point to * elements matching the given selector. */ + @ElementHandle.bindIsolatedHandle async $$( selector: Selector ): Promise>>> { @@ -415,6 +478,7 @@ export abstract class ElementHandle< * If there are no such elements, the method will resolve to an empty array. * @param expression - Expression to {@link https://developer.mozilla.org/en-US/docs/Web/API/Document/evaluate | evaluate} */ + @ElementHandle.bindIsolatedHandle async $x(expression: string): Promise>> { if (expression.startsWith('//')) { expression = `.${expression}`; @@ -459,6 +523,7 @@ export abstract class ElementHandle< * @returns An element matching the given selector. * @throws Throws if an element matching the given selector doesn't appear. */ + @ElementHandle.bindIsolatedHandle async waitForSelector( selector: Selector, options: WaitForSelectorOptions = {} @@ -473,15 +538,13 @@ export abstract class ElementHandle< } async #checkVisibility(visibility: boolean): Promise { - using element = await this.frame.isolatedRealm().adoptHandle(this); - return await this.frame.isolatedRealm().evaluate( - async (PuppeteerUtil, element, visibility) => { + return await this.evaluate( + async (element, PuppeteerUtil, visibility) => { return Boolean(PuppeteerUtil.checkVisibility(element, visibility)); }, LazyArg.create(context => { return context.puppeteerUtil; }), - element, visibility ); } @@ -490,6 +553,7 @@ export abstract class ElementHandle< * Checks if an element is visible using the same mechanism as * {@link ElementHandle.waitForSelector}. */ + @ElementHandle.bindIsolatedHandle async isVisible(): Promise { return await this.#checkVisibility(true); } @@ -498,6 +562,7 @@ export abstract class ElementHandle< * Checks if an element is hidden using the same mechanism as * {@link ElementHandle.waitForSelector}. */ + @ElementHandle.bindIsolatedHandle async isHidden(): Promise { return await this.#checkVisibility(false); } @@ -564,6 +629,7 @@ export abstract class ElementHandle< * default value can be changed by using the {@link Page.setDefaultTimeout} * method. */ + @ElementHandle.bindIsolatedHandle async waitForXPath( xpath: string, options: { @@ -596,6 +662,7 @@ export abstract class ElementHandle< * @throws An error if the handle does not match. **The handle will not be * automatically disposed.** */ + @ElementHandle.bindIsolatedHandle async toElement< K extends keyof HTMLElementTagNameMap | keyof SVGElementTagNameMap, >(tagName: K): Promise>> { @@ -618,9 +685,9 @@ export abstract class ElementHandle< /** * Returns the middle point within an element unless a specific offset is provided. */ + @ElementHandle.bindIsolatedHandle async clickablePoint(offset?: Offset): Promise { - using adoptedThis = await this.frame.isolatedRealm().adoptHandle(this); - const box = await adoptedThis.#clickableBox(); + const box = await this.#clickableBox(); if (!box) { throw new Error('Node is either not clickable or not an Element'); } @@ -641,6 +708,7 @@ export abstract class ElementHandle< * uses {@link Page} to hover over the center of the element. * If the element is detached from DOM, the method throws an error. */ + @ElementHandle.bindIsolatedHandle async hover(this: ElementHandle): Promise { await this.scrollIntoViewIfNeeded(); const {x, y} = await this.clickablePoint(); @@ -652,6 +720,7 @@ export abstract class ElementHandle< * uses {@link Page | Page.mouse} to click in the center of the element. * If the element is detached from DOM, the method throws an error. */ + @ElementHandle.bindIsolatedHandle async click( this: ElementHandle, options: Readonly = {} @@ -733,6 +802,7 @@ export abstract class ElementHandle< * `multiple` attribute, all values are considered, otherwise only the first * one is taken into account. */ + @ElementHandle.bindIsolatedHandle async select(...values: string[]): Promise { for (const value of values) { assert( @@ -801,6 +871,7 @@ export abstract class ElementHandle< * {@link Touchscreen.tap} to tap in the center of the element. * If the element is detached from DOM, the method throws an error. */ + @ElementHandle.bindIsolatedHandle async tap(this: ElementHandle): Promise { await this.scrollIntoViewIfNeeded(); const {x, y} = await this.clickablePoint(); @@ -808,18 +879,21 @@ export abstract class ElementHandle< await this.frame.page().touchscreen.touchEnd(); } + @ElementHandle.bindIsolatedHandle async touchStart(this: ElementHandle): Promise { await this.scrollIntoViewIfNeeded(); const {x, y} = await this.clickablePoint(); await this.frame.page().touchscreen.touchStart(x, y); } + @ElementHandle.bindIsolatedHandle async touchMove(this: ElementHandle): Promise { await this.scrollIntoViewIfNeeded(); const {x, y} = await this.clickablePoint(); await this.frame.page().touchscreen.touchMove(x, y); } + @ElementHandle.bindIsolatedHandle async touchEnd(this: ElementHandle): Promise { await this.scrollIntoViewIfNeeded(); await this.frame.page().touchscreen.touchEnd(); @@ -828,6 +902,7 @@ export abstract class ElementHandle< /** * Calls {@link https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/focus | focus} on the element. */ + @ElementHandle.bindIsolatedHandle async focus(): Promise { await this.evaluate(element => { if (!(element instanceof HTMLElement)) { @@ -862,6 +937,7 @@ export abstract class ElementHandle< * * @param options - Delay in milliseconds. Defaults to 0. */ + @ElementHandle.bindIsolatedHandle async type( text: string, options?: Readonly @@ -884,6 +960,7 @@ export abstract class ElementHandle< * @param key - Name of key to press, such as `ArrowLeft`. * See {@link KeyInput} for a list of all key names. */ + @ElementHandle.bindIsolatedHandle async press( key: KeyInput, options?: Readonly @@ -972,9 +1049,9 @@ export abstract class ElementHandle< * This method returns the bounding box of the element (relative to the main frame), * or `null` if the element is not visible. */ + @ElementHandle.bindIsolatedHandle async boundingBox(): Promise { - using adoptedThis = await this.frame.isolatedRealm().adoptHandle(this); - const box = await adoptedThis.evaluate(element => { + const box = await this.evaluate(element => { if (!(element instanceof Element)) { return null; } @@ -988,7 +1065,7 @@ export abstract class ElementHandle< if (!box) { return null; } - const offset = await adoptedThis.#getTopLeftCornerOfFrame(); + const offset = await this.#getTopLeftCornerOfFrame(); if (!offset) { return null; } @@ -1008,82 +1085,80 @@ export abstract class ElementHandle< * Boxes are represented as an array of points; * Each Point is an object `{x, y}`. Box points are sorted clock-wise. */ + @ElementHandle.bindIsolatedHandle async boxModel(): Promise { - const model = await (async () => { - using adoptedThis = await this.frame.isolatedRealm().adoptHandle(this); - return await adoptedThis.evaluate(element => { - if (!(element instanceof Element)) { - return null; - } - // Element is not visible. - if (element.getClientRects().length === 0) { - return null; - } - const rect = element.getBoundingClientRect(); - const style = window.getComputedStyle(element); - const offsets = { - padding: { - left: parseInt(style.paddingLeft, 10), - top: parseInt(style.paddingTop, 10), - right: parseInt(style.paddingRight, 10), - bottom: parseInt(style.paddingBottom, 10), - }, - margin: { - left: -parseInt(style.marginLeft, 10), - top: -parseInt(style.marginTop, 10), - right: -parseInt(style.marginRight, 10), - bottom: -parseInt(style.marginBottom, 10), - }, - border: { - left: parseInt(style.borderLeft, 10), - top: parseInt(style.borderTop, 10), - right: parseInt(style.borderRight, 10), - bottom: parseInt(style.borderBottom, 10), - }, - }; - const border: Quad = [ - {x: rect.left, y: rect.top}, - {x: rect.left + rect.width, y: rect.top}, - {x: rect.left + rect.width, y: rect.top + rect.bottom}, - {x: rect.left, y: rect.top + rect.bottom}, - ]; - const padding = transformQuadWithOffsets(border, offsets.border); - const content = transformQuadWithOffsets(padding, offsets.padding); - const margin = transformQuadWithOffsets(border, offsets.margin); - return { - content, - padding, - border, - margin, - width: rect.width, - height: rect.height, - }; + const model = await this.evaluate(element => { + if (!(element instanceof Element)) { + return null; + } + // Element is not visible. + if (element.getClientRects().length === 0) { + return null; + } + const rect = element.getBoundingClientRect(); + const style = window.getComputedStyle(element); + const offsets = { + padding: { + left: parseInt(style.paddingLeft, 10), + top: parseInt(style.paddingTop, 10), + right: parseInt(style.paddingRight, 10), + bottom: parseInt(style.paddingBottom, 10), + }, + margin: { + left: -parseInt(style.marginLeft, 10), + top: -parseInt(style.marginTop, 10), + right: -parseInt(style.marginRight, 10), + bottom: -parseInt(style.marginBottom, 10), + }, + border: { + left: parseInt(style.borderLeft, 10), + top: parseInt(style.borderTop, 10), + right: parseInt(style.borderRight, 10), + bottom: parseInt(style.borderBottom, 10), + }, + }; + const border: Quad = [ + {x: rect.left, y: rect.top}, + {x: rect.left + rect.width, y: rect.top}, + {x: rect.left + rect.width, y: rect.top + rect.bottom}, + {x: rect.left, y: rect.top + rect.bottom}, + ]; + const padding = transformQuadWithOffsets(border, offsets.border); + const content = transformQuadWithOffsets(padding, offsets.padding); + const margin = transformQuadWithOffsets(border, offsets.margin); + return { + content, + padding, + border, + margin, + width: rect.width, + height: rect.height, + }; - function transformQuadWithOffsets( - quad: Quad, - offsets: {top: number; left: number; right: number; bottom: number} - ): Quad { - return [ - { - x: quad[0].x + offsets.left, - y: quad[0].y + offsets.top, - }, - { - x: quad[1].x - offsets.right, - y: quad[1].y + offsets.top, - }, - { - x: quad[2].x - offsets.right, - y: quad[2].y - offsets.bottom, - }, - { - x: quad[3].x + offsets.left, - y: quad[3].y - offsets.bottom, - }, - ]; - } - }); - })(); + function transformQuadWithOffsets( + quad: Quad, + offsets: {top: number; left: number; right: number; bottom: number} + ): Quad { + return [ + { + x: quad[0].x + offsets.left, + y: quad[0].y + offsets.top, + }, + { + x: quad[1].x - offsets.right, + y: quad[1].y + offsets.top, + }, + { + x: quad[2].x - offsets.right, + y: quad[2].y - offsets.bottom, + }, + { + x: quad[3].x + offsets.left, + y: quad[3].y - offsets.bottom, + }, + ]; + } + }); if (!model) { return null; } @@ -1198,6 +1273,7 @@ export abstract class ElementHandle< * @param options - Threshold for the intersection between 0 (no intersection) and 1 * (full intersection). Defaults to 1. */ + @ElementHandle.bindIsolatedHandle async isIntersectingViewport( this: ElementHandle, options: { @@ -1227,10 +1303,10 @@ export abstract class ElementHandle< * Scrolls the element into view using either the automation protocol client * or by calling element.scrollIntoView. */ + @ElementHandle.bindIsolatedHandle async scrollIntoView(this: ElementHandle): Promise { - using adoptedThis = await this.frame.isolatedRealm().adoptHandle(this); - await adoptedThis.assertConnectedElement(); - await adoptedThis.evaluate(async (element): Promise => { + await this.assertConnectedElement(); + await this.evaluate(async (element): Promise => { element.scrollIntoView({ block: 'center', inline: 'center', diff --git a/packages/puppeteer-core/src/common/ElementHandle.ts b/packages/puppeteer-core/src/common/ElementHandle.ts index 10b273bd583..e4fe16cc9b4 100644 --- a/packages/puppeteer-core/src/common/ElementHandle.ts +++ b/packages/puppeteer-core/src/common/ElementHandle.ts @@ -87,6 +87,7 @@ export class CDPElementHandle< } @throwIfDisposed() + @ElementHandle.bindIsolatedHandle override async scrollIntoView( this: CDPElementHandle ): Promise { @@ -106,6 +107,7 @@ export class CDPElementHandle< * This method creates and captures a dragevent from the element. */ @throwIfDisposed() + @ElementHandle.bindIsolatedHandle override async drag( this: CDPElementHandle, target: Point @@ -120,6 +122,7 @@ export class CDPElementHandle< } @throwIfDisposed() + @ElementHandle.bindIsolatedHandle override async dragEnter( this: CDPElementHandle, data: Protocol.Input.DragData = {items: [], dragOperationsMask: 1} @@ -130,6 +133,7 @@ export class CDPElementHandle< } @throwIfDisposed() + @ElementHandle.bindIsolatedHandle override async dragOver( this: CDPElementHandle, data: Protocol.Input.DragData = {items: [], dragOperationsMask: 1} @@ -140,6 +144,7 @@ export class CDPElementHandle< } @throwIfDisposed() + @ElementHandle.bindIsolatedHandle override async drop( this: CDPElementHandle, data: Protocol.Input.DragData = {items: [], dragOperationsMask: 1} @@ -150,6 +155,7 @@ export class CDPElementHandle< } @throwIfDisposed() + @ElementHandle.bindIsolatedHandle override async dragAndDrop( this: CDPElementHandle, target: CDPElementHandle, @@ -166,6 +172,7 @@ export class CDPElementHandle< } @throwIfDisposed() + @ElementHandle.bindIsolatedHandle override async uploadFile( this: CDPElementHandle, ...filePaths: string[] @@ -224,6 +231,7 @@ export class CDPElementHandle< } @throwIfDisposed() + @ElementHandle.bindIsolatedHandle override async screenshot( this: CDPElementHandle, options: ScreenshotOptions = {} diff --git a/packages/puppeteer-core/src/common/IsolatedWorld.ts b/packages/puppeteer-core/src/common/IsolatedWorld.ts index 18de2bd29d1..52b66b3e5f8 100644 --- a/packages/puppeteer-core/src/common/IsolatedWorld.ts +++ b/packages/puppeteer-core/src/common/IsolatedWorld.ts @@ -328,6 +328,10 @@ export class IsolatedWorld extends Realm { if (handle.realm === this) { return handle; } + // Implies it's a primitive value, probably. + if (handle.remoteObject().objectId === undefined) { + return handle; + } const info = await this.client.send('DOM.describeNode', { objectId: handle.remoteObject().objectId, }); diff --git a/packages/puppeteer-core/src/common/bidi/ElementHandle.ts b/packages/puppeteer-core/src/common/bidi/ElementHandle.ts index 8c08686aa76..ac2627b311f 100644 --- a/packages/puppeteer-core/src/common/bidi/ElementHandle.ts +++ b/packages/puppeteer-core/src/common/bidi/ElementHandle.ts @@ -55,11 +55,6 @@ export class BidiElementHandle< return this.handle.remoteValue(); } - assertElementHasWorld(): asserts this { - // TODO: Should assert element has a Sandbox - return; - } - override async autofill(data: AutofillData): Promise { const client = this.frame.client; const nodeInfo = await client.send('DOM.describeNode', { @@ -77,9 +72,9 @@ export class BidiElementHandle< override async contentFrame( this: BidiElementHandle ): Promise; + @ElementHandle.bindIsolatedHandle override async contentFrame(): Promise { - using adoptedThis = await this.frame.isolatedRealm().adoptHandle(this); - using handle = (await adoptedThis.evaluateHandle(element => { + using handle = (await this.evaluateHandle(element => { if (element instanceof HTMLIFrameElement) { return element.contentWindow; } diff --git a/packages/puppeteer-core/src/common/bidi/Sandbox.ts b/packages/puppeteer-core/src/common/bidi/Sandbox.ts index f98fc510f2a..efa0ace1539 100644 --- a/packages/puppeteer-core/src/common/bidi/Sandbox.ts +++ b/packages/puppeteer-core/src/common/bidi/Sandbox.ts @@ -114,7 +114,6 @@ export class Sandbox extends Realm { const transferredHandle = await this.evaluateHandle(node => { return node; }, handle); - await handle.dispose(); return transferredHandle as unknown as T; } diff --git a/packages/puppeteer-core/src/common/bidi/Serializer.ts b/packages/puppeteer-core/src/common/bidi/Serializer.ts index 6e03e09eda3..ee10d5fc05b 100644 --- a/packages/puppeteer-core/src/common/bidi/Serializer.ts +++ b/packages/puppeteer-core/src/common/bidi/Serializer.ts @@ -156,7 +156,8 @@ export class BidiSerializer { : null; if (objectHandle) { if ( - objectHandle.realm !== sandbox && + objectHandle.realm.environment.context() !== + sandbox.environment.context() && !('sharedId' in objectHandle.remoteValue()) ) { throw new Error( diff --git a/test/TestExpectations.json b/test/TestExpectations.json index b8c78dcbd9a..c657ca8115c 100644 --- a/test/TestExpectations.json +++ b/test/TestExpectations.json @@ -2111,6 +2111,18 @@ "parameters": ["firefox", "webDriverBiDi"], "expectations": ["FAIL"] }, + { + "testIdPattern": "[elementhandle.spec] ElementHandle specs ElementHandle.isIntersectingViewport should work", + "platforms": ["darwin", "linux", "win32"], + "parameters": ["cdp", "firefox"], + "expectations": ["FAIL"] + }, + { + "testIdPattern": "[elementhandle.spec] ElementHandle specs ElementHandle.isIntersectingViewport should work with svg elements", + "platforms": ["darwin", "linux", "win32"], + "parameters": ["cdp", "firefox"], + "expectations": ["FAIL"] + }, { "testIdPattern": "[emulation.spec] Emulation Page.emulate should support clicking", "platforms": ["darwin", "linux", "win32"], diff --git a/test/src/ariaqueryhandler.spec.ts b/test/src/ariaqueryhandler.spec.ts index fbdc96eea4f..25161563122 100644 --- a/test/src/ariaqueryhandler.spec.ts +++ b/test/src/ariaqueryhandler.spec.ts @@ -208,7 +208,7 @@ describe('AriaQueryHandler', () => { }); describe('queryAllArray', () => { it('$$eval should handle many elements', async function () { - this.timeout(25_000); + this.timeout(40_000); const {page} = await getTestState(); await page.setContent('');