906237 - [e10s] Serialize nsKeyEvent::mNativeKeyEvent to the child process

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[David Anderson [:dvander] - inactive, e-mail if emergency]

bug 282097 introduced a new field in nsKeyEvent. It's only used on OS X, where it contains an NSEvent. It is currently not serializable across IPC, so keyboard input does not really work on Mac.

I'm going to try just serializing whatever bits of it are needed to reconstruct the NSEvent on the other side.

Comment 1

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: null_native_key_event.patch

I don't understand how events work to do a good fix here, yet. In the meantime, we can get text input working again on e10s by just making sure the nsKeyEvent field isn't garbage.

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 792017 [details] [diff] [review]
null_native_key_event.patch

mNativeKeyEvent will be used on GTK2 too in my plan (for same purpose as Mac OS X).

The native key event is necessary to query native keyboard shortcut. So, this patch disables a lot of system wide shortcut keys on Mac OS X if e10s is enabled.

Comment 3

[David Anderson [:dvander] - inactive, e-mail if emergency]

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #2)
> Comment on attachment 792017 [details] [diff] [review]
> null_native_key_event.patch
> 
> mNativeKeyEvent will be used on GTK2 too in my plan (for same purpose as Mac
> OS X).
> 
> The native key event is necessary to query native keyboard shortcut. So,
> this patch disables a lot of system wide shortcut keys on Mac OS X if e10s
> is enabled.

Unless I'm misunderstanding something, this patch doesn't break anything. Text input on OS X is completely broken on e10s. Any keypress will cause it to crash because mNativeKeyEvent isn't initialized.

This patch makes sure the field is initialized to NULL, so we can at least use the browser again until we can figure out a full-fledged solution.

Comment 4

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 792017 [details] [diff] [review]
null_native_key_event.patch

It's only with great reluctance that I override your r-, Masayuki, but this patch looks harmless to me.  Also, it seems to be urgently needed for Electrolysis part deux, which is getting announced at tomorrow's Platform meeting.

Please let me know if I've misunderstood, and if so we can undo the damage later :-)

Comment 5

[David Anderson [:dvander] - inactive, e-mail if emergency]

 https://hg.mozilla.org/integration/mozilla-inbound/rev/c7f5065c808c

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c7f5065c808c

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Okay, but please file a bug for improving the Mac OS X's system wide shortcut key handling.

Comment 8

[David Anderson [:dvander] - inactive, e-mail if emergency]

Filed as bug 950255