Skip to content

Commit

Permalink
feat(click): use browser-provided scrollIntoViewIfNeeded (#893)
Browse files Browse the repository at this point in the history
  • Loading branch information
@dgozman
dgozman committed Feb 11, 2020
1 parent 72b9cf0 commit c69dccf
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 75 deletions.
11 changes: 11 additions & 0 deletions src/chromium/crPage.ts
View file
Expand Up @@ -484,6 +484,17 @@ export class CRPage implements PageDelegate {
return {x, y, width, height};
}

async scrollRectIntoViewIfNeeded(handle: dom.ElementHandle, rect?: types.Rect): Promise<void> {
await this._client.send('DOM.scrollIntoViewIfNeeded', {
objectId: toRemoteObject(handle).objectId,
rect,
}).catch(e => {
if (e instanceof Error && e.message.includes('Node does not have a layout object'))
e.message = 'Node is either not visible or not an HTMLElement';
throw e;
});
}

async getContentQuads(handle: dom.ElementHandle): Promise<types.Quad[] | null> {
const result = await this._client.send('DOM.getContentQuads', {
objectId: toRemoteObject(handle).objectId
Expand Down
74 changes: 9 additions & 65 deletions src/dom.ts
View file
Expand Up @@ -160,58 +160,12 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
return this._page._delegate.getContentFrame(this);
}

async scrollIntoViewIfNeeded() {
const error = await this._evaluateInUtility(async (node: Node, pageJavascriptEnabled: boolean) => {
if (!node.isConnected)
return 'Node is detached from document';
if (node.nodeType !== Node.ELEMENT_NODE)
return 'Node is not of type HTMLElement';
const element = node as Element;
// force-scroll if page's javascript is disabled.
if (!pageJavascriptEnabled) {
// @ts-ignore because only Chromium still supports 'instant'
element.scrollIntoView({block: 'center', inline: 'center', behavior: 'instant'});
return false;
}
const visibleRatio = await new Promise(resolve => {
const observer = new IntersectionObserver(entries => {
resolve(entries[0].intersectionRatio);
observer.disconnect();
});
observer.observe(element);
// Firefox doesn't call IntersectionObserver callback unless
// there are rafs.
requestAnimationFrame(() => {});
});
if (visibleRatio !== 1.0) {
// @ts-ignore because only Chromium still supports 'instant'
element.scrollIntoView({block: 'center', inline: 'center', behavior: 'instant'});
}
return false;
}, !!this._page.context()._options.javaScriptEnabled);
if (error)
throw new Error(error);
async _scrollRectIntoViewIfNeeded(rect?: types.Rect): Promise<void> {
await this._page._delegate.scrollRectIntoViewIfNeeded(this, rect);
}

private async _ensurePointerActionPoint(relativePoint?: types.Point): Promise<types.Point> {
await this.scrollIntoViewIfNeeded();
if (!relativePoint)
return this._clickablePoint();
let r = await this._viewportPointAndScroll(relativePoint);
if (r.scrollX || r.scrollY) {
const error = await this._evaluateInUtility((element, scrollX, scrollY) => {
if (!element.ownerDocument || !element.ownerDocument.defaultView)
return 'Node does not have a containing window';
element.ownerDocument.defaultView.scrollBy(scrollX, scrollY);
return false;
}, r.scrollX, r.scrollY);
if (error)
throw new Error(error);
r = await this._viewportPointAndScroll(relativePoint);
if (r.scrollX || r.scrollY)
throw new Error('Failed to scroll relative point into viewport');
}
return r.point;
async scrollIntoViewIfNeeded() {
await this._scrollRectIntoViewIfNeeded();
}

private async _clickablePoint(): Promise<types.Point> {
Expand Down Expand Up @@ -253,7 +207,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
return result;
}

private async _viewportPointAndScroll(relativePoint: types.Point): Promise<{point: types.Point, scrollX: number, scrollY: number}> {
private async _relativePoint(relativePoint: types.Point): Promise<types.Point> {
const [box, border] = await Promise.all([
this.boundingBox(),
this._evaluateInUtility((node: Node) => {
Expand All @@ -273,23 +227,13 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
point.x += border.x;
point.y += border.y;
}
const metrics = await this._page._delegate.layoutViewport();
// Give 20 extra pixels to avoid any issues on viewport edge.
let scrollX = 0;
if (point.x < 20)
scrollX = point.x - 20;
if (point.x > metrics.width - 20)
scrollX = point.x - metrics.width + 20;
let scrollY = 0;
if (point.y < 20)
scrollY = point.y - 20;
if (point.y > metrics.height - 20)
scrollY = point.y - metrics.height + 20;
return { point, scrollX, scrollY };
return point;
}

async _performPointerAction(action: (point: types.Point) => Promise<void>, options?: input.PointerActionOptions): Promise<void> {
const point = await this._ensurePointerActionPoint(options ? options.relativePoint : undefined);
const relativePoint = options ? options.relativePoint : undefined;
await this._scrollRectIntoViewIfNeeded(relativePoint ? { x: relativePoint.x, y: relativePoint.y, width: 0, height: 0 } : undefined);
const point = relativePoint ? await this._relativePoint(relativePoint) : await this._clickablePoint();
let restoreModifiers: input.Modifier[] | undefined;
if (options && options.modifiers)
restoreModifiers = await this._page.keyboard._ensureModifiers(options.modifiers);
Expand Down
8 changes: 8 additions & 0 deletions src/firefox/ffPage.ts
View file
Expand Up @@ -417,6 +417,14 @@ export class FFPage implements PageDelegate {
return { x: minX, y: minY, width: maxX - minX, height: maxY - minY };
}

async scrollRectIntoViewIfNeeded(handle: dom.ElementHandle, rect?: types.Rect): Promise<void> {
await this._session.send('Page.scrollIntoViewIfNeeded', {
frameId: handle._context.frame._id,
objectId: toRemoteObject(handle).objectId!,
rect,
});
}

async getContentQuads(handle: dom.ElementHandle): Promise<types.Quad[] | null> {
const result = await this._session.send('Page.getContentQuads', {
frameId: handle._context.frame._id,
Expand Down
1 change: 1 addition & 0 deletions src/page.ts
View file
Expand Up @@ -69,6 +69,7 @@ export interface PageDelegate {
setInputFiles(handle: dom.ElementHandle<HTMLInputElement>, files: types.FilePayload[]): Promise<void>;
getBoundingBox(handle: dom.ElementHandle): Promise<types.Rect | null>;
getFrameElement(frame: frames.Frame): Promise<dom.ElementHandle>;
scrollRectIntoViewIfNeeded(handle: dom.ElementHandle, rect?: types.Rect): Promise<void>;

getAccessibilityTree(needle?: dom.ElementHandle): Promise<{tree: accessibility.AXNode, needle: accessibility.AXNode | null}>;
pdf?: (options?: types.PDFOptions) => Promise<platform.BufferType>;
Expand Down
11 changes: 11 additions & 0 deletions src/webkit/wkPage.ts
View file
Expand Up @@ -545,6 +545,17 @@ export class WKPage implements PageDelegate {
return { x: minX, y: minY, width: maxX - minX, height: maxY - minY };
}

async scrollRectIntoViewIfNeeded(handle: dom.ElementHandle, rect?: types.Rect): Promise<void> {
await this._session.send('DOM.scrollIntoViewIfNeeded', {
objectId: toRemoteObject(handle).objectId!,
rect,
}).catch(e => {
if (e instanceof Error && e.message.includes('Node does not have a layout object'))
e.message = 'Node is either not visible or not an HTMLElement';
throw e;
});
}

async getContentQuads(handle: dom.ElementHandle): Promise<types.Quad[] | null> {
const result = await this._session.send('DOM.getContentQuads', {
objectId: toRemoteObject(handle).objectId!
Expand Down
16 changes: 10 additions & 6 deletions test/click.spec.js
View file
Expand Up @@ -286,6 +286,8 @@ module.exports.describe = function({testRunner, expect, playwright, FFOX, CHROMI
expect(await frame.evaluate(() => window.result)).toBe('Clicked');
});
// @see https://github.com/GoogleChrome/puppeteer/issues/4110
// @see https://bugs.chromium.org/p/chromium/issues/detail?id=986390
// @see https://chromium-review.googlesource.com/c/chromium/src/+/1742784
xit('should click the button with fixed position inside an iframe', async({page, server}) => {
await page.goto(server.EMPTY_PAGE);
await page.setViewportSize({width: 500, height: 500});
Expand Down Expand Up @@ -326,7 +328,7 @@ module.exports.describe = function({testRunner, expect, playwright, FFOX, CHROMI
expect(await page.evaluate(() => offsetX)).toBe(WEBKIT ? 12 * 2 + 20 : 20);
expect(await page.evaluate(() => offsetY)).toBe(WEBKIT ? 12 * 2 + 10 : 10);
});
it('should click a very large button with relative point', async({page, server}) => {
it.skip(FFOX)('should click a very large button with relative point', async({page, server}) => {
await page.goto(server.PREFIX + '/input/button.html');
await page.$eval('button', button => button.style.borderWidth = '8px');
await page.$eval('button', button => button.style.height = button.style.width = '2000px');
Expand All @@ -336,7 +338,7 @@ module.exports.describe = function({testRunner, expect, playwright, FFOX, CHROMI
expect(await page.evaluate(() => offsetX)).toBe(WEBKIT ? 1900 + 8 : 1900);
expect(await page.evaluate(() => offsetY)).toBe(WEBKIT ? 1910 + 8 : 1910);
});
xit('should click a button in scrolling container with relative point', async({page, server}) => {
it.skip(FFOX)('should click a button in scrolling container with relative point', async({page, server}) => {
await page.goto(server.PREFIX + '/input/button.html');
await page.$eval('button', button => {
const container = document.createElement('div');
Expand All @@ -347,11 +349,13 @@ module.exports.describe = function({testRunner, expect, playwright, FFOX, CHROMI
container.appendChild(button);
button.style.height = '2000px';
button.style.width = '2000px';
button.style.borderWidth = '8px';
});
await page.click('button', { relativePoint: { x: 1900, y: 1910 } });
expect(await page.evaluate(() => window.result)).toBe('Clicked');
expect(await page.evaluate(() => offsetX)).toBe(1900);
expect(await page.evaluate(() => offsetY)).toBe(1910);
// Safari reports border-relative offsetX/offsetY.
expect(await page.evaluate(() => offsetX)).toBe(WEBKIT ? 1900 + 8 : 1900);
expect(await page.evaluate(() => offsetY)).toBe(WEBKIT ? 1910 + 8 : 1910);
});

it('should update modifiers correctly', async({page, server}) => {
Expand All @@ -370,7 +374,7 @@ module.exports.describe = function({testRunner, expect, playwright, FFOX, CHROMI
await page.click('button');
expect(await page.evaluate(() => shiftKey)).toBe(false);
});
it.skip(CHROMIUM)('should click an offscreen element when scroll-behavior is smooth', async({page}) => {
it('should click an offscreen element when scroll-behavior is smooth', async({page}) => {
await page.setContent(`
<div style="border: 1px solid black; height: 500px; overflow: auto; width: 500px; scroll-behavior: smooth">
<button style="margin-top: 2000px" onClick="window.clicked = true">hi</button>
Expand All @@ -379,7 +383,7 @@ module.exports.describe = function({testRunner, expect, playwright, FFOX, CHROMI
await page.click('button');
expect(await page.evaluate('window.clicked')).toBe(true);
});
it.skip(true)('should click on an animated button', async({page}) => {
xit('should click on an animated button', async({page}) => {
const buttonSize = 50;
const containerWidth = 500;
const transition = 500;
Expand Down
7 changes: 3 additions & 4 deletions test/elementhandle.spec.js
View file
Expand Up @@ -201,17 +201,16 @@ module.exports.describe = function({testRunner, expect, FFOX, CHROMIUM, WEBKIT})
it('should work for TextNodes', async({page, server}) => {
await page.goto(server.PREFIX + '/input/button.html');
const buttonTextNode = await page.evaluateHandle(() => document.querySelector('button').firstChild);
let error = null;
await buttonTextNode.click().catch(err => error = err);
expect(error.message).toBe('Node is not of type HTMLElement');
await buttonTextNode.click();
expect(await page.evaluate(() => result)).toBe('Clicked');
});
it('should throw for detached nodes', async({page, server}) => {
await page.goto(server.PREFIX + '/input/button.html');
const button = await page.$('button');
await page.evaluate(button => button.remove(), button);
let error = null;
await button.click().catch(err => error = err);
expect(error.message).toBe('Node is detached from document');
expect(error.message).toContain('Node is detached from document');
});
it('should throw for hidden nodes', async({page, server}) => {
await page.goto(server.PREFIX + '/input/button.html');
Expand Down

0 comments on commit c69dccf

Please sign in to comment.