fix(handleLocator): address API review feedback (#29412)

- docs improvements;
- `force: true` ignores `handleLocator`;
- wrapping an internal call;
- more test cases;
- `pw:api` log entries for this API.

docs/src/api/class-page.md

@@ -3136,10 +3136,22 @@ return value resolves to `[]`.
 
 Registers a handler for an element that might block certain actions like click. The handler should get rid of the blocking element so that an action may proceed. This is useful for nondeterministic interstitial pages or dialogs, like a cookie consent dialog.
 
-The handler will be executed before [actionability checks](../actionability.md) for each action, and also before each attempt of the [web assertions](../test-assertions.md). When no actions or assertions are executed, the handler will not be run at all, even if the interstitial element appears on the page.
+The handler will be executed before the [actionability checks](../actionability.md) for each action, as well as before each probe of the [web assertions](../test-assertions.md). When no actions are executed and no assertions are probed, the handler does not run at all, even if the given locator appears on the page. Actions that pass the `force` option do not trigger the handler.
 
 Note that execution time of the handler counts towards the timeout of the action/assertion that executed the handler.
 
+You can register multiple handlers. However, only a single handler will be running at a time. Any actions inside a handler must not require another handler to run.
+
+:::warning
+Running the interceptor will alter your page state mid-test. For example it will change the currently focused element and move the mouse. Make sure that the actions that run after the interceptor are self-contained and do not rely on the focus and mouse state.
+<br />
+<br />
+For example, consider a test that calls [`method: Locator.focus`] followed by [`method: Keyboard.press`]. If your handler clicks a button between these two actions, the focused element most likely will be wrong, and key press will happen on the unexpected element. Use [`method: Locator.press`] instead to avoid this problem.
+<br />
+<br />
+Another example is a series of mouse actions, where [`method: Mouse.move`] is followed by [`method: Mouse.down`]. Again, when the handler runs between these two actions, the mouse position will be wrong during the mouse down. Prefer methods like [`method: Locator.click`] that are self-contained.
+:::
+
 **Usage**
 
 An example that closes a cookie dialog when it appears:

packages/playwright-core/src/client/page.ts

@@ -374,7 +374,7 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
       const handler = this._locatorHandlers.get(uid);
       await handler?.();
     } finally {
-      this._channel.resolveLocatorHandlerNoReply({ uid }).catch(() => {});
+      this._wrapApiCall(() => this._channel.resolveLocatorHandlerNoReply({ uid }), true).catch(() => {});
     }
   }
 

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

@@ -306,7 +306,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
       } else {
         progress.log(`attempting ${actionName} action${options.trial ? ' (trial run)' : ''}`);
       }
-      if (!options.skipLocatorHandlersCheckpoint)
+      if (!options.skipLocatorHandlersCheckpoint && !options.force)
         await this._frame._page.performLocatorHandlersCheckpoint(progress);
       const result = await action(retry);
       ++retry;

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

