feat(addLocatorHandler): various improvements (#30494)

- Automatically waiting for the overlay locator to be hidden, with `allowStayingVisible` opt-out. - `times: 1` option. - `removeLocatorHandler(locator, handler)` method. - Passing `locator` as first argument to `handler`. Fixes #30471. Fixes #30424. Fixes #29779.
microsoft · Apr 24, 2024 · 59689c9 · 59689c9
1 parent e2f7ace
commit 59689c9
Show file tree

Hide file tree

Showing 13 changed files with 392 additions and 81 deletions.
diff --git a/docs/src/api/class-page.md b/docs/src/api/class-page.md
@@ -3155,6 +3155,7 @@ This method lets you set up a special function, called a handler, that activates
 Things to keep in mind:
 * When an overlay is shown predictably, we recommend explicitly waiting for it in your test and dismissing it as a part of your normal test flow, instead of using [`method: Page.addLocatorHandler`].
 * Playwright checks for the overlay every time before executing or retrying an action that requires an [actionability check](../actionability.md), or before performing an auto-waiting assertion check. When overlay is visible, Playwright calls the handler first, and then proceeds with the action/assertion. Note that the handler is only called when you perform an action/assertion - if the overlay becomes visible but you don't perform any actions, the handler will not be triggered.
+* After executing the handler, Playwright will ensure that overlay that triggered the handler is not visible anymore. You can opt-out of this behavior with [`option: allowStayingVisible`].
 * The execution time of the handler counts towards the timeout of the action/assertion that executed the handler. If your handler takes too long, it might cause timeouts.
 * You can register multiple handlers. However, only a single handler will be running at a time. Make sure the actions within a handler don't depend on another handler.
 
@@ -3284,13 +3285,13 @@ await page.GotoAsync("https://example.com");
 await page.GetByRole("button", new() { Name = "Start here" }).ClickAsync();
 ```
 
-An example with a custom callback on every actionability check. It uses a `<body>` locator that is always visible, so the handler is called before every actionability check:
+An example with a custom callback on every actionability check. It uses a `<body>` locator that is always visible, so the handler is called before every actionability check. It is important to specify [`option: allowStayingVisible`], because the handler does not hide the `<body>` element.
 
 ```js
 // Setup the handler.
 await page.addLocatorHandler(page.locator('body'), async () => {
   await page.evaluate(() => window.removeObstructionsForTestIfNeeded());
-});
+}, { allowStayingVisible: true });
 
 // Write the test as usual.
 await page.goto('https://example.com');
@@ -3301,7 +3302,7 @@ await page.getByRole('button', { name: 'Start here' }).click();
 // Setup the handler.
 page.addLocatorHandler(page.locator("body")), () => {
   page.evaluate("window.removeObstructionsForTestIfNeeded()");
-});
+}, new Page.AddLocatorHandlerOptions.setAllowStayingVisible(true));
 
 // Write the test as usual.
 page.goto("https://example.com");
@@ -3312,7 +3313,7 @@ page.getByRole("button", Page.GetByRoleOptions().setName("Start here")).click();
 # Setup the handler.
 def handler():
   page.evaluate("window.removeObstructionsForTestIfNeeded()")
-page.add_locator_handler(page.locator("body"), handler)
+page.add_locator_handler(page.locator("body"), handler, allow_staying_visible=True)
 
 # Write the test as usual.
 page.goto("https://example.com")
@@ -3323,7 +3324,7 @@ page.get_by_role("button", name="Start here").click()
 # Setup the handler.
 def handler():
   await page.evaluate("window.removeObstructionsForTestIfNeeded()")
-await page.add_locator_handler(page.locator("body"), handler)
+await page.add_locator_handler(page.locator("body"), handler, allow_staying_visible=True)
 
 # Write the test as usual.
 await page.goto("https://example.com")
