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(webkit): click moving targets on windows #2101

Merged
merged 1 commit into from
May 4, 2020
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions src/chromium/crPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,10 @@ export class CRPage implements PageDelegate {
await this._forAllFrameSessions(frame => frame._setActivityPaused(paused));
}

rafCountForStablePosition(): number {
return 1;
}

async getContentQuads(handle: dom.ElementHandle): Promise<types.Quad[] | null> {
return this._sessionForHandle(handle)._getContentQuads(handle);
}
Expand Down
7 changes: 4 additions & 3 deletions src/dom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -455,9 +455,10 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {

async _waitForDisplayedAtStablePosition(deadline: number): Promise<void> {
this._page._log(inputLog, 'waiting for element to be displayed and not moving...');
const stablePromise = this._evaluateInUtility(({ injected, node }, timeout) => {
return injected.waitForDisplayedAtStablePosition(node, timeout);
}, helper.timeUntilDeadline(deadline));
const rafCount = this._page._delegate.rafCountForStablePosition();
const stablePromise = this._evaluateInUtility(({ injected, node }, { rafCount, timeout }) => {
return injected.waitForDisplayedAtStablePosition(node, rafCount, timeout);
}, { rafCount, timeout: helper.timeUntilDeadline(deadline) });
const timeoutMessage = 'element to be displayed and not moving';
const injectedResult = await helper.waitWithDeadline(stablePromise, timeoutMessage, deadline);
handleInjectedResult(injectedResult, timeoutMessage);
Expand Down
4 changes: 4 additions & 0 deletions src/firefox/ffPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,10 @@ export class FFPage implements PageDelegate {
async setActivityPaused(paused: boolean): Promise<void> {
}

rafCountForStablePosition(): number {
return 1;
}

async getContentQuads(handle: dom.ElementHandle): Promise<types.Quad[] | null> {
const result = await this._session.send('Page.getContentQuads', {
frameId: handle._context.frame._id,
Expand Down
18 changes: 16 additions & 2 deletions src/injected/injected.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ export class Injected {
input.dispatchEvent(new Event('change', { 'bubbles': true }));
}

async waitForDisplayedAtStablePosition(node: Node, timeout: number): Promise<InjectedResult> {
async waitForDisplayedAtStablePosition(node: Node, rafCount: number, timeout: number): Promise<InjectedResult> {
if (!node.isConnected)
return { status: 'notconnected' };
const element = node.nodeType === Node.ELEMENT_NODE ? (node as Element) : node.parentElement;
Expand All @@ -271,6 +271,8 @@ export class Injected {

let lastRect: types.Rect | undefined;
let counter = 0;
let samePositionCounter = 0;
let lastTime = 0;
const result = await this.poll('raf', timeout, (): 'notconnected' | boolean => {
// First raf happens in the same animation frame as evaluation, so it does not produce
// any client rect difference compared to synchronous call. We skip the synchronous call
Expand All @@ -279,10 +281,22 @@ export class Injected {
return false;
if (!node.isConnected)
return 'notconnected';

// Drop frames that are shorter than 16ms - WebKit Win bug.
const time = performance.now();
if (rafCount > 1 && time - lastTime < 15)
return false;
lastTime = time;

// Note: this logic should be similar to isVisible() to avoid surprises.
const clientRect = element.getBoundingClientRect();
const rect = { x: clientRect.top, y: clientRect.left, width: clientRect.width, height: clientRect.height };
let isDisplayedAndStable = lastRect && rect.x === lastRect.x && rect.y === lastRect.y && rect.width === lastRect.width && rect.height === lastRect.height && rect.width > 0 && rect.height > 0;
const samePosition = lastRect && rect.x === lastRect.x && rect.y === lastRect.y && rect.width === lastRect.width && rect.height === lastRect.height && rect.width > 0 && rect.height > 0;
if (samePosition)
++samePositionCounter;
else
samePositionCounter = 0;
let isDisplayedAndStable = samePositionCounter >= rafCount;
const style = element.ownerDocument && element.ownerDocument.defaultView ? element.ownerDocument.defaultView.getComputedStyle(element) : undefined;
isDisplayedAndStable = isDisplayedAndStable && (!!style && style.visibility !== 'hidden');
lastRect = rect;
Expand Down
1 change: 1 addition & 0 deletions src/page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export interface PageDelegate {
getFrameElement(frame: frames.Frame): Promise<dom.ElementHandle>;
scrollRectIntoViewIfNeeded(handle: dom.ElementHandle, rect?: types.Rect): Promise<void>;
setActivityPaused(paused: boolean): Promise<void>;
rafCountForStablePosition(): number;

getAccessibilityTree(needle?: dom.ElementHandle): Promise<{tree: accessibility.AXNode, needle: accessibility.AXNode | null}>;
pdf?: (options?: types.PDFOptions) => Promise<Buffer>;
Expand Down
4 changes: 4 additions & 0 deletions src/webkit/wkPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,10 @@ export class WKPage implements PageDelegate {
async setActivityPaused(paused: boolean): Promise<void> {
}

rafCountForStablePosition(): number {
return process.platform === 'win32' ? 5 : 1;
}

async getContentQuads(handle: dom.ElementHandle): Promise<types.Quad[] | null> {
const result = await this._session.send('DOM.getContentQuads', {
objectId: toRemoteObject(handle).objectId!
Expand Down
4 changes: 2 additions & 2 deletions test/click.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ describe('Page.click', function() {
await context.close();
});

it.fail(WEBKIT && WIN)('should wait for stable position', async({page, server}) => {
it('should wait for stable position', async({page, server}) => {
await page.goto(server.PREFIX + '/input/button.html');
await page.$eval('button', button => {
button.style.transition = 'margin 500ms linear 0s';
Expand All @@ -424,7 +424,7 @@ describe('Page.click', function() {
expect(await page.evaluate(() => pageX)).toBe(300);
expect(await page.evaluate(() => pageY)).toBe(10);
});
it.fail(WEBKIT && WIN)('should timeout waiting for stable position', async({page, server}) => {
it('should timeout waiting for stable position', async({page, server}) => {
await page.goto(server.PREFIX + '/input/button.html');
const button = await page.$('button');
await button.evaluate(button => {
Expand Down