Closed Bug 1543142 Opened 5 years ago Closed 5 years ago

Implement Input.dispatchKeyEvent

Categories

(Remote Protocol :: Agent, enhancement, P1)

enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ochameau, Assigned: jdescottes)

References

Details

Attachments

(3 files, 5 obsolete files)

Wordpress test suite fake some key events by using puppeteer's page.keyboard.down, page.keyboard.press and page.keyboard.up.

These three puppeteer's method all use CDP's Input.dispatchKeyEvent method.
https://chromedevtools.github.io/devtools-protocol/tot/Input#method-dispatchKeyEvent
Input.dispatchKeyEvent:
Dispatches a key event to the page.

Priority: -- → P2

EventUtils.js may be useful here. Marionette uses an older fork
of it with some tweaks.

Depends on: 1543185
Assignee: poirot.alex → nobody
Attachment #9058255 - Attachment is obsolete: true
Priority: P2 → P3
Priority: P3 → P2

I piled up a very terrible hack in order to workaround issues with a set of "special keys".
Any key involving Ctrl and Shift doesn't seem to work when fired from the content process.
I tried many things around EventUtils and TextInputProcessor without finding any setup that worked.
While it is much easier to make this work from the parent process.
So in this patch I'm using the message manager to fire the key event in the parent process.
Also, I was more successful with EventUtils than TextInputProcessor...
And then, when you start firing only some events in the parent process and all the others in the content process,
TextInputProcessor ends up being confused and throws. So I started using EventUtils from the content process as well.

Ctrl+Shift isn't the only special case. There is at least End, which fails the same way.
And it is very likely that other keys are having the same issue.

Comment on attachment 9065128 [details]
Bug 1543142 - Workaround to support End and Ctrl+Shift keys.

Hi Masayuki,

Andreas told me you may know something about the issue I'm having with key events here.
I'm having a hard time simulating various key events from the content process.
I know about two cases:

  • The End key,
  • Any key involving Ctrl and Shift.

I'm able to make them work by firing them from the parent process instead.

So I was wondering:

  • if you know why these events are any special?
  • if there is any way to fire them from the content process?
  • and more generally, if you know about key events, what is the best API to use: EventUtils, TextInputProcessor, native KeyboardEvents, ... something else?

Thanks a lot for your help!

(By the way, I only tested on Linux, in case there is platform specifics here)

Attachment #9065128 - Flags: feedback?(masayuki)

Sorry, I still don't understand about this bug.

Do we try to create just compatible API? I mean that, for example, we use different keyCode mapping for some keys. Therefore, if we just dispatch KeyboardEvent with given keyCode value, it does not check actual our user's input on the product.
https://github.com/GoogleChrome/puppeteer/blob/90a1032300ae9802c96046f1ebe550fb76c19cbb/lib/Input.js#L54

But I don't know whether these API should cause trusted events or not. TextInputProcessor synthesizes only trusted events. That means that Gecko treats such inputs coming from users. So, they are better for tests, but not safe feature if it could be run from web contents too.

Any key involving Ctrl and Shift doesn't seem to work when fired from the content process.

Really? If so, some tests may not run as expected. Or, did you mean that it won't kick shortcut keys of Firefox? If so, yes it is unless using synthesizing API for tests and setting a gfx pref.

I'm able to make them work by firing them from the parent process instead.

I think that there are no users of nsITextInputProcessor::beginInputTransaction() since it was designed for Firefox OS and XUL addons (although automated tests check its behavior). So, I think that you can add new argument to run it as async or sync.

and more generally, if you know about key events, what is the best API to use: EventUtils, TextInputProcessor, native KeyboardEvents, ... something else?

EventUtils.js is not optimized for performance. So, I recommend that you should create similar and faster code rather than copying it. Anyway it's sometimes modified so that syncing them is difficult.

(By the way, I only tested on Linux, in case there is platform specifics here)

Oh, on macOS and Linux, Gecko checks native key binding with hacky way. However, we don't have a way to resolve them with synthesized keys, IIRC. It might be okay if we synthesize key events from the main process though but I'm not sure.

Flags: needinfo?(poirot.alex)
Attached file Bug 1543142 - Support type=char (obsolete) —

But I don't know whether these API should cause trusted events or not.
So, they are better for tests, but not safe feature if it could be run from web contents too.

This will only be used in automated tests and we want to be as close to real user input as possible, so I think we want trusted events.

EventUtils.js is not optimized for performance.

Our implementation will only be used from automated tests, so performance is not an immediate concern.

Oh, on macOS and Linux, Gecko checks native key binding with hacky way.
However, we don't have a way to resolve them with synthesized keys, IIRC.
It might be okay if we synthesize key events from the main process though but I'm not sure.

We tried to simulate deleting backwards with "Backspace" + ctrlKey (or metaKey on OSX), and it seems impossible to trigger the corresponding action with EventUtils. From your comment, I understand that if we want to simulate native key bindings, we need to directly call the command?
We can't synthesize an event to trigger it? Would it be interesting to add this directly to EventUtils.js?

Flags: needinfo?(masayuki)

Taking this for now to add a basic implementation supporting at least typing text.
EventUtils seems to be the most complete API available for now, so will start with this.

I wonder if we should rather implement a parent Input domain in addition to the content one, and call EventUtils from there?
My initial intention was just to call everything from the content Input process even though it is limited.
But from what I understand in the current patches, calling EventUtils from the parent process just seems to unlock more keys?

Will investigate a bit, but I want to have something landable soon.

(clearing ni? for Alex here since it's a bit old)

Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Flags: needinfo?(poirot.alex)
Priority: P2 → P1
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b5af82d4b350
Implement basic dispatchKeyEvent in parent Input domain r=ato
https://hg.mozilla.org/integration/autoland/rev/cca50c60ce53
Add basic test for dispatchKeyEvent r=remote-protocol-reviewers,ochameau
https://hg.mozilla.org/integration/autoland/rev/f5d63a8f1a29
Wait for explicit events in dispatchKeyEvent test r=ochameau

(In reply to Julian Descottes [:jdescottes] from comment #10)

But I don't know whether these API should cause trusted events or not.
So, they are better for tests, but not safe feature if it could be run from web contents too.

This will only be used in automated tests and we want to be as close to real user input as possible, so I think we want trusted events.

EventUtils.js is not optimized for performance.

Our implementation will only be used from automated tests, so performance is not an immediate concern.

Understand. Thanks for the clarification.

Oh, on macOS and Linux, Gecko checks native key binding with hacky way.
However, we don't have a way to resolve them with synthesized keys, IIRC.
It might be okay if we synthesize key events from the main process though but I'm not sure.

We tried to simulate deleting backwards with "Backspace" + ctrlKey (or metaKey on OSX), and it seems impossible to trigger the corresponding action with EventUtils. From your comment, I understand that if we want to simulate native key bindings, we need to directly call the command?

Unfortunately, yes for now. for example,
https://searchfox.org/mozilla-central/rev/f91bd38732d4a330eba4e780812274b98eb81274/layout/base/tests/test_moving_and_expanding_selection_per_page.html#20

We can't synthesize an event to trigger it? Would it be interesting to add this directly to EventUtils.js?

Yeah, I'd like to add it, but my queue is already full for now...

Flags: needinfo?(masayuki)
Regressions: 1562205
Blocks: 1562740
Blocks: 1563205
Blocks: 1563206
Attachment #9058245 - Attachment is obsolete: true
Attachment #9066353 - Attachment is obsolete: true
Attachment #9066352 - Attachment is obsolete: true
Attachment #9065128 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: