Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: correctly compute clickable points for elements inside OOPIFs #7900

Merged
merged 1 commit into from Jan 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion src/common/FrameManager.ts
Expand Up @@ -73,7 +73,6 @@ export class FrameManager extends EventEmitter {
private _contextIdToContext = new Map<string, ExecutionContext>();
private _isolatedWorlds = new Set<string>();
private _mainFrame: Frame;
private _disconnectPromise?: Promise<Error>;

constructor(
client: CDPSession,
Expand Down
33 changes: 32 additions & 1 deletion src/common/JSHandle.ts
Expand Up @@ -80,6 +80,7 @@ export function createJSHandle(
context,
context._client,
remoteObject,
frame,
frameManager.page(),
frameManager
);
Expand Down Expand Up @@ -331,6 +332,7 @@ export class JSHandle<HandleObjectType = unknown> {
export class ElementHandle<
ElementType extends Element = Element
> extends JSHandle<ElementType> {
private _frame: Frame;
private _page: Page;
private _frameManager: FrameManager;

Expand All @@ -341,12 +343,14 @@ export class ElementHandle<
context: ExecutionContext,
client: CDPSession,
remoteObject: Protocol.Runtime.RemoteObject,
frame: Frame,
page: Page,
frameManager: FrameManager
) {
super(context, client, remoteObject);
this._client = client;
this._remoteObject = remoteObject;
this._frame = frame;
this._page = page;
this._frameManager = frameManager;
}
Expand Down Expand Up @@ -475,16 +479,43 @@ export class ElementHandle<
objectId: this._remoteObject.objectId,
})
.catch(debugError),
this._client.send('Page.getLayoutMetrics'),
this._page.client().send('Page.getLayoutMetrics'),
]);
if (!result || !result.quads.length)
throw new Error('Node is either not clickable or not an HTMLElement');
// Filter out quads that have too small area to click into.
// Fallback to `layoutViewport` in case of using Firefox.
const { clientWidth, clientHeight } =
layoutMetrics.cssLayoutViewport || layoutMetrics.layoutViewport;
let offsetX = 0;
let offsetY = 0;
let frame = this._frame;
while (frame.parentFrame()) {
const parent = frame.parentFrame();
if (!frame.isOOPFrame()) {
frame = parent;
continue;
}
const { backendNodeId } = await parent._client.send('DOM.getFrameOwner', {
frameId: frame._id,
});
const { quads } = await parent._client.send('DOM.getContentQuads', {
backendNodeId: backendNodeId,
});
if (!quads || !quads.length) {
break;
}
const protocolQuads = quads.map((quad) => this._fromProtocolQuad(quad));
const topLeftCorner = protocolQuads[0][0];
offsetX += topLeftCorner.x;
offsetY += topLeftCorner.y;
frame = parent;
}
const quads = result.quads
.map((quad) => this._fromProtocolQuad(quad))
.map((quad) =>
quad.map((part) => ({ x: part.x + offsetX, y: part.y + offsetY }))
)
.map((quad) =>
this._intersectQuadWithViewport(quad, clientWidth, clientHeight)
)
Expand Down
43 changes: 42 additions & 1 deletion test/oopif.spec.ts
Expand Up @@ -210,10 +210,20 @@ describeChromeOnly('OOPIF', function () {
await frame.evaluate(() => {
const button = document.createElement('button');
button.id = 'test-button';
button.innerText = 'click';
button.onclick = () => {
button.id = 'clicked';
};
document.body.appendChild(button);
});

await page.evaluate(() => {
document.body.style.border = '150px solid black';
document.body.style.margin = '250px';
document.body.style.padding = '50px';
});
await frame.waitForSelector('#test-button', { visible: true });
await frame.click('#test-button');
await frame.waitForSelector('#clicked');
});
it('should report oopif frames', async () => {
const { server } = getTestState();
Expand Down Expand Up @@ -268,6 +278,37 @@ describeChromeOnly('OOPIF', function () {
await utils.detachFrame(oopIframe, 'frame1');
expect(oopIframe.childFrames()).toHaveLength(0);
});

it('clickablePoint should work for elements inside OOPIFs', async () => {
const { server } = getTestState();
await page.goto(server.EMPTY_PAGE);
const framePromise = page.waitForFrame((frame) => {
return page.frames().indexOf(frame) === 1;
});
await utils.attachFrame(
page,
'frame1',
server.CROSS_PROCESS_PREFIX + '/empty.html'
);
const frame = await framePromise;
await page.evaluate(() => {
document.body.style.border = '50px solid black';
document.body.style.margin = '50px';
document.body.style.padding = '50px';
});
await frame.evaluate(() => {
const button = document.createElement('button');
button.id = 'test-button';
button.innerText = 'click';
document.body.appendChild(button);
});
const button = await frame.waitForSelector('#test-button', {
visible: true,
});
const result = await button.clickablePoint();
expect(result.x).toBeGreaterThan(150); // padding + margin + border left
expect(result.y).toBeGreaterThan(150); // padding + margin + border top
});
});

/**
Expand Down