Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore(bidi): propertly dispatch ControlRight key event #34246

Merged
merged 1 commit into from
Jan 8, 2025
Merged

chore(bidi): propertly dispatch ControlRight key event #34246

merged 1 commit into from
Jan 8, 2025
+13 −5

Conversation

yury-s
Copy link
Member

@yury-s yury-s commented Jan 7, 2025

No description provided.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 8, 2025

Test results for "tests 1"

3 failed
❌ [installation tests] › tests/playwright-electron-should-work.spec.ts:21:5 › electron should work @package-installations-ubuntu-latest
❌ [installation tests] › tests/playwright-electron-should-work.spec.ts:31:5 › electron should work with special characters in path @package-installations-ubuntu-latest
❌ [installation tests] › tests/playwright-electron-should-work.spec.ts:44:5 › should work when wrapped inside @playwright/test and trace is enabled @package-installations-ubuntu-latest

6 flaky ⚠️ [firefox-page] › tests/page/page-evaluate.spec.ts:403:3 › should throw for too deep reference chain @firefox-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/browsercontext-clearcookies.spec.ts:116:3 › should remove cookies by path @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/browsercontext-reuse.spec.ts:50:1 › should reset serviceworker @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/selector-generator.spec.ts:265:5 › selector generator › should use nested ordinals @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/trace-viewer.spec.ts:149:1 › should open simple trace viewer @webkit-ubuntu-22.04-node18
⚠️ [webkit-page] › tests/page/page-set-input-files.spec.ts:205:3 › should upload multiple large files @webkit-ubuntu-22.04-node18

37518 passed, 649 skipped
✔️✔️✔️

Merge workflow run.

@yury-s yury-s merged commit 6fb6282 into microsoft:main Jan 8, 2025
27 of 31 checks passed
@yury-s yury-s deleted the bidi-control-right branch January 8, 2025 00:46
whimboo
whimboo reviewed Jan 8, 2025
View reviewed changes
packages/playwright-core/src/server/bidi/bidiInput.ts
@@ -33,7 +33,7 @@ export class RawKeyboardImpl implements input.RawKeyboard {

async keydown(modifiers: Set<types.KeyboardModifier>, code: string, keyCode: number, keyCodeWithoutLocation: number, key: string, location: number, autoRepeat: boolean, text: string | undefined): Promise<void> {
const actions: bidi.Input.KeySourceAction[] = [];
actions.push({ type: 'keyDown', value: getBidiKeyValue(key) });
actions.push({ type: 'keyDown', value: getBidiKeyValue(code) });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall getBidiKeyValue = (key: string) be as well get updated to reflect that code has to be passed in and not key? That explains why I didn't find the problem. Thanks for fixing it!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, renaming makes sense, feel free to send a PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(the function has been copied from puppeteer)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@whimboo whimboo whimboo left review comments

@pavelfeldman pavelfeldman pavelfeldman approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@yury-s @whimboo @pavelfeldman