Input.dispatchKeyEvent should wait for events to be fired in the content process
Categories
(Remote Protocol :: Agent, enhancement, P1)
Tracking
(Not tracked)
People
(Reporter: jdescottes, Assigned: jdescottes)
References
Details
Attachments
(4 files, 3 obsolete files)
In the initial implementation of Input.dispatchKeyEvent in Bug 1543142, we did the implementation completely in the parent process, because events created in the parent process are closer to real/trusted user events.
However, this means we are waiting for the appropriate dom event in the parent process as well. And this can lead to race conditions in the tests.
Typically if the puppeteer test is doing:
await page.type(cssSelector, someText);
const actualValue = await page.evaluate(() => getValueFromContentPage());
// assert actualValue === expected value
If the content page is listening to an input's change
or input
event in order to update its internal state and if the handler takes too much time to process the event, then await page.type
will resolve before the content page has fully processed the event.
Waiting for the event in the content process instead of the parent process fixes the issue, at least according to a quick test that moved the implementation of dispatchKeyInput to the content Input domain.
The API is not clear about when we should actually resolve dispatchKeyEvent, but that seems to match Chrome's behavior here.
Now for the actual fix, we either can move the whole implementation to the content process, but we know this will limit the kind of keys we can simulate. Or we reintroduce a mix of parent and content process. We can initiate the call in the parent process and only listen to the event in the content process. Would probably be easier if our domain design allowed for an easy communication from parent to content.
Comment 1•5 years ago
|
||
Could we use docShell.chromeEventHandler
for this?
It might still be a good idea to spin the event loop after waiting
for the precise content DOM events.
Assignee | ||
Comment 2•5 years ago
|
||
(In reply to Andreas Tolfsen 「:ato」 from comment #1)
Could we use
docShell.chromeEventHandler
for this?
Do you mean from the content process? If so, then yes that's what we would use (or at least that's what I tried, and it worked :) ).
It might still be a good idea to spin the event loop after waiting
for the precise content DOM events.
I agree, it's probably best to do both.
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 3•5 years ago
|
||
Depends on D36678
Updated•5 years ago
|
Assignee | ||
Comment 4•5 years ago
|
||
Depends on D36626
In theory the mochitest no longer needs to wait for so many events
Assignee | ||
Comment 5•5 years ago
|
||
Depends on D36727
Assignee | ||
Comment 6•5 years ago
•
|
||
try at https://treeherder.mozilla.org/#/jobs?repo=try&revision=3a31c3c99019dae280aad70224bf569b199b598e
(leaks in the previous try run)
New try run at: https://treeherder.mozilla.org/#/jobs?repo=try&revision=534d7239ffa8b957c93353edea35e42eceb64263
Assignee | ||
Comment 7•5 years ago
|
||
Alternative approach with less impact. More similar to what was originally suggested by ato.
Assignee | ||
Comment 8•5 years ago
|
||
Depends on D37165
Assignee | ||
Comment 9•5 years ago
|
||
Depends on D37166
Assignee | ||
Comment 10•5 years ago
|
||
Depends on D37167
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Comment 11•5 years ago
|
||
Comment 12•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/be3c1b453e2c
https://hg.mozilla.org/mozilla-central/rev/3220f8938d1c
https://hg.mozilla.org/mozilla-central/rev/394b9ac5eed9
https://hg.mozilla.org/mozilla-central/rev/0e47b22aefcc
Description
•