Fix handler wrapping for keyword-only parameters

[rohan-patnaik]

Fixes #3067

Summary

This fixes handler wrapping so keyword-only parameters are not counted as positional handler arguments.

Why

Before this change, a handler like locator.blur was treated as if it accepted the triggering locator because it has a keyword-only timeout option. Playwright then passed the locator positionally and crashed with TypeError.

Test plan

• python3 -m pytest tests/common/test_impl_to_api_mapping.py
• python3 -m black --check playwright/_impl/_impl_to_api_mapping.py tests/common/test_impl_to_api_mapping.py
• python3 -m mypy playwright/_impl/_impl_to_api_mapping.py

[rohan-patnaik]

@microsoft-github-policy-service agree

[biswajeetdev]

The fix correctly identifies the problem: inspect.signature(handler).parameters includes KEYWORD_ONLY and VAR_KEYWORD entries in its count, so a handler like def blur(self, *, timeout=None) was getting counted as having 2 parameters and receiving a spurious positional arg it could not accept.

The new logic — counting only POSITIONAL_ONLY and POSITIONAL_OR_KEYWORD params, and falling back to len(args) when *args is present — is exactly right.

Two small suggestions:

1. Add a test for the *args / has_varargs branch

The current test covers keyword-only params, but the has_varargs path is not exercised:

def test_wrap_handler_passes_all_args_for_varargs_handler() -> None:
    calls = []

    def handler(*args):
        calls.append(args)

    ImplToApiMapping().wrap_handler(handler)("a", "b", "c")
    assert len(calls[0]) == 3

2. POSITIONAL_ONLY params in practice

Python's POSITIONAL_ONLY kind (params before /) is rare in pure-Python code but common in C-extension signatures and some inspect-wrapped callables. The fix correctly includes them — just noting it is a good edge case to keep in mind if future test fixtures include C-extension handlers.

[rohan-patnaik]

Addressed the review suggestion by adding coverage for the *args / has_varargs path in wrap_handler.

[rohan-patnaik]

@Skn0tt This PR solves the #3067 , I addressed the review feedback above by adding coverage for the *args path and verified the focused tests/type checks. Would you be able to take a look when you have a chance?

[Skn0tt]

The change itself looks good, could you turn the test into an e2e test in tests/async/test_page_add_locator_handler.py and tests/sync/test_page_add_locator_handler.py?

[rohan-patnaik]

Addressed, thanks. I moved the handler coverage from the lower-level tests/common/test_impl_to_api_mapping.py test into e2e coverage in both:

-tests/async/test_page_add_locator_handler.py
-tests/sync/test_page_add_locator_handler.py

The new tests cover keyword-only handlers and the *args path through page.add_locator_handler.
Verified locally as well.