feat(input): restore modified layout shift check (#11032)

This changes previous layout shift attempt (see #9546)
to account for more valid usecases:
- On the first event that is intercepted we enforce the hit target. This
  is similar to the current mode that checks hit target before the action,
  but is better timed.
- On subsequent events we assume that everything is fine. This covers more
  scenarios like react rerender, glass pane on mousedown, detach on mouseup.

This check is enabled by default, with `process.env.PLAYWRIGHT_NO_LAYOUT_SHIFT_CHECK`
to opt out.

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<types.MouseButton>, modifiers: Set<types.KeyboardModifier>): Promise<void> {
-    await this._dragManager.interceptDragCausedByMove(x, y, button, buttons, modifiers, async () => {
+  async move(x: number, y: number, button: types.MouseButton | 'none', buttons: Set<types.MouseButton>, modifiers: Set<types.KeyboardModifier>, forClick: boolean): Promise<void> {
+    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<types.MouseButton>, modifiers: Set<types.KeyboardModifier>, clickCount: number): Promise<void> {

packages/playwright-core/src/server/dom.ts

@@ -407,7 +407,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
     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);

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<types.MouseButton>, modifiers: Set<types.KeyboardModifier>): Promise<void> {
+  async move(x: number, y: number, button: types.MouseButton | 'none', buttons: Set<types.MouseButton>, modifiers: Set<types.KeyboardModifier>, forClick: boolean): Promise<void> {
     await this._client.send('Page.dispatchMouseEvent', {
       type: 'mousemove',
       button: 0,

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();

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<types.MouseButton>, modifiers: Set<types.KeyboardModifier>): Promise<void>;
+  move(x: number, y: number, button: types.MouseButton | 'none', buttons: Set<types.MouseButton>, modifiers: Set<types.KeyboardModifier>, forClick: boolean): Promise<void>;
   down(x: number, y: number, button: types.MouseButton, buttons: Set<types.MouseButton>, modifiers: Set<types.KeyboardModifier>, clickCount: number): Promise<void>;
   up(x: number, y: number, button: types.MouseButton, buttons: Set<types.MouseButton>, modifiers: Set<types.KeyboardModifier>, clickCount: number): Promise<void>;
   wheel(x: number, y: number, buttons: Set<types.MouseButton>, modifiers: Set<types.KeyboardModifier>, deltaX: number, deltaY: number): Promise<void>;
@@ -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 }));

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<types.MouseButton>, modifiers: Set<types.KeyboardModifier>): Promise<void> {
+  async move(x: number, y: number, button: types.MouseButton | 'none', buttons: Set<types.MouseButton>, modifiers: Set<types.KeyboardModifier>, forClick: boolean): Promise<void> {
     await this._pageProxySession.send('Input.dispatchMouseEvent', {
       type: 'move',
       button,

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'));
     });

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);
 });