@@ -1146,21 +1146,21 @@ export class Frame extends SdkObject {
   async click(metadata: CallMetadata, selector: string, options: types.MouseClickOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions) {
     const controller = new ProgressController(metadata, this);
     return controller.run(async progress => {
-      return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, true /* performLocatorHandlersCheckpoint */, handle => handle._click(progress, options)));
+      return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, !options.force /* performLocatorHandlersCheckpoint */, handle => handle._click(progress, options)));
     }, this._page._timeoutSettings.timeout(options));
   }
 
   async dblclick(metadata: CallMetadata, selector: string, options: types.MouseMultiClickOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions = {}) {
     const controller = new ProgressController(metadata, this);
     return controller.run(async progress => {
-      return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, true /* performLocatorHandlersCheckpoint */, handle => handle._dblclick(progress, options)));
+      return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, !options.force /* performLocatorHandlersCheckpoint */, handle => handle._dblclick(progress, options)));
     }, this._page._timeoutSettings.timeout(options));
   }
 
   async dragAndDrop(metadata: CallMetadata, source: string, target: string,  options: types.DragActionOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions = {}) {
     const controller = new ProgressController(metadata, this);
     await controller.run(async progress => {
-      dom.assertDone(await this._retryWithProgressIfNotConnected(progress, source, options.strict, true /* performLocatorHandlersCheckpoint */, async handle => {
+      dom.assertDone(await this._retryWithProgressIfNotConnected(progress, source, options.strict, !options.force /* performLocatorHandlersCheckpoint */, async handle => {
         return handle._retryPointerAction(progress, 'move and down', false, async point => {
           await this._page.mouse.move(point.x, point.y);
           await this._page.mouse.down();
@@ -1189,14 +1189,14 @@ export class Frame extends SdkObject {
       throw new Error('The page does not support tap. Use hasTouch context option to enable touch support.');
     const controller = new ProgressController(metadata, this);
     return controller.run(async progress => {
-      return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, true /* performLocatorHandlersCheckpoint */, handle => handle._tap(progress, options)));
+      return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, !options.force /* performLocatorHandlersCheckpoint */, handle => handle._tap(progress, options)));
     }, this._page._timeoutSettings.timeout(options));
   }
 
   async fill(metadata: CallMetadata, selector: string, value: string, options: types.NavigatingActionWaitOptions & { force?: boolean }) {
     const controller = new ProgressController(metadata, this);
     return controller.run(async progress => {
-      return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, true /* performLocatorHandlersCheckpoint */, handle => handle._fill(progress, value, options)));
+      return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, !options.force /* performLocatorHandlersCheckpoint */, handle => handle._fill(progress, value, options)));
     }, this._page._timeoutSettings.timeout(options));
   }
 
@@ -1317,14 +1317,14 @@ export class Frame extends SdkObject {
   async hover(metadata: CallMetadata, selector: string, options: types.PointerActionOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions = {}) {
     const controller = new ProgressController(metadata, this);
     return controller.run(async progress => {
-      return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, true /* performLocatorHandlersCheckpoint */, handle => handle._hover(progress, options)));
+      return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, !options.force /* performLocatorHandlersCheckpoint */, handle => handle._hover(progress, options)));
     }, this._page._timeoutSettings.timeout(options));
   }
 
   async selectOption(metadata: CallMetadata, selector: string, elements: dom.ElementHandle[], values: types.SelectOption[], options: types.NavigatingActionWaitOptions & types.ForceOptions = {}): Promise<string[]> {
     const controller = new ProgressController(metadata, this);
     return controller.run(async progress => {
-      return await this._retryWithProgressIfNotConnected(progress, selector, options.strict, true /* performLocatorHandlersCheckpoint */, handle => handle._selectOption(progress, elements, values, options));
+      return await this._retryWithProgressIfNotConnected(progress, selector, options.strict, !options.force /* performLocatorHandlersCheckpoint */, handle => handle._selectOption(progress, elements, values, options));
     }, this._page._timeoutSettings.timeout(options));
   }
 
@@ -1353,14 +1353,14 @@ export class Frame extends SdkObject {
   async check(metadata: CallMetadata, selector: string, options: types.PointerActionWaitOptions & types.NavigatingActionWaitOptions = {}) {
     const controller = new ProgressController(metadata, this);
     return controller.run(async progress => {
-      return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, true /* performLocatorHandlersCheckpoint */, handle => handle._setChecked(progress, true, options)));
+      return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, !options.force /* performLocatorHandlersCheckpoint */, handle => handle._setChecked(progress, true, options)));
     }, this._page._timeoutSettings.timeout(options));
   }
 
   async uncheck(metadata: CallMetadata, selector: string, options: types.PointerActionWaitOptions & types.NavigatingActionWaitOptions = {}) {
     const controller = new ProgressController(metadata, this);
     return controller.run(async progress => {
-      return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, true /* performLocatorHandlersCheckpoint */, handle => handle._setChecked(progress, false, options)));
+      return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, !options.force /* performLocatorHandlersCheckpoint */, handle => handle._setChecked(progress, false, options)));
     }, this._page._timeoutSettings.timeout(options));
   }
 

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

@@ -44,6 +44,7 @@ import { isInvalidSelectorError } from '../utils/isomorphic/selectorParser';
 import { parseEvaluationResultValue, source } from './isomorphic/utilityScriptSerializers';
 import type { SerializedValue } from './isomorphic/utilityScriptSerializers';
 import { TargetClosedError } from './errors';
+import { asLocator } from '../utils/isomorphic/locatorGenerators';
 
 export interface PageDelegate {
   readonly rawMouse: input.RawMouse;
@@ -458,9 +459,11 @@ export class Page extends SdkObject {
       }
       if (handler.resolved) {
         ++this._locatorHandlerRunningCounter;
+        progress.log(`  found ${asLocator(this.attribution.playwright.options.sdkLanguage, handler.selector)}, intercepting action to run the handler`);
         await this.openScope.race(handler.resolved).finally(() => --this._locatorHandlerRunningCounter);
         // Avoid side-effects after long-running operation.
         progress.throwIfAborted();
+        progress.log(`  interception handler has finished, continuing`);
       }
     }
   }

