follow-up

Author: mxschmitt

README.md

@@ -4,7 +4,7 @@ Playwright is a Python library to automate [Chromium](https://www.chromium.org/H
 
 |          | Linux | macOS | Windows |
 |   :---   | :---: | :---: | :---:   |
-| Chromium <!-- GEN:chromium-version -->125.0.6422.14<!-- GEN:stop --> | ✅ | ✅ | ✅ |
+| Chromium <!-- GEN:chromium-version -->125.0.6422.26<!-- GEN:stop --> | ✅ | ✅ | ✅ |
 | WebKit <!-- GEN:webkit-version -->17.4<!-- GEN:stop --> | ✅ | ✅ | ✅ |
 | Firefox <!-- GEN:firefox-version -->125.0.1<!-- GEN:stop --> | ✅ | ✅ | ✅ |
 

playwright/_impl/_json_pipe.py

@@ -13,7 +13,7 @@
 # limitations under the License.
 
 import asyncio
-from typing import Dict, cast
+from typing import Dict, Optional, cast
 
 from pyee.asyncio import AsyncIOEventEmitter
 
@@ -54,9 +54,10 @@ def handle_message(message: Dict) -> None:
                 return
             self.on_message(cast(ParsedMessagePayload, message))
 
-        def handle_closed(reason: str) -> None:
+        def handle_closed(reason: Optional[str]) -> None:
             self.emit("close", reason)
-            self.on_error_future.set_exception(TargetClosedError(reason))
+            if reason:
+                self.on_error_future.set_exception(TargetClosedError(reason))
             self._stopped_future.set_result(None)
 
         self._pipe_channel.on(
@@ -65,7 +66,7 @@ def handle_closed(reason: str) -> None:
         )
         self._pipe_channel.on(
             "closed",
-            lambda params: handle_closed(params["reason"]),
+            lambda params: handle_closed(params.get("reason")),
         )
 
     async def run(self) -> None:

playwright/_impl/_page.py

@@ -1292,7 +1292,7 @@ async def set_checked(
     async def add_locator_handler(
         self,
         locator: "Locator",
-        handler: Callable,
+        handler: Union[Callable[["Locator"], Any], Callable[[], Any]],
         noWaitAfter: bool = None,
         times: int = None,
     ) -> None:

playwright/async_api/_generated.py

@@ -11723,7 +11723,9 @@ async def set_checked(
     async def add_locator_handler(
         self,
         locator: "Locator",
-        handler: typing.Callable,
+        handler: typing.Union[
+            typing.Callable[["Locator"], typing.Any], typing.Callable[[], typing.Any]
+        ],
         *,
         no_wait_after: typing.Optional[bool] = None,
         times: typing.Optional[int] = None
@@ -11818,7 +11820,7 @@ def handler(locator):
         ----------
         locator : Locator
             Locator that triggers the handler.
-        handler : Callable
+        handler : Union[Callable[[Locator], Any], Callable[[], Any]]
             Function that should be run once `locator` appears. This function should get rid of the element that blocks actions
             like click.
         no_wait_after : Union[bool, None]

playwright/sync_api/_generated.py

@@ -11808,7 +11808,9 @@ def set_checked(
     def add_locator_handler(
         self,
         locator: "Locator",
-        handler: typing.Callable,
+        handler: typing.Union[
+            typing.Callable[["Locator"], typing.Any], typing.Callable[[], typing.Any]
+        ],
         *,
         no_wait_after: typing.Optional[bool] = None,
         times: typing.Optional[int] = None
@@ -11903,7 +11905,7 @@ def handler(locator):
         ----------
         locator : Locator
             Locator that triggers the handler.
-        handler : Callable
+        handler : Union[Callable[[Locator], Any], Callable[[], Any]]
             Function that should be run once `locator` appears. This function should get rid of the element that blocks actions
             like click.
         no_wait_after : Union[bool, None]

scripts/expected_api_mismatch.txt

@@ -13,4 +13,5 @@ Parameter type mismatch in BrowserContext.unroute(handler=): documented as Union
 Parameter type mismatch in Page.route(handler=): documented as Callable[[Route, Request], Union[Any, Any]], code has Union[Callable[[Route, Request], Any], Callable[[Route], Any]]
 Parameter type mismatch in Page.unroute(handler=): documented as Union[Callable[[Route, Request], Union[Any, Any]], None], code has Union[Callable[[Route, Request], Any], Callable[[Route], Any], None]
 
-Parameter type mismatch in Page.add_locator_handler(handler=): documented as Callable[[Locator], Any], code has Callable
+# One vs two arguments in the callback, Python explicitly unions.
+Parameter type mismatch in Page.add_locator_handler(handler=): documented as Callable[[Locator], Any], code has Union[Callable[[Locator], Any], Callable[[], Any]]

tests/async/test_browsertype_connect_cdp.py

@@ -92,6 +92,7 @@ async def test_conect_over_a_ws_endpoint(
 async def test_connect_over_cdp_passing_header_works(
     browser_type: BrowserType, server: Server
 ) -> None:
+    server.send_on_web_socket_connection(b"incoming")
     request = asyncio.create_task(server.wait_for_request("/ws"))
     with pytest.raises(Error):
         await browser_type.connect_over_cdp(

tests/server.py

@@ -86,17 +86,17 @@ def process(self) -> None:
         uri = urlparse(self.uri.decode())
         path = uri.path
 
-        if path == "/ws":
-            server._ws_resource.render(self)
-            return
-
         request_subscriber = server.request_subscribers.get(path)
         if request_subscriber:
             request_subscriber._loop.call_soon_threadsafe(
                 request_subscriber.set_result, self
             )
             server.request_subscribers.pop(path)
 
+        if path == "/ws":
+            server._ws_resource.render(self)
+            return
+
         if server.auth.get(path):
             authorization_header = self.requestHeaders.getRawHeaders("authorization")
             creds_correct = False

tests/sync/test_page_add_locator_handler.py

@@ -12,9 +12,10 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+
 import pytest
 
-from playwright.sync_api import Error, Page, expect
+from playwright.sync_api import Error, Locator, Page, expect
 from tests.server import Server
 from tests.utils import TARGET_CLOSED_ERROR_MESSAGE
 
@@ -25,16 +26,18 @@ def test_should_work(page: Page, server: Server) -> None:
     before_count = 0
     after_count = 0
 
-    def handler() -> None:
+    original_locator = page.get_by_text("This interstitial covers the button")
+
+    def handler(locator: Locator) -> None:
+        nonlocal original_locator
+        assert locator == original_locator
         nonlocal before_count
         nonlocal after_count
         before_count += 1
         page.locator("#close").click()
         after_count += 1
 
-    page.add_locator_handler(
-        page.locator("text=This interstitial covers the button"), handler
-    )
+    page.add_locator_handler(original_locator, handler)
 
     for args in [
         ["mouseover", 1],
@@ -70,7 +73,7 @@ def handler() -> None:
         if page.get_by_text("This interstitial covers the button").is_visible():
             page.locator("#close").click()
 
-    page.add_locator_handler(page.locator("body"), handler)
+    page.add_locator_handler(page.locator("body"), handler, no_wait_after=True)
 
     for args in [
         ["mouseover", 2],
@@ -152,7 +155,7 @@ def handler() -> None:
         # Deliberately timeout.
         try:
             page.wait_for_timeout(9999999)
-        except Error:
+        except Exception:
             pass
 
     page.add_locator_handler(
@@ -195,3 +198,190 @@ def handler() -> None:
     expect(page.locator("#target")).to_be_visible()
     expect(page.locator("#interstitial")).not_to_be_visible()
     assert called == 1
+
+
+def test_should_work_when_owner_frame_detaches(page: Page, server: Server) -> None:
+    page.goto(server.EMPTY_PAGE)
+    page.evaluate(
+        """
+    () => {
+        const iframe = document.createElement('iframe');
+        iframe.src = 'data:text/html,<body>hello from iframe</body>';
+        document.body.append(iframe);
+
+        const target = document.createElement('button');
+        target.textContent = 'Click me';
+        target.id = 'target';
+        target.addEventListener('click', () => window._clicked = true);
+        document.body.appendChild(target);
+
+        const closeButton = document.createElement('button');
+        closeButton.textContent = 'close';
+        closeButton.id = 'close';
+        closeButton.addEventListener('click', () => iframe.remove());
+        document.body.appendChild(closeButton);
+    }
+    """
+    )
+    page.add_locator_handler(
+        page.frame_locator("iframe").locator("body"),
+        lambda: page.locator("#close").click(),
+    )
+    page.locator("#target").click()
+    assert page.query_selector("iframe") is None
+    assert page.evaluate("window._clicked") is True
+
+
+def test_should_work_with_times_option(page: Page, server: Server) -> None:
+    page.goto(server.PREFIX + "/input/handle-locator.html")
+    called = 0
+
+    def _handler() -> None:
+        nonlocal called
+        called += 1
+
+    page.add_locator_handler(
+        page.locator("body"), _handler, no_wait_after=True, times=2
+    )
+    page.locator("#aside").hover()
+    page.evaluate(
+        """
+    () => {
+        window.clicked = 0;
+        window.setupAnnoyingInterstitial('mouseover', 4);
+    }
+    """
+    )
+    with pytest.raises(Error) as exc_info:
+        page.locator("#target").click(timeout=3000)
+    assert called == 2
+    assert page.evaluate("window.clicked") == 0
+    expect(page.locator("#interstitial")).to_be_visible()
+    assert "Timeout 3000ms exceeded" in exc_info.value.message
+    assert (
+        '<div>This interstitial covers the button</div> from <div class="visible" id="interstitial">…</div> subtree intercepts pointer events'
+        in exc_info.value.message
+    )
+
+
+def test_should_wait_for_hidden_by_default(page: Page, server: Server) -> None:
+    page.goto(server.PREFIX + "/input/handle-locator.html")
+    called = 0
+
+    def _handler(button: Locator) -> None:
+        nonlocal called
+        called += 1
+        button.click()
+
+    page.add_locator_handler(page.get_by_role("button", name="close"), _handler)
+    page.locator("#aside").hover()
+    page.evaluate(
+        """
+    () => {
+        window.clicked = 0;
+        window.setupAnnoyingInterstitial('timeout', 1);
+    }
+    """
+    )
+    page.locator("#target").click()
+    assert page.evaluate("window.clicked") == 1
+    expect(page.locator("#interstitial")).not_to_be_visible()
+    assert called == 1
+
+
+def test_should_wait_for_hidden_by_default_2(page: Page, server: Server) -> None:
+    page.goto(server.PREFIX + "/input/handle-locator.html")
+    called = 0
+
+    def _handler() -> None:
+        nonlocal called
+        called += 1
+
+    page.add_locator_handler(page.get_by_role("button", name="close"), _handler)
+    page.locator("#aside").hover()
+    page.evaluate(
+        """
+    () => {
+        window.clicked = 0;
+        window.setupAnnoyingInterstitial('hide', 1);
+    }
+    """
+    )
+    with pytest.raises(Error) as exc_info:
+        page.locator("#target").click(timeout=3000)
+    assert page.evaluate("window.clicked") == 0
+    expect(page.locator("#interstitial")).to_be_visible()
+    assert called == 1
+    assert (
+        'locator handler has finished, waiting for get_by_role("button", name="close") to be hidden'
+        in exc_info.value.message
+    )
+
+
+def test_should_work_with_noWaitAfter(page: Page, server: Server) -> None:
+    page.goto(server.PREFIX + "/input/handle-locator.html")
+    called = 0
+
+    def _handler(button: Locator) -> None:
+        nonlocal called
+        called += 1
+        if called == 1:
+            button.click()
+        else:
+            page.locator("#interstitial").wait_for(state="hidden")
+
+    page.add_locator_handler(
+        page.get_by_role("button", name="close"), _handler, no_wait_after=True
+    )
+    page.locator("#aside").hover()
+    page.evaluate(
+        """
+    () => {
+        window.clicked = 0;
+        window.setupAnnoyingInterstitial('timeout', 1);
+    }
+    """
+    )
+    page.locator("#target").click()
+    assert page.evaluate("window.clicked") == 1
+    expect(page.locator("#interstitial")).not_to_be_visible()
+    assert called == 2
+
+
+def test_should_removeLocatorHandler(page: Page, server: Server) -> None:
+    page.goto(server.PREFIX + "/input/handle-locator.html")
+    called = 0
+
+    def _handler(locator: Locator) -> None:
+        nonlocal called
+        called += 1
+        locator.click()
+
+    page.add_locator_handler(page.get_by_role("button", name="close"), _handler)
+    page.evaluate(
+        """
+    () => {
+        window.clicked = 0;
+        window.setupAnnoyingInterstitial('hide', 1);
+    }
+    """
+    )
+    page.locator("#target").click()
+    assert called == 1
+    assert page.evaluate("window.clicked") == 1
+    expect(page.locator("#interstitial")).not_to_be_visible()
+    page.evaluate(
+        """
+    () => {
+        window.clicked = 0;
+        window.setupAnnoyingInterstitial('hide', 1);
+    }
+    """
+    )
+    page.remove_locator_handler(page.get_by_role("button", name="close"))
+    with pytest.raises(Error) as error:
+        page.locator("#target").click(timeout=3000)
+    assert called == 1
+    assert page.evaluate("window.clicked") == 0
+    expect(page.locator("#interstitial")).to_be_visible()
+    assert "Timeout 3000ms exceeded" in error.value.message