From c4581e54c0b54e395ee9d87fb2b91e1438bc3a93 Mon Sep 17 00:00:00 2001
From: Dmitry Gozman <dgozman@gmail.com>
Date: Wed, 1 Jun 2022 15:23:41 -0700
Subject: [PATCH] fix(click): detect iframe overlays that cover target element
 (#13876)

This restores the old hit target check, in addition to the new
hit target interceptor.

This way, we got some coverage for iframes and other quirky cases,
but keep the bullet-proof hit target check in place.
---
 packages/playwright-core/src/server/dom.ts    | 51 ++++++++----------
 .../src/server/injected/injectedScript.ts     | 54 +++++++++++++++----
 tests/library/hit-target.spec.ts              | 15 ++++++
 3 files changed, 82 insertions(+), 38 deletions(-)

diff --git a/packages/playwright-core/src/server/dom.ts b/packages/playwright-core/src/server/dom.ts
index 8c0c54bd68..e1942d13fd 100644
--- a/packages/playwright-core/src/server/dom.ts
+++ b/packages/playwright-core/src/server/dom.ts
@@ -436,34 +436,25 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
       if ((options as any).__testHookBeforeHitTarget)
         await (options as any).__testHookBeforeHitTarget();
 
-      if (actionName === 'move and up') {
-        // When dropping, the "element that is being dragged" often stays under the cursor,
-        // so hit target check at the moment we receive mousedown does not work -
-        // it finds the "element that is being dragged" instead of the
-        // "element that we drop onto".
-        progress.log(`  checking that element receives pointer events at (${point.x},${point.y})`);
-        const hitTargetResult = await this._checkHitTargetAt(point);
-        if (hitTargetResult !== 'done')
-          return hitTargetResult;
-        progress.log(`  element does receive pointer events`);
-        if (options.trial) {
-          progress.log(`  trial ${actionName} has finished`);
-          return 'done';
-        }
-      } else {
-        const actionType = (actionName === 'hover' || actionName === 'tap') ? actionName : 'mouse';
-        const handle = await this.evaluateHandleInUtility(([injected, node, { actionType, trial }]) => injected.setupHitTargetInterceptor(node, actionType, trial), { actionType, trial: !!options.trial } as const);
-        if (handle === 'error:notconnected')
-          return handle;
-        if (!handle._objectId)
-          return handle.rawValue() as 'error:notconnected';
-        hitTargetInterceptionHandle = handle as any;
-        progress.cleanupWhenAborted(() => {
-          // Do not await here, just in case the renderer is stuck (e.g. on alert)
-          // and we won't be able to cleanup.
-          hitTargetInterceptionHandle!.evaluate(h => h.stop()).catch(e => {});
-        });
+      const hitPoint = await this._viewportPointToDocument(point);
+      if (hitPoint === 'error:notconnected')
+        return hitPoint;
+      const actionType = actionName === 'move and up' ? 'drag' : ((actionName === 'hover' || actionName === 'tap') ? actionName : 'mouse');
+      const handle = await this.evaluateHandleInUtility(([injected, node, { actionType, hitPoint, trial }]) => injected.setupHitTargetInterceptor(node, actionType, hitPoint, trial), { actionType, hitPoint, trial: !!options.trial } as const);
+      if (handle === 'error:notconnected')
+        return handle;
+      if (!handle._objectId) {
+        const error = handle.rawValue() as string;
+        if (error === 'error:notconnected')
+          return error;
+        return { hitTargetDescription: error };
       }
+      hitTargetInterceptionHandle = handle as any;
+      progress.cleanupWhenAborted(() => {
+        // Do not await here, just in case the renderer is stuck (e.g. on alert)
+        // and we won't be able to cleanup.
+        hitTargetInterceptionHandle!.evaluate(h => h.stop()).catch(e => {});
+      });
     }
 
     const actionResult = await this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => {
@@ -864,7 +855,9 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
     return result;
   }
 
-  async _checkHitTargetAt(point: types.Point): Promise<'error:notconnected' | { hitTargetDescription: string } | 'done'> {
+  async _viewportPointToDocument(point: types.Point): Promise<types.Point | 'error:notconnected'> {
+    if (!this._frame.parentFrame())
+      return point;
     const frame = await this.ownerFrame();
     if (frame && frame.parentFrame()) {
       const element = await frame.frameElement();
@@ -874,7 +867,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
       // Translate from viewport coordinates to frame coordinates.
       point = { x: point.x - box.x, y: point.y - box.y };
     }
-    return this.evaluateInUtility(([injected, node, point]) => injected.checkHitTargetAt(node, point), point);
+    return point;
   }
 }
 
diff --git a/packages/playwright-core/src/server/injected/injectedScript.ts b/packages/playwright-core/src/server/injected/injectedScript.ts
index 33d6f6a549..c9c5896b6c 100644
--- a/packages/playwright-core/src/server/injected/injectedScript.ts
+++ b/packages/playwright-core/src/server/injected/injectedScript.ts
@@ -715,14 +715,6 @@ export class InjectedScript {
     input.dispatchEvent(new Event('change', { 'bubbles': true }));
   }
 
-  checkHitTargetAt(node: Node, point: { x: number, y: number }): 'error:notconnected' | 'done' | { hitTargetDescription: string } {
-    const element: Element | null | undefined = node.nodeType === Node.ELEMENT_NODE ? (node as Element) : node.parentElement;
-    if (!element || !element.isConnected)
-      return 'error:notconnected';
-    const hitElement = this.deepElementFromPoint(document, point.x, point.y);
-    return this._expectHitTargetParent(hitElement, element);
-  }
-
   private _expectHitTargetParent(hitElement: Element | undefined, targetElement: Element) {
     targetElement = targetElement.closest('button, [role=button], a, [role=link]') || targetElement;
     const hitParents: Element[] = [];
@@ -752,11 +744,55 @@ export class InjectedScript {
     return { hitTargetDescription };
   }
 
-  setupHitTargetInterceptor(node: Node, action: 'hover' | 'tap' | 'mouse', blockAllEvents: boolean): HitTargetInterceptionResult | 'error:notconnected' {
+  // Life of a pointer action, for example click.
+  //
+  // 0. Retry items 1 and 2 while action fails due to navigation or element being detached.
+  //   1. Resolve selector to an element.
+  //   2. Retry the following steps until the element is detached or frame navigates away.
+  //     2a. Wait for the element to be stable (not moving), visible and enabled.
+  //     2b. Scroll element into view. Scrolling alternates between:
+  //         - Built-in protocol scrolling.
+  //         - Anchoring to the top/left, bottom/right and center/center.
+  //         This is to scroll elements from under sticky headers/footers.
+  //     2c. Click point is calculated, either based on explicitly specified position,
+  //         or some visible point of the element based on protocol content quads.
+  //     2d. Click point relative to page viewport is converted relative to the target iframe
+  //         for the next hit-point check.
+  //     2e. (injected) Hit target at the click point must be a descendant of the target element.
+  //         This prevents mis-clicking in edge cases like <iframe> overlaying the target.
+  //     2f. (injected) Events specific for click (or some other action type) are intercepted on
+  //         the Window with capture:true. See 2i for details.
+  //         Note: this step is skipped for drag&drop (see inline comments for the reason).
+  //     2g. Necessary keyboard modifiers are pressed.
+  //     2h. Click event is issued (mousemove + mousedown + mouseup).
+  //     2i. (injected) For each event, we check that hit target at the event point
+  //         is a descendant of the target element.
+  //         This guarantees no race between issuing the event and handling it in the page,
+  //         for example due to layout shift.
+  //         When hit target check fails, we block all future events in the page.
+  //     2j. Keyboard modifiers are restored.
+  //     2k. (injected) Event interceptor is removed.
+  //     2l. All navigations triggered between 2g-2k are awaited to be either committed or canceled.
+  //     2m. If failed, wait for increasing amount of time before the next retry.
+  setupHitTargetInterceptor(node: Node, action: 'hover' | 'tap' | 'mouse' | 'drag', hitPoint: { x: number, y: number }, blockAllEvents: boolean): HitTargetInterceptionResult | 'error:notconnected' | string /* hitTargetDescription */ {
     const element: Element | null | undefined = node.nodeType === Node.ELEMENT_NODE ? (node as Element) : node.parentElement;
     if (!element || !element.isConnected)
       return 'error:notconnected';
 
+    // First do a preliminary check, to reduce the possibility of some iframe
+    // intercepting the action.
+    const preliminaryHitElement = this.deepElementFromPoint(document, hitPoint.x, hitPoint.y);
+    const preliminaryResult = this._expectHitTargetParent(preliminaryHitElement, element);
+    if (preliminaryResult !== 'done')
+      return preliminaryResult.hitTargetDescription;
+
+    // When dropping, the "element that is being dragged" often stays under the cursor,
+    // so hit target check at the moment we receive mousedown does not work -
+    // it finds the "element that is being dragged" instead of the
+    // "element that we drop onto".
+    if (action === 'drag')
+      return { stop: () => 'done' };
+
     const events = {
       'hover': kHoverHitTargetInterceptorEvents,
       'tap': kTapHitTargetInterceptorEvents,
diff --git a/tests/library/hit-target.spec.ts b/tests/library/hit-target.spec.ts
index c36d3e665a..07bdf475c4 100644
--- a/tests/library/hit-target.spec.ts
+++ b/tests/library/hit-target.spec.ts
@@ -239,3 +239,18 @@ it('should work with block inside inline in shadow dom', async ({ page, server }
   await page.locator('#target').click();
   expect(await page.evaluate('window._clicked')).toBe(true);
 });
+
+it('should not click iframe overlaying the target', async ({ page, server }) => {
+  await page.goto(server.EMPTY_PAGE);
+  await page.setContent(`
+    <button style="position: absolute; left: 250px;bottom: 0;height: 40px;width: 200px;" onclick="window._clicked=1">
+      click-me
+    </button>
+    <div style="background: transparent; bottom: 0px; left: 0px; margin: 0px; padding: 0px; position: fixed; z-index: 2147483647;">
+      <iframe srcdoc="<body onclick='window.top._clicked=2' style='background-color:red;height:40px;'></body>" style="display: block; border: 0px; width: 100vw; height: 48px;"></iframe>
+    </div>
+  `);
+  const error = await page.click('text=click-me', { timeout: 500 }).catch(e => e);
+  expect(await page.evaluate('window._clicked')).toBe(undefined);
+  expect(error.message).toContain(`<iframe srcdoc="<body onclick='window.top._clicked=2' st…></iframe> from <div>…</div> subtree intercepts pointer events`);
+});