Skip to content

Commit

Permalink
fix(webkit): click moving targets on windows (#2101)
Browse files Browse the repository at this point in the history
  • Loading branch information
pavelfeldman authored May 4, 2020
1 parent 7e9a8dd commit f6210ae
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 8 deletions.
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

0 comments on commit f6210ae

Please sign in to comment.