Skip to content

Commit

Permalink
fix(click): wait for button, input and select to be enabled before cl…
Browse files Browse the repository at this point in the history
…icking (#2414)
  • Loading branch information
dgozman committed May 30, 2020
1 parent fdd8df6 commit acf059f
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 29 deletions.
10 changes: 5 additions & 5 deletions src/dom.ts
Expand Up @@ -248,7 +248,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
async _performPointerAction(actionName: string, action: (point: types.Point) => Promise<void>, deadline: number, options: PointerActionOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions = {}): Promise<'done' | 'retry'> {
const { force = false, position } = options;
if (!force)
await this._waitForDisplayedAtStablePosition(deadline);
await this._waitForDisplayedAtStablePositionAndEnabled(deadline);

let paused = false;
try {
Expand Down Expand Up @@ -489,16 +489,16 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
return result;
}

async _waitForDisplayedAtStablePosition(deadline: number): Promise<void> {
this._page._log(inputLog, 'waiting for element to be displayed and not moving...');
async _waitForDisplayedAtStablePositionAndEnabled(deadline: number): Promise<void> {
this._page._log(inputLog, 'waiting for element to be displayed, enabled and not moving...');
const rafCount = this._page._delegate.rafCountForStablePosition();
const stablePromise = this._evaluateInUtility(({ injected, node }, { rafCount, timeout }) => {
return injected.waitForDisplayedAtStablePosition(node, rafCount, timeout);
return injected.waitForDisplayedAtStablePositionAndEnabled(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, 'pw:input');
handleInjectedResult(injectedResult, timeoutMessage);
this._page._log(inputLog, '...element is displayed and does not move');
this._page._log(inputLog, '...element is displayed, enabled and does not move');
}

async _checkHitTargetAt(point: types.Point): Promise<boolean> {
Expand Down
15 changes: 10 additions & 5 deletions src/injected/injectedScript.ts
Expand Up @@ -330,7 +330,7 @@ export default class InjectedScript {
input.dispatchEvent(new Event('change', { 'bubbles': true }));
}

async waitForDisplayedAtStablePosition(node: Node, rafCount: number, timeout: number): Promise<types.InjectedScriptResult> {
async waitForDisplayedAtStablePositionAndEnabled(node: Node, rafCount: number, timeout: number): Promise<types.InjectedScriptResult> {
if (!node.isConnected)
return { status: 'notconnected' };
const element = node.nodeType === Node.ELEMENT_NODE ? (node as Element) : node.parentElement;
Expand Down Expand Up @@ -360,15 +360,20 @@ export default class InjectedScript {
const clientRect = element.getBoundingClientRect();
const rect = { x: clientRect.top, y: clientRect.left, width: clientRect.width, height: clientRect.height };
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;
lastRect = rect;
if (samePosition)
++samePositionCounter;
else
samePositionCounter = 0;
let isDisplayedAndStable = samePositionCounter >= rafCount;
const isDisplayedAndStable = samePositionCounter >= rafCount;

const style = element.ownerDocument && element.ownerDocument.defaultView ? element.ownerDocument.defaultView.getComputedStyle(element) : undefined;
isDisplayedAndStable = isDisplayedAndStable && (!!style && style.visibility !== 'hidden');
lastRect = rect;
return !!isDisplayedAndStable;
const isVisible = !!style && style.visibility !== 'hidden';

const elementOrButton = element.closest('button, [role=button]') || element;
const isDisabled = ['BUTTON', 'INPUT', 'SELECT'].includes(elementOrButton.nodeName) && elementOrButton.hasAttribute('disabled');

return isDisplayedAndStable && isVisible && !isDisabled;
});
return { status: result === 'notconnected' ? 'notconnected' : (result ? 'success' : 'timeout') };
}
Expand Down
72 changes: 53 additions & 19 deletions test/click.spec.js
Expand Up @@ -18,6 +18,11 @@
const utils = require('./utils');
const {FFOX, CHROMIUM, WEBKIT, WIN} = utils.testOptions(browserType);

async function giveItAChanceToClick(page) {
for (let i = 0; i < 5; i++)
await page.evaluate(() => new Promise(f => requestAnimationFrame(() => requestAnimationFrame(f))));
}

describe('Page.click', function() {
it('should click the button', async({page, server}) => {
await page.goto(server.PREFIX + '/input/button.html');
Expand Down Expand Up @@ -151,10 +156,7 @@ describe('Page.click', function() {
await page.goto(server.PREFIX + '/input/button.html');
await page.$eval('button', b => b.style.display = 'none');
const clicked = page.click('button', { timeout: 0 }).then(() => done = true);
for (let i = 0; i < 10; i++) {
// Do enough double rafs to check for possible races.
await page.evaluate(() => new Promise(f => requestAnimationFrame(() => requestAnimationFrame(f))));
}
await giveItAChanceToClick(page);
expect(await page.evaluate(() => result)).toBe('Was not clicked');
expect(done).toBe(false);
await page.$eval('button', b => b.style.display = 'block');
Expand All @@ -167,10 +169,7 @@ describe('Page.click', function() {
await page.goto(server.PREFIX + '/input/button.html');
await page.$eval('button', b => b.style.visibility = 'hidden');
const clicked = page.click('button', { timeout: 0 }).then(() => done = true);
for (let i = 0; i < 10; i++) {
// Do enough double rafs to check for possible races.
await page.evaluate(() => new Promise(f => requestAnimationFrame(() => requestAnimationFrame(f))));
}
await giveItAChanceToClick(page);
expect(await page.evaluate(() => result)).toBe('Was not clicked');
expect(done).toBe(false);
await page.$eval('button', b => b.style.visibility = 'visible');
Expand All @@ -197,10 +196,7 @@ describe('Page.click', function() {
await page.goto(server.PREFIX + '/input/button.html');
await page.$eval('button', b => b.parentElement.style.display = 'none');
const clicked = page.click('button', { timeout: 0 }).then(() => done = true);
for (let i = 0; i < 10; i++) {
// Do enough double rafs to check for possible races.
await page.evaluate(() => new Promise(f => requestAnimationFrame(() => requestAnimationFrame(f))));
}
await giveItAChanceToClick(page);
expect(done).toBe(false);
await page.$eval('button', b => b.parentElement.style.display = 'block');
await clicked;
Expand Down Expand Up @@ -460,8 +456,7 @@ describe('Page.click', function() {
expect(clicked).toBe(false);

await page.$eval('.flyover', flyOver => flyOver.style.left = '0');
await page.evaluate(() => new Promise(requestAnimationFrame));
await page.evaluate(() => new Promise(requestAnimationFrame));
await giveItAChanceToClick(page);
expect(clicked).toBe(false);

await page.$eval('.flyover', flyOver => flyOver.style.left = '200px');
Expand Down Expand Up @@ -503,6 +498,45 @@ describe('Page.click', function() {
expect(await page.evaluate(() => window.result)).toBe('Was not clicked');
});

it('should wait for button to be enabled', async({page, server}) => {
await page.setContent('<button onclick="javascript:window.__CLICKED=true;" disabled><span>Click target</span></button>');
let done = false;
const clickPromise = page.click('text=Click target').then(() => done = true);
await giveItAChanceToClick(page);
expect(await page.evaluate(() => window.__CLICKED)).toBe(undefined);
expect(done).toBe(false);
await page.evaluate(() => document.querySelector('button').removeAttribute('disabled'));
await clickPromise;
expect(await page.evaluate(() => window.__CLICKED)).toBe(true);
});
it('should wait for input to be enabled', async({page, server}) => {
await page.setContent('<input onclick="javascript:window.__CLICKED=true;" disabled>');
let done = false;
const clickPromise = page.click('input').then(() => done = true);
await giveItAChanceToClick(page);
expect(await page.evaluate(() => window.__CLICKED)).toBe(undefined);
expect(done).toBe(false);
await page.evaluate(() => document.querySelector('input').removeAttribute('disabled'));
await clickPromise;
expect(await page.evaluate(() => window.__CLICKED)).toBe(true);
});
it('should wait for select to be enabled', async({page, server}) => {
await page.setContent('<select onclick="javascript:window.__CLICKED=true;" disabled><option selected>Hello</option></select>');
let done = false;
const clickPromise = page.click('select').then(() => done = true);
await giveItAChanceToClick(page);
expect(await page.evaluate(() => window.__CLICKED)).toBe(undefined);
expect(done).toBe(false);
await page.evaluate(() => document.querySelector('select').removeAttribute('disabled'));
await clickPromise;
expect(await page.evaluate(() => window.__CLICKED)).toBe(true);
});
it('should click disabled div', async({page, server}) => {
await page.setContent('<div onclick="javascript:window.__CLICKED=true;" disabled>Click target</div>');
await page.click('text=Click target');
expect(await page.evaluate(() => window.__CLICKED)).toBe(true);
});

it('should climb dom for inner label with pointer-events:none', async({page, server}) => {
await page.setContent('<button onclick="javascript:window.__CLICKED=true;"><label style="pointer-events:none">Click target</label></button>');
await page.click('text=Click target');
Expand All @@ -515,11 +549,11 @@ describe('Page.click', function() {
});
it('should wait for BUTTON to be clickable when it has pointer-events:none', async({page, server}) => {
await page.setContent('<button onclick="javascript:window.__CLICKED=true;" style="pointer-events:none"><span>Click target</span></button>');
const clickPromise = page.click('text=Click target');
// Do a few roundtrips to the page.
for (let i = 0; i < 5; ++i)
expect(await page.evaluate(() => window.__CLICKED)).toBe(undefined);
// remove `pointer-events: none` css from button.
let done = false;
const clickPromise = page.click('text=Click target').then(() => done = true);
await giveItAChanceToClick(page);
expect(await page.evaluate(() => window.__CLICKED)).toBe(undefined);
expect(done).toBe(false);
await page.evaluate(() => document.querySelector('button').style.removeProperty('pointer-events'));
await clickPromise;
expect(await page.evaluate(() => window.__CLICKED)).toBe(true);
Expand Down

0 comments on commit acf059f

Please sign in to comment.