packages/playwright-core/types/types.d.ts

@@ -2928,13 +2928,31 @@ export interface Page {
    * blocking element so that an action may proceed. This is useful for nondeterministic interstitial pages or dialogs,
    * like a cookie consent dialog.
    *
-   * The handler will be executed before [actionability checks](https://playwright.dev/docs/actionability) for each action, and also before
-   * each attempt of the [web assertions](https://playwright.dev/docs/test-assertions). When no actions or assertions are executed, the
-   * handler will not be run at all, even if the interstitial element appears on the page.
+   * The handler will be executed before the [actionability checks](https://playwright.dev/docs/actionability) for each action, as well as
+   * before each probe of the [web assertions](https://playwright.dev/docs/test-assertions). When no actions are executed and no assertions
+   * are probed, the handler does not run at all, even if the given locator appears on the page. Actions that pass the
+   * `force` option do not trigger the handler.
    *
    * Note that execution time of the handler counts towards the timeout of the action/assertion that executed the
    * handler.
    *
+   * You can register multiple handlers. However, only a single handler will be running at a time. Any actions inside a
+   * handler must not require another handler to run.
+   *
+   * **NOTE** Running the interceptor will alter your page state mid-test. For example it will change the currently
+   * focused element and move the mouse. Make sure that the actions that run after the interceptor are self-contained
+   * and do not rely on the focus and mouse state. <br /> <br /> For example, consider a test that calls
+   * [locator.focus([options])](https://playwright.dev/docs/api/class-locator#locator-focus) followed by
+   * [keyboard.press(key[, options])](https://playwright.dev/docs/api/class-keyboard#keyboard-press). If your handler
+   * clicks a button between these two actions, the focused element most likely will be wrong, and key press will happen
+   * on the unexpected element. Use
+   * [locator.press(key[, options])](https://playwright.dev/docs/api/class-locator#locator-press) instead to avoid this
+   * problem. <br /> <br /> Another example is a series of mouse actions, where
+   * [mouse.move(x, y[, options])](https://playwright.dev/docs/api/class-mouse#mouse-move) is followed by
+   * [mouse.down([options])](https://playwright.dev/docs/api/class-mouse#mouse-down). Again, when the handler runs
+   * between these two actions, the mouse position will be wrong during the mouse down. Prefer methods like
+   * [locator.click([options])](https://playwright.dev/docs/api/class-locator#locator-click) that are self-contained.
+   *
    * **Usage**
    *
    * An example that closes a cookie dialog when it appears:

tests/assets/input/handle-locator.html

@@ -14,6 +14,9 @@
       #target.hidden {
         visibility: hidden;
       }
+      #target:hover {
+        background: yellow;
+      }
       #interstitial {
         position: absolute;
         top: 0;

tests/page/page-handle-locator.spec.ts

@@ -85,6 +85,38 @@ test('should work with a custom check', async ({ page, server }) => {
   }
 });
 
+test('should work with locator.hover()', async ({ page, server }) => {
+  await page.goto(server.PREFIX + '/input/handle-locator.html');
+
+  await page.handleLocator(page.getByText('This interstitial covers the button'), async () => {
+    await page.locator('#close').click();
+  });
+
+  await page.locator('#aside').hover();
+  await page.evaluate(() => {
+    (window as any).setupAnnoyingInterstitial('pointerover', 1, 'capture');
+  });
+  await page.locator('#target').hover();
+  await expect(page.locator('#interstitial')).not.toBeVisible();
+  expect(await page.$eval('#target', e => window.getComputedStyle(e).backgroundColor)).toBe('rgb(255, 255, 0)');
+});
+
+test('should not work with force:true', async ({ page, server }) => {
+  await page.goto(server.PREFIX + '/input/handle-locator.html');
+
+  await page.handleLocator(page.getByText('This interstitial covers the button'), async () => {
+    await page.locator('#close').click();
+  });
+
+  await page.locator('#aside').hover();
+  await page.evaluate(() => {
+    (window as any).setupAnnoyingInterstitial('none', 1);
+  });
+  await page.locator('#target').click({ force: true, timeout: 2000 });
+  expect(await page.locator('#interstitial').isVisible()).toBe(true);
+  expect(await page.evaluate('window.clicked')).toBe(undefined);
+});
+
 test('should throw when page closes', async ({ page, server }) => {
   await page.goto(server.PREFIX + '/input/handle-locator.html');