-
Notifications
You must be signed in to change notification settings - Fork 19
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.
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
Broken keyboard input implementation #124
Comments
@lino-levan what is a realistic expectation for when this bug might be resolved? |
I can try to prioritize working on this today. Planning to take some time off work to get Astral into much better shape, both in terms of fixing issues and documentation. |
Unfortunately, my test suit that passes with Puppeteer still doesn't pass with Astral v0.5.2: // Test holding the alt key when clicking the link prevents a
// client side navigation…
await page.keyboard.down("Alt");
await page.locator('a[href="/b"]').click();
await page.keyboard.up("Alt");
assertStrictEquals(await browserPageUrl(page), urlPageA); The assertion fails that a navigation did occur. Do you think it's a bug with Astral, or do I need to use different capitalisation with |
Is there some way for you to write a keyboard testcase that I can work on passing? Here's an example of a testcase in the codebaseDeno.test("Keyboard - tab navigation", async () => {
const browser = await launch();
const page = await browser.newPage();
await page.setContent(`
<!DOCTYPE html>
<html>
<body>
<input id="first" type="text" placeholder="First input">
<input id="second" type="text" placeholder="Second input">
<button id="button">Click me</button>
<textarea id="textarea">Text area</textarea>
<div id="focused"></div>
<script>
const focused = document.getElementById('focused');
document.addEventListener('focusin', (e) => {
if (e.target.id) {
focused.textContent = (focused.textContent || '') + e.target.id + ',';
}
});
</script>
</body>
</html>
`);
// Start with first input focused
const firstInput = await page.$("input#first");
assertExists(firstInput);
await firstInput.click();
// Press tab multiple times to move through elements
await page.keyboard.press("Tab");
await page.keyboard.press("Tab");
await page.keyboard.press("Tab");
// Check the focus order
const focusOrder = await page.evaluate(() =>
document.getElementById("focused")?.textContent || ""
);
assertEquals(focusOrder, "first,second,button,textarea,");
await browser.close();
}); Would also be good for regression testing. |
I think the thing to test is that mouse events like click have https://github.com/search?q=repo%3Alino-levan%2Fastral%20dispatchMouseEvent&type=code It seems none of the event details include the keyboard modifier properties like Lines 47 to 53 in 348dda3
|
Yes: https://chromedevtools.github.io/devtools-protocol/tot/Input/#method-dispatchMouseEvent I wonder if touch events, and other input events, need attention too. |
Very likely the case, this'll be fun to work through. |
They are failing due to an upstream Astral issue: lino-levan/astral#124 (comment)
If you do
await page.keyboard.down("Alt")
, then thiskeydown
listener will fire:Incorrectly however,
event.key
will be""
.At this point, because there was not yet
await page.keyboard.up("Alt")
, when you do.click()
on an element in the page, the click event propertyaltKey
should betrue
. But, incorrectly it'sfalse
.This test passes when using Puppeteer:
https://github.com/jaydenseric/ruck/blob/aa068afa7f0630c4e3e8617b2936210c14815964/useOnClickRouteLink.test.mjs#L121-L126
Here is the source code being tested:
https://github.com/jaydenseric/ruck/blob/aa068afa7f0630c4e3e8617b2936210c14815964/useOnClickRouteLink.mjs#L31
But with Astral, the test incorrectly fails when it should pass.
Here is the relevant code in Astral vs Puppeteer:
astral/src/keyboard.ts
Lines 278 to 287 in 0a4ed05
https://github.com/puppeteer/puppeteer/blob/83449567c7013ae1c87c54e42501aa74575640d0/packages/puppeteer-core/src/cdp/Input.ts#L55-L82
The text was updated successfully, but these errors were encountered: