diff --git a/packages/playwright-core/src/server/chromium/crInput.ts b/packages/playwright-core/src/server/chromium/crInput.ts index 60ef79e524..0e1d0bfe07 100644 --- a/packages/playwright-core/src/server/chromium/crInput.ts +++ b/packages/playwright-core/src/server/chromium/crInput.ts @@ -96,8 +96,8 @@ export class RawMouseImpl implements input.RawMouse { this._dragManager = dragManager; } - async move(x: number, y: number, button: types.MouseButton | 'none', buttons: Set, modifiers: Set): Promise { - await this._dragManager.interceptDragCausedByMove(x, y, button, buttons, modifiers, async () => { + async move(x: number, y: number, button: types.MouseButton | 'none', buttons: Set, modifiers: Set, forClick: boolean): Promise { + const actualMove = async () => { await this._client.send('Input.dispatchMouseEvent', { type: 'mouseMoved', button, @@ -105,7 +105,13 @@ export class RawMouseImpl implements input.RawMouse { y, modifiers: toModifiersMask(modifiers) }); - }); + }; + if (forClick) { + // Avoid extra protocol calls related to drag and drop, because click relies on + // move-down-up protocol commands being sent synchronously. + return actualMove(); + } + await this._dragManager.interceptDragCausedByMove(x, y, button, buttons, modifiers, actualMove); } async down(x: number, y: number, button: types.MouseButton, buttons: Set, modifiers: Set, clickCount: number): Promise { diff --git a/packages/playwright-core/src/server/dom.ts b/packages/playwright-core/src/server/dom.ts index a6cf6178c1..79881a99c8 100644 --- a/packages/playwright-core/src/server/dom.ts +++ b/packages/playwright-core/src/server/dom.ts @@ -407,7 +407,7 @@ export class ElementHandle extends js.JSHandle { const point = roundPoint(maybePoint); progress.metadata.point = point; - if (!process.env.PLAYWRIGHT_LAYOUT_SHIFT_CHECK) + if (process.env.PLAYWRIGHT_NO_LAYOUT_SHIFT_CHECK) return this._finishPointerAction(progress, actionName, point, options, action); else return this._finishPointerActionDetectLayoutShift(progress, actionName, point, options, action); diff --git a/packages/playwright-core/src/server/firefox/ffInput.ts b/packages/playwright-core/src/server/firefox/ffInput.ts index b0408c80b9..7190c0f63b 100644 --- a/packages/playwright-core/src/server/firefox/ffInput.ts +++ b/packages/playwright-core/src/server/firefox/ffInput.ts @@ -108,7 +108,7 @@ export class RawMouseImpl implements input.RawMouse { this._client = client; } - async move(x: number, y: number, button: types.MouseButton | 'none', buttons: Set, modifiers: Set): Promise { + async move(x: number, y: number, button: types.MouseButton | 'none', buttons: Set, modifiers: Set, forClick: boolean): Promise { await this._client.send('Page.dispatchMouseEvent', { type: 'mousemove', button: 0, diff --git a/packages/playwright-core/src/server/injected/injectedScript.ts b/packages/playwright-core/src/server/injected/injectedScript.ts index 49c1b59fb6..9c91949ed5 100644 --- a/packages/playwright-core/src/server/injected/injectedScript.ts +++ b/packages/playwright-core/src/server/injected/injectedScript.ts @@ -731,19 +731,17 @@ export class InjectedScript { if (!event.isTrusted) return; - // Element was detached during the action, for example in some event handler. - // If events before that were correctly pointing to it, consider this a valid scenario. - if (!element.isConnected) - return; - // Determine the event point. Note that Firefox does not always have window.TouchEvent. const point = (!!window.TouchEvent && (event instanceof window.TouchEvent)) ? event.touches[0] : (event as MouseEvent | PointerEvent); - if (!!point && (result === undefined || result === 'done')) { - // Determine whether we hit the target element, unless we have already failed. + + // Check that we hit the right element at the first event, and assume all + // subsequent events will be fine. + if (result === undefined && point) { const hitElement = this.deepElementFromPoint(document, point.clientX, point.clientY); result = this._expectHitTargetParent(hitElement, element); } - if (blockAllEvents || result !== 'done') { + + if (blockAllEvents || (result !== 'done' && result !== undefined)) { event.preventDefault(); event.stopPropagation(); event.stopImmediatePropagation(); diff --git a/packages/playwright-core/src/server/input.ts b/packages/playwright-core/src/server/input.ts index a08e7b2bf1..305abd566a 100644 --- a/packages/playwright-core/src/server/input.ts +++ b/packages/playwright-core/src/server/input.ts @@ -157,7 +157,7 @@ export class Keyboard { } export interface RawMouse { - move(x: number, y: number, button: types.MouseButton | 'none', buttons: Set, modifiers: Set): Promise; + move(x: number, y: number, button: types.MouseButton | 'none', buttons: Set, modifiers: Set, forClick: boolean): Promise; down(x: number, y: number, button: types.MouseButton, buttons: Set, modifiers: Set, clickCount: number): Promise; up(x: number, y: number, button: types.MouseButton, buttons: Set, modifiers: Set, clickCount: number): Promise; wheel(x: number, y: number, buttons: Set, modifiers: Set, deltaX: number, deltaY: number): Promise; @@ -178,7 +178,7 @@ export class Mouse { this._keyboard = this._page.keyboard; } - async move(x: number, y: number, options: { steps?: number } = {}) { + async move(x: number, y: number, options: { steps?: number, forClick?: boolean } = {}) { const { steps = 1 } = options; const fromX = this._x; const fromY = this._y; @@ -187,7 +187,7 @@ export class Mouse { for (let i = 1; i <= steps; i++) { const middleX = fromX + (x - fromX) * (i / steps); const middleY = fromY + (y - fromY) * (i / steps); - await this._raw.move(middleX, middleY, this._lastButton, this._buttons, this._keyboard._modifiers()); + await this._raw.move(middleX, middleY, this._lastButton, this._buttons, this._keyboard._modifiers(), !!options.forClick); await this._page._doSlowMo(); } } @@ -211,7 +211,7 @@ export class Mouse { async click(x: number, y: number, options: { delay?: number, button?: types.MouseButton, clickCount?: number } = {}) { const { delay = null, clickCount = 1 } = options; if (delay) { - this.move(x, y); + this.move(x, y, { forClick: true }); for (let cc = 1; cc <= clickCount; ++cc) { await this.down({ ...options, clickCount: cc }); await new Promise(f => setTimeout(f, delay)); @@ -221,7 +221,7 @@ export class Mouse { } } else { const promises = []; - promises.push(this.move(x, y)); + promises.push(this.move(x, y, { forClick: true })); for (let cc = 1; cc <= clickCount; ++cc) { promises.push(this.down({ ...options, clickCount: cc })); promises.push(this.up({ ...options, clickCount: cc })); diff --git a/packages/playwright-core/src/server/webkit/wkInput.ts b/packages/playwright-core/src/server/webkit/wkInput.ts index 87554d287b..b31a05d5ec 100644 --- a/packages/playwright-core/src/server/webkit/wkInput.ts +++ b/packages/playwright-core/src/server/webkit/wkInput.ts @@ -113,7 +113,7 @@ export class RawMouseImpl implements input.RawMouse { this._session = session; } - async move(x: number, y: number, button: types.MouseButton | 'none', buttons: Set, modifiers: Set): Promise { + async move(x: number, y: number, button: types.MouseButton | 'none', buttons: Set, modifiers: Set, forClick: boolean): Promise { await this._pageProxySession.send('Input.dispatchMouseEvent', { type: 'move', button, diff --git a/tests/hit-target.spec.ts b/tests/hit-target.spec.ts index 4556130b62..479cade2e8 100644 --- a/tests/hit-target.spec.ts +++ b/tests/hit-target.spec.ts @@ -48,18 +48,18 @@ it('should block all events when hit target is wrong', async ({ page, server }) expect(allEvents).toEqual([]); }); -it('should block click when mousedown succeeds but mouseup fails', async ({ page, server }) => { - it.skip(!process.env.PLAYWRIGHT_LAYOUT_SHIFT_CHECK); +it('should block click when mousedown fails', async ({ page, server }) => { + it.skip(!!process.env.PLAYWRIGHT_NO_LAYOUT_SHIFT_CHECK); await page.goto(server.PREFIX + '/input/button.html'); await page.$eval('button', button => { - button.addEventListener('mousedown', () => { + button.addEventListener('mousemove', () => { button.style.marginLeft = '100px'; }); const allEvents = []; (window as any).allEvents = allEvents; - for (const name of ['mousedown', 'mouseup', 'click', 'dblclick', 'auxclick', 'contextmenu', 'pointerdown', 'pointerup']) + for (const name of ['mousemove', 'mousedown', 'mouseup', 'click', 'dblclick', 'auxclick', 'contextmenu', 'pointerdown', 'pointerup']) button.addEventListener(name, e => allEvents.push(e.type)); }); @@ -68,9 +68,9 @@ it('should block click when mousedown succeeds but mouseup fails', async ({ page const allEvents = await page.evaluate(() => (window as any).allEvents); expect(allEvents).toEqual([ // First attempt failed. - 'pointerdown', 'mousedown', + 'mousemove', // Second attempt succeeded. - 'pointerdown', 'mousedown', 'pointerup', 'mouseup', 'click', + 'mousemove', 'pointerdown', 'mousedown', 'pointerup', 'mouseup', 'click', ]); }); @@ -87,12 +87,42 @@ it('should click when element detaches in mousedown', async ({ page, server }) = expect(await page.evaluate('result')).toBe('Mousedown'); }); +it('should block all events when hit target is wrong and element detaches', async ({ page, server }) => { + await page.goto(server.PREFIX + '/input/button.html'); + await page.$eval('button', button => { + const blocker = document.createElement('div'); + blocker.style.position = 'absolute'; + blocker.style.width = '400px'; + blocker.style.height = '400px'; + blocker.style.left = '0'; + blocker.style.top = '0'; + document.body.appendChild(blocker); + + window.addEventListener('mousemove', () => button.remove()); + + const allEvents = []; + (window as any).allEvents = allEvents; + for (const name of ['mousedown', 'mouseup', 'click', 'dblclick', 'auxclick', 'contextmenu', 'pointerdown', 'pointerup']) { + window.addEventListener(name, e => allEvents.push(e.type)); + blocker.addEventListener(name, e => allEvents.push(e.type)); + } + }); + + const error = await page.click('button', { timeout: 1000 }).catch(e => e); + expect(error.message).toContain('page.click: Timeout 1000ms exceeded.'); + + // Give it some time, just in case. + await page.waitForTimeout(1000); + const allEvents = await page.evaluate(() => (window as any).allEvents); + expect(allEvents).toEqual([]); +}); + it('should not block programmatic events', async ({ page, server }) => { - it.skip(!process.env.PLAYWRIGHT_LAYOUT_SHIFT_CHECK); + it.skip(!!process.env.PLAYWRIGHT_NO_LAYOUT_SHIFT_CHECK); await page.goto(server.PREFIX + '/input/button.html'); await page.$eval('button', button => { - button.addEventListener('mousedown', () => { + button.addEventListener('mousemove', () => { button.style.marginLeft = '100px'; button.dispatchEvent(new MouseEvent('click')); }); diff --git a/tests/tap.spec.ts b/tests/tap.spec.ts index 450af40160..b898045d94 100644 --- a/tests/tap.spec.ts +++ b/tests/tap.spec.ts @@ -48,7 +48,7 @@ it('trial run should not tap', async ({ page }) => { await page.tap('#a'); const eventsHandle = await trackEvents(await page.$('#b')); await page.tap('#b', { trial: true }); - const expected = !process.env.PLAYWRIGHT_LAYOUT_SHIFT_CHECK ? [] : ['pointerover', 'pointerenter', 'pointerout', 'pointerleave']; + const expected = !!process.env.PLAYWRIGHT_NO_LAYOUT_SHIFT_CHECK ? [] : ['pointerover', 'pointerenter', 'pointerout', 'pointerleave']; expect(await eventsHandle.jsonValue()).toEqual(expected); });