From 3f222f142b73cfafb3d0bd1b00f9c749b123f688 Mon Sep 17 00:00:00 2001 From: Rohan Patnaik Date: Fri, 22 May 2026 12:45:58 +0530 Subject: [PATCH 1/3] fix: ignore keyword-only handler parameters --- playwright/_impl/_impl_to_api_mapping.py | 18 ++++++++++++++- tests/common/test_impl_to_api_mapping.py | 29 ++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 tests/common/test_impl_to_api_mapping.py diff --git a/playwright/_impl/_impl_to_api_mapping.py b/playwright/_impl/_impl_to_api_mapping.py index e26d22025..8b3cbd756 100644 --- a/playwright/_impl/_impl_to_api_mapping.py +++ b/playwright/_impl/_impl_to_api_mapping.py @@ -119,7 +119,23 @@ def to_impl( def wrap_handler(self, handler: Callable[..., Any]) -> Callable[..., None]: def wrapper_func(*args: Any) -> Any: - arg_count = len(inspect.signature(handler).parameters) + parameters = inspect.signature(handler).parameters + has_varargs = any( + parameter.kind == inspect.Parameter.VAR_POSITIONAL + for parameter in parameters.values() + ) + arg_count = ( + len(args) + if has_varargs + else sum( + parameter.kind + in ( + inspect.Parameter.POSITIONAL_ONLY, + inspect.Parameter.POSITIONAL_OR_KEYWORD, + ) + for parameter in parameters.values() + ) + ) return handler( *list(map(lambda a: self.from_maybe_impl(a), args))[:arg_count] ) diff --git a/tests/common/test_impl_to_api_mapping.py b/tests/common/test_impl_to_api_mapping.py new file mode 100644 index 000000000..74142bc3f --- /dev/null +++ b/tests/common/test_impl_to_api_mapping.py @@ -0,0 +1,29 @@ +# Copyright (c) Microsoft Corporation. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from typing import List, Optional + +from playwright._impl._impl_to_api_mapping import ImplToApiMapping + + +def test_wrap_handler_ignores_keyword_only_parameters() -> None: + calls: List[Optional[int]] = [] + + class Input: + def blur(self, *, timeout: Optional[int] = None) -> None: + calls.append(timeout) + + ImplToApiMapping().wrap_handler(Input().blur)("locator") + + assert calls == [None] From 7ef31fb4e21cb4952415c552f3237f06a7f74ae1 Mon Sep 17 00:00:00 2001 From: Rohan Patnaik Date: Tue, 2 Jun 2026 04:12:39 +0530 Subject: [PATCH 2/3] test: cover varargs handler wrapping --- tests/common/test_impl_to_api_mapping.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/common/test_impl_to_api_mapping.py b/tests/common/test_impl_to_api_mapping.py index 74142bc3f..387ae1cc6 100644 --- a/tests/common/test_impl_to_api_mapping.py +++ b/tests/common/test_impl_to_api_mapping.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -from typing import List, Optional +from typing import List, Optional, Tuple from playwright._impl._impl_to_api_mapping import ImplToApiMapping @@ -27,3 +27,14 @@ def blur(self, *, timeout: Optional[int] = None) -> None: ImplToApiMapping().wrap_handler(Input().blur)("locator") assert calls == [None] + + +def test_wrap_handler_passes_all_args_for_varargs_handler() -> None: + calls: List[Tuple[object, ...]] = [] + + def handler(*args: object) -> None: + calls.append(args) + + ImplToApiMapping().wrap_handler(handler)("a", "b", "c") + + assert calls == [("a", "b", "c")] From 92a0d8c20ddbec64c3de21b3293bc6e8c28474f0 Mon Sep 17 00:00:00 2001 From: Rohan Patnaik Date: Mon, 8 Jun 2026 16:11:49 +0530 Subject: [PATCH 3/3] test: move locator handler coverage to e2e tests --- tests/async/test_page_add_locator_handler.py | 51 ++++++++++++++++++++ tests/common/test_impl_to_api_mapping.py | 40 --------------- tests/sync/test_page_add_locator_handler.py | 49 +++++++++++++++++++ 3 files changed, 100 insertions(+), 40 deletions(-) delete mode 100644 tests/common/test_impl_to_api_mapping.py diff --git a/tests/async/test_page_add_locator_handler.py b/tests/async/test_page_add_locator_handler.py index 5f44170f8..ba798692e 100644 --- a/tests/async/test_page_add_locator_handler.py +++ b/tests/async/test_page_add_locator_handler.py @@ -91,6 +91,57 @@ async def handler() -> None: await expect(page.locator("#interstitial")).not_to_be_visible() +async def test_should_work_with_keyword_only_handler( + page: Page, server: Server +) -> None: + await page.goto(server.PREFIX + "/input/handle-locator.html") + + called = False + + async def handler(*, timeout: object = None) -> None: + nonlocal called + assert timeout is None + called = True + await page.locator("#close").click() + + await page.add_locator_handler( + page.get_by_text("This interstitial covers the button"), handler + ) + + await page.locator("#aside").hover() + await page.evaluate( + '() => { window.clicked = 0; window.setupAnnoyingInterstitial("mouseover", 1); }' + ) + await page.locator("#target").click() + assert called + assert await page.evaluate("window.clicked") == 1 + await expect(page.locator("#interstitial")).not_to_be_visible() + + +async def test_should_work_with_varargs_handler(page: Page, server: Server) -> None: + await page.goto(server.PREFIX + "/input/handle-locator.html") + + called = False + original_locator = page.get_by_text("This interstitial covers the button") + + async def handler(*locators: Locator) -> None: + nonlocal called + assert locators == (original_locator,) + called = True + await page.locator("#close").click() + + await page.add_locator_handler(original_locator, handler) + + await page.locator("#aside").hover() + await page.evaluate( + '() => { window.clicked = 0; window.setupAnnoyingInterstitial("mouseover", 1); }' + ) + await page.locator("#target").click() + assert called + assert await page.evaluate("window.clicked") == 1 + await expect(page.locator("#interstitial")).not_to_be_visible() + + async def test_should_work_with_locator_hover(page: Page, server: Server) -> None: await page.goto(server.PREFIX + "/input/handle-locator.html") diff --git a/tests/common/test_impl_to_api_mapping.py b/tests/common/test_impl_to_api_mapping.py deleted file mode 100644 index 387ae1cc6..000000000 --- a/tests/common/test_impl_to_api_mapping.py +++ /dev/null @@ -1,40 +0,0 @@ -# Copyright (c) Microsoft Corporation. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -from typing import List, Optional, Tuple - -from playwright._impl._impl_to_api_mapping import ImplToApiMapping - - -def test_wrap_handler_ignores_keyword_only_parameters() -> None: - calls: List[Optional[int]] = [] - - class Input: - def blur(self, *, timeout: Optional[int] = None) -> None: - calls.append(timeout) - - ImplToApiMapping().wrap_handler(Input().blur)("locator") - - assert calls == [None] - - -def test_wrap_handler_passes_all_args_for_varargs_handler() -> None: - calls: List[Tuple[object, ...]] = [] - - def handler(*args: object) -> None: - calls.append(args) - - ImplToApiMapping().wrap_handler(handler)("a", "b", "c") - - assert calls == [("a", "b", "c")] diff --git a/tests/sync/test_page_add_locator_handler.py b/tests/sync/test_page_add_locator_handler.py index 7a2b6a438..047b4a775 100644 --- a/tests/sync/test_page_add_locator_handler.py +++ b/tests/sync/test_page_add_locator_handler.py @@ -90,6 +90,55 @@ def handler() -> None: expect(page.locator("#interstitial")).not_to_be_visible() +def test_should_work_with_keyword_only_handler(page: Page, server: Server) -> None: + page.goto(server.PREFIX + "/input/handle-locator.html") + + called = False + + def handler(*, timeout: object = None) -> None: + nonlocal called + assert timeout is None + called = True + page.locator("#close").click() + + page.add_locator_handler( + page.get_by_text("This interstitial covers the button"), handler + ) + + page.locator("#aside").hover() + page.evaluate( + '() => { window.clicked = 0; window.setupAnnoyingInterstitial("mouseover", 1); }' + ) + page.locator("#target").click() + assert called + assert page.evaluate("window.clicked") == 1 + expect(page.locator("#interstitial")).not_to_be_visible() + + +def test_should_work_with_varargs_handler(page: Page, server: Server) -> None: + page.goto(server.PREFIX + "/input/handle-locator.html") + + called = False + original_locator = page.get_by_text("This interstitial covers the button") + + def handler(*locators: Locator) -> None: + nonlocal called + assert locators == (original_locator,) + called = True + page.locator("#close").click() + + page.add_locator_handler(original_locator, handler) + + page.locator("#aside").hover() + page.evaluate( + '() => { window.clicked = 0; window.setupAnnoyingInterstitial("mouseover", 1); }' + ) + page.locator("#target").click() + assert called + assert page.evaluate("window.clicked") == 1 + expect(page.locator("#interstitial")).not_to_be_visible() + + def test_should_work_with_locator_hover(page: Page, server: Server) -> None: page.goto(server.PREFIX + "/input/handle-locator.html")