@@ -3334,13 +3335,45 @@ await page.get_by_role("button", name="Start here").click()
 // Setup the handler.
 await page.AddLocatorHandlerAsync(page.Locator("body"), async () => {
   await page.EvaluateAsync("window.removeObstructionsForTestIfNeeded()");
-});
+}, new() { AllowStayingVisible = true });
 
 // Write the test as usual.
 await page.GotoAsync("https://example.com");
 await page.GetByRole("button", new() { Name = "Start here" }).ClickAsync();
 ```
 
+Handler takes the original locator as an argument. You can also automatically remove the handler after a number of invocations by setting [`option: times`]:
+
+```js
+await page.addLocatorHandler(page.getByLabel('Close'), async locator => {
+  await locator.click();
+}, { times: 1 });
+```
+
+```java
+page.addLocatorHandler(page.getByLabel("Close"), locator => {
+  locator.click();
+}, new Page.AddLocatorHandlerOptions().setTimes(1));
+```
+
+```python sync
+def handler(locator):
+  locator.click()
+page.add_locator_handler(page.get_by_label("Close"), handler, times=1)
+```
+
+```python async
+def handler(locator):
+  await locator.click()
+await page.add_locator_handler(page.get_by_label("Close"), handler, times=1)
+```
+
+```csharp
+await page.AddLocatorHandlerAsync(page.GetByText("Sign up to the newsletter"), async locator => {
+  await locator.ClickAsync();
+}, new() { Times = 1 });
+```
+
 ### param: Page.addLocatorHandler.locator
 * since: v1.42
 - `locator` <[Locator]>
@@ -3350,24 +3383,67 @@ Locator that triggers the handler.
 ### param: Page.addLocatorHandler.handler
 * langs: js, python
 * since: v1.42
-- `handler` <[function]>
+- `handler` <[function]\([Locator]\): [Promise<any>]>
 
 Function that should be run once [`param: locator`] appears. This function should get rid of the element that blocks actions like click.
 
 ### param: Page.addLocatorHandler.handler
 * langs: csharp
 * since: v1.42
-- `handler` <[function](): [Promise<any>]>
+- `handler` <[function]\([Locator]\)>
 
 Function that should be run once [`param: locator`] appears. This function should get rid of the element that blocks actions like click.
 
 ### param: Page.addLocatorHandler.handler
 * langs: java
 * since: v1.42
-- `handler` <[Runnable]>
+- `handler` <[function]\([Locator]\)>
 
 Function that should be run once [`param: locator`] appears. This function should get rid of the element that blocks actions like click.
 
+### option: Page.addLocatorHandler.times
+* since: v1.44
+- `times` <[int]>
+
+Specifies the maximum number of times this handler should be called. Unlimited by default.
+
+### option: Page.addLocatorHandler.allowStayingVisible
+* since: v1.44
+- `allowStayingVisible` <[boolean]>
+
+By default, after calling the handler Playwright will wait until the overlay becomes hidden, and only then Playwright will continue with the action/assertion that triggered the handler. This option allows to opt-out of this behavior, so that overlay can stay visible after the handler has run.
+
+
+## async method: Page.removeLocatorHandler
+* since: v1.44
+
+:::warning[Experimental]
+This method is experimental and its behavior may change in the upcoming releases.
+:::
+
+Removes locator handler added by [`method: Page.addLocatorHandler`].
+
+### param: Page.removeLocatorHandler.locator
+* since: v1.44
+- `locator` <[Locator]>
+
+Locator passed to [`method: Page.addLocatorHandler`].
+
+### param: Page.removeLocatorHandler.handler
+* langs: js, python
+* since: v1.44
+- `handler` <[function]\([Locator]\): [Promise<any>]>
+
+Handler passed to [`method: Page.addLocatorHandler`].
+
+### param: Page.addLocatorHandler.handler
+* langs: csharp, java
+* since: v1.44
+- `handler` <[function]\([Locator]\)>
+
+Handler passed to [`method: Page.addLocatorHandler`].
+
+
 ## async method: Page.reload
 * since: v1.8
 - returns: <[null]|[Response]>

diff --git a/packages/playwright-core/src/client/locator.ts b/packages/playwright-core/src/client/locator.ts
@@ -80,6 +80,10 @@ export class Locator implements api.Locator {
     });
   }
 
+  _equals(locator: Locator) {
+    return this._frame === locator._frame && this._selector === locator._selector;
+  }
+
   page() {
     return this._frame.page();
   }

diff --git a/packages/playwright-core/src/client/page.ts b/packages/playwright-core/src/client/page.ts
@@ -96,7 +96,7 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
   _closeWasCalled: boolean = false;
   private _harRouters: HarRouter[] = [];
 
-  private _locatorHandlers = new Map<number, Function>();
+  private _locatorHandlers = new Map<number, { locator: Locator, handler: (locator: Locator) => any, times: number | undefined }>();
 
   static from(page: channels.PageChannel): Page {
     return (page as any)._object;
@@ -362,19 +362,36 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
     return Response.fromNullable((await this._channel.reload({ ...options, waitUntil })).response);
   }
 
-  async addLocatorHandler(locator: Locator, handler: Function): Promise<void> {
+  async addLocatorHandler(locator: Locator, handler: (locator: Locator) => any, options: { times?: number, allowStayingVisible?: boolean } = {}): Promise<void> {
     if (locator._frame !== this._mainFrame)
       throw new Error(`Locator must belong to the main frame of this page`);
-    const { uid } = await this._channel.registerLocatorHandler({ selector: locator._selector });
-    this._locatorHandlers.set(uid, handler);
+    if (options.times === 0)
+      return;
+    const { uid } = await this._channel.registerLocatorHandler({ selector: locator._selector, allowStayingVisible: options.allowStayingVisible });
+    this._locatorHandlers.set(uid, { locator, handler, times: options.times });
   }
 
   private async _onLocatorHandlerTriggered(uid: number) {
+    let remove = false;
     try {
       const handler = this._locatorHandlers.get(uid);
-      await handler?.();
+      if (handler && handler.times !== 0) {
+        if (handler.times !== undefined)
+          handler.times--;
+        await handler.handler(handler.locator);
+      }
+      remove = handler?.times === 0;
     } finally {
-      this._wrapApiCall(() => this._channel.resolveLocatorHandlerNoReply({ uid }), true).catch(() => {});
+      this._wrapApiCall(() => this._channel.resolveLocatorHandlerNoReply({ uid, remove }), true).catch(() => {});
+    }
+  }
+
+  async removeLocatorHandler(locator: Locator, handler: (locator: Locator) => any): Promise<void> {
+    for (const [uid, data] of this._locatorHandlers) {
+      if (data.locator._equals(locator) && data.handler === handler) {
+        this._locatorHandlers.delete(uid);
+        await this._channel.unregisterLocatorHandlerNoReply({ uid }).catch(() => {});
+      }
     }
   }
 

diff --git a/packages/playwright-core/src/protocol/validator.ts b/packages/playwright-core/src/protocol/validator.ts
@@ -1046,14 +1046,20 @@ scheme.PageGoForwardResult = tObject({
 });
 scheme.PageRegisterLocatorHandlerParams = tObject({
   selector: tString,
+  allowStayingVisible: tOptional(tBoolean),
 });
 scheme.PageRegisterLocatorHandlerResult = tObject({
   uid: tNumber,
 });
 scheme.PageResolveLocatorHandlerNoReplyParams = tObject({
   uid: tNumber,
+  remove: tOptional(tBoolean),
 });
 scheme.PageResolveLocatorHandlerNoReplyResult = tOptional(tObject({}));
+scheme.PageUnregisterLocatorHandlerNoReplyParams = tObject({
+  uid: tNumber,
+});
+scheme.PageUnregisterLocatorHandlerNoReplyResult = tOptional(tObject({}));
 scheme.PageReloadParams = tObject({
   timeout: tOptional(tNumber),
   waitUntil: tOptional(tType('LifecycleEvent')),

diff --git a/packages/playwright-core/src/server/dispatchers/pageDispatcher.ts b/packages/playwright-core/src/server/dispatchers/pageDispatcher.ts
@@ -138,12 +138,16 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageChannel, Brows
   }
 
   async registerLocatorHandler(params: channels.PageRegisterLocatorHandlerParams, metadata: CallMetadata): Promise<channels.PageRegisterLocatorHandlerResult> {
-    const uid = this._page.registerLocatorHandler(params.selector);
+    const uid = this._page.registerLocatorHandler(params.selector, params.allowStayingVisible);
     return { uid };
   }
 
   async resolveLocatorHandlerNoReply(params: channels.PageResolveLocatorHandlerNoReplyParams, metadata: CallMetadata): Promise<void> {
-    this._page.resolveLocatorHandler(params.uid);
+    this._page.resolveLocatorHandler(params.uid, params.remove);
+  }
+
+  async unregisterLocatorHandlerNoReply(params: channels.PageUnregisterLocatorHandlerNoReplyParams, metadata: CallMetadata): Promise<void> {
+    this._page.unregisterLocatorHandler(params.uid);
   }
 
   async emulateMedia(params: channels.PageEmulateMediaParams, metadata: CallMetadata): Promise<void> {

diff --git a/packages/playwright-core/src/server/frames.ts b/packages/playwright-core/src/server/frames.ts
@@ -773,54 +773,59 @@ export class Frame extends SdkObject {
       throw new Error(`state: expected one of (attached|detached|visible|hidden)`);
     return controller.run(async progress => {
       progress.log(`waiting for ${this._asLocator(selector)}${state === 'attached' ? '' : ' to be ' + state}`);
-      const promise = this.retryWithProgressAndTimeouts(progress, [0, 20, 50, 100, 100, 500], async continuePolling => {
-        const resolved = await this.selectors.resolveInjectedForSelector(selector, options, scope);
-        progress.throwIfAborted();
-        if (!resolved) {
-          if (state === 'hidden' || state === 'detached')
-            return null;
-          return continuePolling;
-        }
-        const result = await resolved.injected.evaluateHandle((injected, { info, root }) => {
-          const elements = injected.querySelectorAll(info.parsed, root || document);
-          const element: Element | undefined  = elements[0];
-          const visible = element ? injected.isVisible(element) : false;
-          let log = '';
-          if (elements.length > 1) {
-            if (info.strict)
-              throw injected.strictModeViolationError(info.parsed, elements);
-            log = `  locator resolved to ${elements.length} elements. Proceeding with the first one: ${injected.previewNode(elements[0])}`;
-          } else if (element) {
-            log = `  locator resolved to ${visible ? 'visible' : 'hidden'} ${injected.previewNode(element)}`;
-          }
-          return { log, element, visible, attached: !!element };
-        }, { info: resolved.info, root: resolved.frame === this ? scope : undefined });
-        const { log, visible, attached } = await result.evaluate(r => ({ log: r.log, visible: r.visible, attached: r.attached }));
-        if (log)
-          progress.log(log);
-        const success = { attached, detached: !attached, visible, hidden: !visible }[state];
-        if (!success) {
-          result.dispose();
-          return continuePolling;
-        }
-        if (options.omitReturnValue) {
-          result.dispose();
+      return await this.waitForSelectorInternal(progress, selector, options, scope);
+    }, this._page._timeoutSettings.timeout(options));
+  }
+
+  async waitForSelectorInternal(progress: Progress, selector: string, options: types.WaitForElementOptions, scope?: dom.ElementHandle): Promise<dom.ElementHandle<Element> | null> {
+    const { state = 'visible' } = options;
+    const promise = this.retryWithProgressAndTimeouts(progress, [0, 20, 50, 100, 100, 500], async continuePolling => {
+      const resolved = await this.selectors.resolveInjectedForSelector(selector, options, scope);
+      progress.throwIfAborted();
+      if (!resolved) {
+        if (state === 'hidden' || state === 'detached')
           return null;
+        return continuePolling;
+      }
+      const result = await resolved.injected.evaluateHandle((injected, { info, root }) => {
+        const elements = injected.querySelectorAll(info.parsed, root || document);
+        const element: Element | undefined  = elements[0];
+        const visible = element ? injected.isVisible(element) : false;
+        let log = '';
+        if (elements.length > 1) {
+          if (info.strict)
+            throw injected.strictModeViolationError(info.parsed, elements);
+          log = `  locator resolved to ${elements.length} elements. Proceeding with the first one: ${injected.previewNode(elements[0])}`;
+        } else if (element) {
+          log = `  locator resolved to ${visible ? 'visible' : 'hidden'} ${injected.previewNode(element)}`;
         }
-        const element = state === 'attached' || state === 'visible' ? await result.evaluateHandle(r => r.element) : null;
+        return { log, element, visible, attached: !!element };
+      }, { info: resolved.info, root: resolved.frame === this ? scope : undefined });
+      const { log, visible, attached } = await result.evaluate(r => ({ log: r.log, visible: r.visible, attached: r.attached }));
+      if (log)
+        progress.log(log);
+      const success = { attached, detached: !attached, visible, hidden: !visible }[state];
+      if (!success) {
         result.dispose();
-        if (!element)
-          return null;
-        if ((options as any).__testHookBeforeAdoptNode)
-          await (options as any).__testHookBeforeAdoptNode();
-        try {
-          return await element._adoptTo(await resolved.frame._mainContext());
-        } catch (e) {
-          return continuePolling;
-        }
-      });
-      return scope ? scope._context._raceAgainstContextDestroyed(promise) : promise;
-    }, this._page._timeoutSettings.timeout(options));
+        return continuePolling;
+      }
+      if (options.omitReturnValue) {
+        result.dispose();
+        return null;
+      }
+      const element = state === 'attached' || state === 'visible' ? await result.evaluateHandle(r => r.element) : null;
+      result.dispose();
+      if (!element)
+        return null;
+      if ((options as any).__testHookBeforeAdoptNode)
+        await (options as any).__testHookBeforeAdoptNode();
+      try {
+        return await element._adoptTo(await resolved.frame._mainContext());
+      } catch (e) {
+        return continuePolling;
+      }
+    });
+    return scope ? scope._context._raceAgainstContextDestroyed(promise) : promise;
   }
 
   async dispatchEvent(metadata: CallMetadata, selector: string, type: string, eventInit: Object = {}, options: types.QueryOnSelectorOptions = {}, scope?: dom.ElementHandle): Promise<void> {