From a65aa0d7a07a96c91851f0b542f88027d8b81ead Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Tue, 2 Mar 2021 11:30:31 -0800 Subject: [PATCH] fix(click): do not retarget from label to control when clicking And in other carefully considered cases. --- src/server/injected/injectedScript.ts | 43 +++++++++++++------------- test/elementhandle-convenience.spec.ts | 16 ++++++++++ test/page-click.spec.ts | 11 +++++++ 3 files changed, 49 insertions(+), 21 deletions(-) diff --git a/src/server/injected/injectedScript.ts b/src/server/injected/injectedScript.ts index 1b2cfe35b9ea7..1400cb982596f 100644 --- a/src/server/injected/injectedScript.ts +++ b/src/server/injected/injectedScript.ts @@ -326,34 +326,34 @@ export class InjectedScript { return { left: parseInt(style.borderLeftWidth || '', 10), top: parseInt(style.borderTopWidth || '', 10) }; } - private _retarget(node: Node): Element | null { + private _retarget(node: Node, behavior: 'follow-label' | 'no-follow-label'): Element | null { let element = node.nodeType === Node.ELEMENT_NODE ? node as Element : node.parentElement; if (!element) return null; element = element.closest('button, [role=button], [role=checkbox], [role=radio]') || element; - if (!element.matches('input, textarea, button, select, [role=button], [role=checkbox], [role=radio]') && - !(element as any).isContentEditable) { - // Go up to the label that might be connected to the input/textarea. - element = element.closest('label') || element; + if (behavior === 'follow-label') { + if (!element.matches('input, textarea, button, select, [role=button], [role=checkbox], [role=radio]') && + !(element as any).isContentEditable) { + // Go up to the label that might be connected to the input/textarea. + element = element.closest('label') || element; + } + if (element.nodeName === 'LABEL') + element = (element as HTMLLabelElement).control || element; } - if (element.nodeName === 'LABEL') - element = (element as HTMLLabelElement).control || element; return element; } waitForElementStatesAndPerformAction(node: Node, states: ElementState[], - callback: (element: Element | null, progress: InjectedScriptProgress, continuePolling: symbol) => T | symbol): InjectedScriptPoll { + callback: (node: Node, progress: InjectedScriptProgress, continuePolling: symbol) => T | symbol): InjectedScriptPoll { let lastRect: { x: number, y: number, width: number, height: number } | undefined; let counter = 0; let samePositionCounter = 0; let lastTime = 0; const predicate = (progress: InjectedScriptProgress, continuePolling: symbol) => { - const element = this._retarget(node); - for (const state of states) { if (state !== 'stable') { - const result = this._checkElementState(element, state); + const result = this.checkElementState(node, state); if (typeof result !== 'boolean') return result; if (!result) { @@ -363,6 +363,7 @@ export class InjectedScript { continue; } + const element = this._retarget(node, 'no-follow-label'); if (!element) return 'error:notconnected'; @@ -394,7 +395,7 @@ export class InjectedScript { return continuePolling; } - return callback(element, progress, continuePolling); + return callback(node, progress, continuePolling); }; if (this._replaceRafWithTimeout) @@ -403,12 +404,14 @@ export class InjectedScript { return this.pollRaf(predicate); } - private _checkElementState(element: Element | null, state: ElementStateWithoutStable): boolean | 'error:notconnected' | FatalDOMError { + checkElementState(node: Node, state: ElementStateWithoutStable): boolean | 'error:notconnected' | FatalDOMError { + const element = this._retarget(node, ['stable', 'visible', 'hidden'].includes(state) ? 'no-follow-label' : 'follow-label'); if (!element || !element.isConnected) { if (state === 'hidden') return true; return 'error:notconnected'; } + if (state === 'visible') return this.isVisible(element); if (state === 'hidden') @@ -436,13 +439,9 @@ export class InjectedScript { throw new Error(`Unexpected element state "${state}"`); } - checkElementState(node: Node, state: ElementStateWithoutStable): boolean | 'error:notconnected' | FatalDOMError { - const element = this._retarget(node); - return this._checkElementState(element, state); - } - selectOptions(optionsToSelect: (Node | { value?: string, label?: string, index?: number })[], - element: Element | null, progress: InjectedScriptProgress, continuePolling: symbol): string[] | 'error:notconnected' | FatalDOMError | symbol { + node: Node, progress: InjectedScriptProgress, continuePolling: symbol): string[] | 'error:notconnected' | FatalDOMError | symbol { + const element = this._retarget(node, 'follow-label'); if (!element) return 'error:notconnected'; if (element.nodeName.toLowerCase() !== 'select') @@ -487,7 +486,8 @@ export class InjectedScript { return selectedOptions.map(option => option.value); } - fill(value: string, element: Element | null, progress: InjectedScriptProgress): FatalDOMError | 'error:notconnected' | 'needsinput' | 'done' { + fill(value: string, node: Node, progress: InjectedScriptProgress): FatalDOMError | 'error:notconnected' | 'needsinput' | 'done' { + const element = this._retarget(node, 'follow-label'); if (!element) return 'error:notconnected'; if (element.nodeName.toLowerCase() === 'input') { @@ -523,7 +523,8 @@ export class InjectedScript { return 'needsinput'; // Still need to input the value. } - selectText(element: Element | null): 'error:notconnected' | 'done' { + selectText(node: Node): 'error:notconnected' | 'done' { + const element = this._retarget(node, 'follow-label'); if (!element) return 'error:notconnected'; if (element.nodeName.toLowerCase() === 'input') { diff --git a/test/elementhandle-convenience.spec.ts b/test/elementhandle-convenience.spec.ts index 3f7cea81354a9..98836a2778829 100644 --- a/test/elementhandle-convenience.spec.ts +++ b/test/elementhandle-convenience.spec.ts @@ -176,6 +176,22 @@ it('isVisible and isHidden should work', async ({ page }) => { expect(await page.isHidden('no-such-element')).toBe(true); }); +it('element state checks should work for label with zero-sized input', async ({page, server}) => { + await page.setContent(` + + `); + // Visible checks the label. + expect(await page.isVisible('text=Click me')).toBe(true); + expect(await page.isHidden('text=Click me')).toBe(false); + + // Enabled checks the input. + expect(await page.isEnabled('text=Click me')).toBe(false); + expect(await page.isDisabled('text=Click me')).toBe(true); +}); + it('isEnabled and isDisabled should work', async ({ page }) => { await page.setContent(` diff --git a/test/page-click.spec.ts b/test/page-click.spec.ts index ebfd53d8ede6a..5bbfddb5d1b59 100644 --- a/test/page-click.spec.ts +++ b/test/page-click.spec.ts @@ -768,3 +768,14 @@ it('should click the button when window.innerWidth is corrupted', async ({page, await page.click('button'); expect(await page.evaluate('result')).toBe('Clicked'); }); + +it('should click zero-sized input by label', async ({page, server}) => { + await page.setContent(` + + `); + await page.click('text=Click me'); + expect(await page.evaluate('window.__clicked')).toBe(true); +});