feat: support for different keyboard layouts

[ruifigueira]

Layouts were generated from scraping kbdlayout.info (see generate_keyboard_layouts.js).

For locale mapping, see #7396 (comment)

There are some things to improve:

• how to select the keyboard layout (I left an environment variable PW_KEYBOARD_LAYOUT but I think it probably should be a browserContext option
• better testing of non-trivial keyboard layouts
• documentation

Refs: #7396

[mxschmitt]

Hey! Thank you for your great contribution!

We discussed it in our team meeting and would like to just go with the 5 most common keyboard layouts instead of all of them for the beginning. We can follow up for more (bundle size is our reasoning for now). Also it would be cool if its possible to find another way of getting the data for our generation. Since scraping is not ideal.

[ruifigueira]

@mxschmitt do you already know which keyboard layouts you want to start with, so that I can adjust everything? (tests. documentation, etc.)

[mxschmitt]

@mxschmitt do you already know which keyboard layouts you want to start with, so that I can adjust everything? (tests. documentation, etc.)

This would be our preferred list:

US English
Arabic
British
Danish
French
German
Italian
Portuguese
Russian
Ukrainian
Spanish & Catalan (Spain)
Spanish (Latin America)
Swiss

thanks!

[ruifigueira]

Arabic is tricky, because it contains ligatures (see #24249 (comment)). The generator extracts them, but as they are not a multi-unicode characters, they are not yet handled properly by current playwright implementation.

I did some tests and it doesn't seem to have a trivial solution, so I suggest we open an issue for ligature characters and address arabic keyboard layouts after that.

For reference, here are the actual key events sent when pressing Shift+KeyT with an arabic layout at https://w3c.github.io/uievents/tools/key-event-viewer.html (notice the two keypress events, one for each unicode character of the ligature):

[mxschmitt]

Great investigation! Actually I added it by mistake, since as you said blocked by #5777. Feel free to leave it out.

[ruifigueira]

Here is the list, with their codes (I'm using both locale and linux layout codes).

Values	Name
us, en-US	US English
gb, en-GB	British
dk, da-DK	Danish
fr, fr-FR	French
de, de-DE	German
it, it-IT	Italian
pt, pt-PT	Portuguese (Portugal)
br, pt-BR	Portuguese (Brazil)
ru, ru-RU	Russian
ua, uk-UA	Ukrainian
es, es-ES, ca-ES	Spanish & Catalan (Spain)
latam	Spanish (Latin America)
ch, de-CH	German (Switzerland)
fr-CH, it-CH	French and Italian (Switzerland)

[ruifigueira]

I'm doing some deadkey tests. There are some discrepancies among browsers: for instance, with Portuguese keyboard layout and Windows 11, key BracketRight, which is a deadkey, has a keyCode=186 in both chromium and webkit, but has keyCode=192 in firefox.

chromium

firefox

Nevertheless, keyCode is a deprecated property, so maybe we can leave it this way?

[ruifigueira]

Another doubt: reading keyboard.press documentation, I get the sense that it should not handle accented keys, because it normally means pressing keys and hold them while pressing others. For accented keys, this is not true: we first press the dead key, and then press the unaccented key. For instance, á in portuguese corresponds to press BracketRight (a dead key), and then pressing a. And the same rationale applies for down and up.

I think we could handle them propertly in keyboard.type, though, For instance, Olá in portuguese would send the following instructions: press('O'), press('l'), press('BracketRight') and press('a').

Let me know your thoughts.

[ruifigueira]

Thanks @dgozman. I just rebased my branch,removed those extra commits trying to fix the unmapped deadkey, and adapted the code not to fail in that scenario. Now, pressing BracketRight followed by KeyT will just type t and ignore the deadkey, as you proposed.

[dgozman]

Sorry, I forgot to publish review comments!

[github-actions]

Test results for "tests 1"

6 flaky
⚠️ [chromium] › library/tracing.spec.ts:239:5 › should not include trace resources from the previous chunks
⚠️ [chromium] › page/page-event-request.spec.ts:101:3 › should report navigation requests and responses handled by service worker
⚠️ [chromium] › library/tracing.spec.ts:239:5 › should not include trace resources from the previous chunks
⚠️ [webkit] › library/browsercontext-reuse.spec.ts:50:1 › should reset serviceworker
⚠️ [webkit] › library/trace-viewer.spec.ts:131:1 › should render network bars
⚠️ [playwright-test] › ui-mode-test-progress.spec.ts:167:5 › should update tracing network live

26207 passed, 595 skipped
✔️✔️✔️

Merge workflow run.

[github-actions]

Test results for "tests 1"

11 flaky
⚠️ [chromium] › library/tracing.spec.ts:239:5 › should not include trace resources from the previous chunks
⚠️ [chromium] › page/page-event-request.spec.ts:130:3 › should report navigation requests and responses handled by service worker with routing
⚠️ [chromium] › library/tracing.spec.ts:239:5 › should not include trace resources from the previous chunks
⚠️ [chromium] › library/tracing.spec.ts:395:14 › should produce screencast frames crop
⚠️ [chromium] › library/tracing.spec.ts:239:5 › should not include trace resources from the previous chunks
⚠️ [chromium] › page/page-event-request.spec.ts:130:3 › should report navigation requests and responses handled by service worker with routing
⚠️ [firefox] › library/proxy.spec.ts:179:3 › should exclude patterns
⚠️ [chromium] › page/page-event-request.spec.ts:101:3 › should report navigation requests and responses handled by service worker
⚠️ [webkit] › library/browsercontext-reuse.spec.ts:50:1 › should reset serviceworker
⚠️ [webkit] › library/tracing.spec.ts:469:5 › should work with multiple chunks
⚠️ [webkit] › page/page-goto.spec.ts:266:3 › should fail when navigating to bad SSL

25722 passed, 595 skipped
✔️✔️✔️

Merge workflow run.

.

[dgozman]

This looks great, just a few more comments.

[github-actions]

Test results for "tests 1"

23 passed
✔️✔️✔️

Merge workflow run.

[github-actions]

Test results for "tests 1"

1 failed
❌ [playwright-test] › reporter-html.spec.ts:1491:7 › merged › labels › filter should update stats

6 flaky
⚠️ [chromium] › library/tracing.spec.ts:239:5 › should not include trace resources from the previous chunks
⚠️ [chromium] › library/tracing.spec.ts:239:5 › should not include trace resources from the previous chunks
⚠️ [chromium] › library/tracing.spec.ts:239:5 › should not include trace resources from the previous chunks
⚠️ [chromium] › page/page-event-request.spec.ts:101:3 › should report navigation requests and responses handled by service worker
⚠️ [chromium] › library/inspector/cli-codegen-pytest.spec.ts:56:5 › should save the codegen output to a file if specified
⚠️ [playwright-test] › ui-mode-test-progress.spec.ts:22:5 › should update trace live

25756 passed, 595 skipped
✔️✔️✔️

❌ [playwright-test] › reporter-html.spec.ts:1491:7 › merged › labels › filter should update stats

Error: expect(received).toBe(expected) // Object.is equality

Expected: "Total time: 1419ms"
Received: "Total time: 1.4s"

  1524 |           }
  1525 |           const totalDuration = await page.getByTestId('overall-duration').textContent();
> 1526 |           expect(totalDuration).toBe(`Total time: ${total}ms`);
       |                                 ^
  1527 |         }
  1528 |
  1529 |         const searchInput = page.locator('.subnav-search-input');

    at checkTotalDuration (D:\a\playwright\playwright\tests\playwright-test\reporter-html.spec.ts:1526:33)
    at D:\a\playwright\playwright\tests\playwright-test\reporter-html.spec.ts:1535:9

Merge workflow run.

[github-actions]

Test results for "tests 1"

1 failed
❌ [chromium] › library/tracing.spec.ts:239:5 › should not include trace resources from the previous chunks

9 flaky

⚠️ [chromium] › page/page-event-request.spec.ts:101:3 › should report navigation requests and responses handled by service worker
⚠️ [chromium] › page/page-event-request.spec.ts:101:3 › should report navigation requests and responses handled by service worker
⚠️ [firefox] › library/fetch-proxy.spec.ts:30:3 › context request should pick up proxy credentials
⚠️ [chromium] › library/tracing.spec.ts:239:5 › should not include trace resources from the previous chunks
⚠️ [chromium] › page/page-event-request.spec.ts:101:3 › should report navigation requests and responses handled by service worker
⚠️ [webkit] › library/browsercontext-reuse.spec.ts:50:1 › should reset serviceworker
⚠️ [webkit] › library/inspector/cli-codegen-python-async.spec.ts:82:5 › should save the codegen output to a file if specified
⚠️ [webkit] › page/page-goto.spec.ts:277:3 › should fail when navigating to bad SSL after redirects
⚠️ [webkit] › page/page-wait-for-navigation.spec.ts:85:3 › should work with clicking on links which do not commit navigation

25819 passed, 605 skipped
✔️✔️✔️

Merge workflow run.

[ruifigueira]

@dgozman @mxschmitt did you had a chance to review this? I noticed that 1.38 was released already, so at least I'll have to change versions in documentation.

[dgozman]

@ruifigueira This looks good. I was waiting for v1.38 to ship, so this can go to v1.39. Please update since fields and I'll merge this in. Thank you!

[ruifigueira]

Done, updated to 1.39.

[github-actions]

Test results for "tests 1"

1 failed
❌ [playwright-test] › timeout.spec.ts:270:5 › fixture timeout in beforeAll hook should not affect test

10 flaky

⚠️ [chromium] › library/tracing.spec.ts:239:5 › should not include trace resources from the previous chunks
⚠️ [chromium] › library/tracing.spec.ts:239:5 › should not include trace resources from the previous chunks
⚠️ [chromium] › library/tracing.spec.ts:395:14 › should produce screencast frames crop
⚠️ [chromium] › library/tracing.spec.ts:239:5 › should not include trace resources from the previous chunks
⚠️ [chromium] › page/page-event-request.spec.ts:101:3 › should report navigation requests and responses handled by service worker
⚠️ [chromium] › library/tracing.spec.ts:239:5 › should not include trace resources from the previous chunks
⚠️ [webkit] › library/browsercontext-reuse.spec.ts:50:1 › should reset serviceworker
⚠️ [webkit] › page/page-goto.spec.ts:266:3 › should fail when navigating to bad SSL
⚠️ [webkit] › page/page-wait-for-navigation.spec.ts:85:3 › should work with clicking on links which do not commit navigation
⚠️ [playwright-test] › ui-mode-test-progress.spec.ts:167:5 › should update tracing network live

25825 passed, 606 skipped
✔️✔️✔️

Merge workflow run.

[ruifigueira]

@dgozman any updates?

[dgozman]

Short summary

We've discussed this feature once more, and we'd prefer to postpone it for now, until we gather more feedback on type() vs fill(). @ruifigueira, I am sorry for being indecisive, and very much thankful for all the work you've put in!

More details

Your work has prompted us to make changes to the type() vs fill() methods in v1.38, and stronger encourage everyone to use fill() whenever possible. That's what we recommended from the start, but we never articulated it clearly enough. Now, with the pressSequentially() instead of type(), we expect much more usage of fill() instead.

Given that keyboard layouts feature is a smaller subset of type() usage, that we already think should be small enough, we'd like a strong signal before committing to maintaining the new API. This does not mean that your PR or proposed API are in some way flawed. In general, the more niche and complicated the API is, the less inclined we are to introduce it.

Next steps

With the next 1.39 release, I'll ask for more feedback on the usecases for type/pressSequentially usage and keyboard layouts support among our users. Then we'll re-asses whether the feature would be worth releasing. For now, I'd recommend to park this PR so that we can revisit it later. Again, thank you for the work you've done, and sorry for all the time you have put into the feature that we are not releasing yet.

[ruifigueira]

@dgozman I noticed that API change and I actually thought what would be the impact in this PR, but I wasn't expecting this, after all the work I had 😔.

I understand your explanation, but that still leaves a void on trusted key events for any character not available in US keyboard layout, similar to IME events, but with the difference that playwright internally already has a way to have full keyboard events support.

Currently, we can do it low level by creating a CDP session and use the Input.dispatchKeyEvent method, but that only works on chromium. Both webkit and firefox sessions are not available in client API (and even if they were, their protocols appear to be internal), and so there's no chance to fire trusted events there for non-US characters there.

I honestly don't have an alternative proposal, but playwright is able to do so many incredible things like intercepting requests and change them on the fly, that it just feels odd that it cannot fire some basic trusted events for non-US keys.

[dgozman]

@ruifigueira Thank you for working on this PR! Unfortunately, we decided to close it for now, since we do not think that implementing this feature is worth the maintenance cost at the moment. Given the update in #7396 and no strong reactions to this call, it seems like there is not enough interest in this.

I am sorry that we haven't researched this much earlier, and so you and me spent more time on this PR than necessary. That said, I hope that we learned a few things along the way 😄

Again, thank you for the great work you've done! I am looking forward to more collaboration if you'd like.

[ruifigueira]

I understand, and I actually noticed that it was more complex than I initially thought (for instance, different events in Windows and MacOS on dead keys). And I did learn a lot on the way playwright works internally, which was great. No hard feelings.

That been said, it would be great if there was a way to generate custom key / composition events, even if with a low level API like the chromium CDP session. As I said previously, there are some scenarios where we need to test some trusted key events and there's no way to do that in firefox and webkit.

[dgozman]

That been said, it would be great if there was a way to generate custom key / composition events

I think discussion about composition events should be in #5777.