Get basic KeyEvents working in RWHVA

Get KeyEvents to limp in Aura. This assumes the desktop host is going to send two aura::KeyEvents for every keydown - one raw key down, and one char. They share the same ET, but the first has is_char = false, the second is_char = true.

http://crbug.com/99757
TEST=none
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=106374

[Ben Goodger (Google), 2011-10-19 04:53:41]

OK, this actually looks like the real bandaid. I'm not going to bother with the
IME hookup right now. This gets this working on Windows. I need to handle
WM_CHAR and fwd to webkit. This means ignoring WM_CHAR in NWA.

Not sure what the right X11 fix is.

[Hironori Bono, 2011-10-19 05:45:31]

Greetings Ben,

This is just for your information.

On 2011/10/19 04:53:41, Ben Goodger (Google) wrote:
> Not sure what the right X11 fix is.

From my rusty knowledge about X, XLookupKeysym() looks up a keyboard-layout
table and converts an XKeyEvent to its character. (In terms of Windows, it is a
equilavent function to MapVirtualKey().) So, we need to call XLookupKeysym()
when we receive a KeyPress event in DesktopHostLinux::Dispatch() and to call
desktop_->OnKeyEvent() twice (one for a rawkeydown and the other is for a char)
as we did for GTK? (I do not have good knowledge about aura and this may be
wrong, though.)

Regards,

Hironori Bono

[Hironori Bono, 2011-10-19 10:43:53]

Greetings,

On 2011/10/19 05:45:31, hbono wrote:
> From my rusty knowledge about X, XLookupKeysym() looks up...
                                   ~~~~~~~~~~~~~~~
Sorry, it is XLookupString(). (I remember XLookupKeysym() does not use
modifier-key status.)

Regards,

Hironori Bono

[sadrul, 2011-10-19 13:05:26]

Quite interesting. The change to make keyevents work in X11 is here:
http://codereview.chromium.org/8353005/

[Ben Goodger (Google), 2011-10-19 14:21:38]

So, I am beginning to think DesktopHostLinux should send two key events,
just like Windows ends up doing. One for keypress and one for char. We would
have to add a flag to aura::KeyEvent |is_char| to disambiguate. I don't know
if we can introduce a new event type without breaking the universe.

In NativeWidgetAura, we would discard the |is_char| ones.
In RWHVA, we'd fwd them just like keypress.

The reason for doing this is so that we propagate an identical event
sequence on windows and linux. This makes it possible for the IME folk to
write an InputMethodAura that actually processes the char events properly in
NativeWidgetAura.

Sadrul, WDYT?

-Ben

On Wed, Oct 19, 2011 at 6:05 AM, <sadrul@chromium.org> wrote:

> Quite interesting. The change to make keyevents work in X11 is here:
>
http://codereview.chromium.**org/8353005/<http://codereview.chromium.org/8353...
>
>
http://codereview.chromium.**org/8342026/<http://codereview.chromium.org/8342...
>

[sadrul, 2011-10-19 14:36:10]

On 2011/10/19 14:21:38, Ben Goodger (Google) wrote:
> So, I am beginning to think DesktopHostLinux should send two key events,
> just like Windows ends up doing. One for keypress and one for char. We would
> have to add a flag to aura::KeyEvent |is_char| to disambiguate. I don't know
> if we can introduce a new event type without breaking the universe.
> 
> In NativeWidgetAura, we would discard the |is_char| ones.
> In RWHVA, we'd fwd them just like keypress.
> 
> The reason for doing this is so that we propagate an identical event
> sequence on windows and linux. This makes it possible for the IME folk to
> write an InputMethodAura that actually processes the char events properly in
> NativeWidgetAura.

This sounds like a good idea to me. However, I am not sure if this will make it
easier to get the current IME stuff to work with aura (InputMethodAura), or if
this will make it even more difficult. Added Peng in the CC list. Peng, what do
you think?

> 
> Sadrul, WDYT?
> 
> -Ben
> 
> On Wed, Oct 19, 2011 at 6:05 AM, <mailto:sadrul@chromium.org> wrote:
> 
> > Quite interesting. The change to make keyevents work in X11 is here:
> >
>
http://codereview.chromium.**org/8353005/%3Chttp://codereview.chromium.org/83...>
> >
> >
>
http://codereview.chromium.**org/8342026/%3Chttp://codereview.chromium.org/83...>
> >

[Ben Goodger (Google), 2011-10-19 14:39:11]

Peng, to bring you up to speed... in the Aura environment we are talking
about sending two aura::KeyEvents to an aura::Window for each keydown - both
ui::ET_KEY_PRESSED but the first with is_char = false and the second with
is_char = true.

This makes the sequence of events received by the views::NativeWidgetAura
similar to that received by views::NativeWidgetWin - first a keydown, then a
char. So there would need to be an InputMethodAura that handles the
aura::KeyEvents (char and non-char) and figure out what to dispatch.

-Ben

On Wed, Oct 19, 2011 at 7:36 AM, <sadrul@chromium.org> wrote:

> On 2011/10/19 14:21:38, Ben Goodger (Google) wrote:
>
>> So, I am beginning to think DesktopHostLinux should send two key events,
>> just like Windows ends up doing. One for keypress and one for char. We
>> would
>> have to add a flag to aura::KeyEvent |is_char| to disambiguate. I don't
>> know
>> if we can introduce a new event type without breaking the universe.
>>
>
>  In NativeWidgetAura, we would discard the |is_char| ones.
>> In RWHVA, we'd fwd them just like keypress.
>>
>
>  The reason for doing this is so that we propagate an identical event
>> sequence on windows and linux. This makes it possible for the IME folk to
>> write an InputMethodAura that actually processes the char events properly
>> in
>> NativeWidgetAura.
>>
>
> This sounds like a good idea to me. However, I am not sure if this will
> make it
> easier to get the current IME stuff to work with aura (InputMethodAura), or
> if
> this will make it even more difficult. Added Peng in the CC list. Peng,
> what do
> you think?
>
>
>  Sadrul, WDYT?
>>
>
>  -Ben
>>
>
>  On Wed, Oct 19, 2011 at 6:05 AM, <mailto:sadrul@chromium.org> wrote:
>>
>
>  > Quite interesting. The change to make keyevents work in X11 is here:
>> >
>>
>
> http://codereview.chromium.****org/8353005/%3Chttp://coderevi**
> ew.chromium.org/8353005/ <http://codereview.chromium.org/8353005/>>
>
>> >
>> >
>>
>
> http://codereview.chromium.****org/8342026/%3Chttp://coderevi**
> ew.chromium.org/8342026/ <http://codereview.chromium.org/8342026/>>
>
>> >
>>
>
>
>
>
http://codereview.chromium.**org/8342026/<http://codereview.chromium.org/8342...
>

[Ben Goodger (Google), 2011-10-19 14:44:09]

On Wed, Oct 19, 2011 at 7:36 AM, <sadrul@chromium.org> wrote:

> On 2011/10/19 14:21:38, Ben Goodger (Google) wrote:
>
>> So, I am beginning to think DesktopHostLinux should send two key events,
>> just like Windows ends up doing. One for keypress and one for char. We
>> would
>> have to add a flag to aura::KeyEvent |is_char| to disambiguate. I don't
>> know
>> if we can introduce a new event type without breaking the universe.
>>
>
>  In NativeWidgetAura, we would discard the |is_char| ones.
>> In RWHVA, we'd fwd them just like keypress.
>>
>
>  The reason for doing this is so that we propagate an identical event
>> sequence on windows and linux. This makes it possible for the IME folk to
>> write an InputMethodAura that actually processes the char events properly
>> in
>> NativeWidgetAura.
>>
>
> This sounds like a good idea to me. However, I am not sure if this will
> make it
> easier to get the current IME stuff to work with aura (InputMethodAura), or
> if
> this will make it even more difficult. Added Peng in the CC list. Peng,
> what do
> you think?
>

I have a hard time imagining it'd make it more difficult, since the model
would be similar to Windows where we receive a KeyDown followed by a Char.

The particulars of how it's processed are probably going to be a little
different since is no Gtk/Win IME to rely on in the Aura world.

-Ben

[Ben Goodger (Google), 2011-10-19 16:15:32]

OK, I updated this. The DesktopHostLinux should send another KeyEvent passing
true through the ctor. I wasn't sure if any of your other patches were needed to
make this work, so I didn't do that here.

[Peng, 2011-10-19 16:16:49]

I am not very familiar with IME on Windows, but I think on Windows, IME will
filter the WM_KEYDOWN and WM_KEYUP events only, and generate result.  If the
WM_KEYDOWN & WM_KEYUP events are not filtered by IME, they will be sent to
TranslateMessage, and then application will get WM_CHAR message.

So if we want to keep same behavior on Linux, I guess the message sequence would
be:
1. Get XKeyEvent from X server
2. Translate it to Keysymbol
3. Send it to IME
3.1 If IME filters the key event. than do nothing, or send a KEYDOWN event which
keysym is VK_PROCSSED (which is used on Windows) to views.
3.2 If IME ignores the key events, send the event to view, and translate it to a
CHAR event and send it to views.


On 2011/10/19 14:39:11, Ben Goodger (Google) wrote:
> Peng, to bring you up to speed... in the Aura environment we are talking
> about sending two aura::KeyEvents to an aura::Window for each keydown - both
> ui::ET_KEY_PRESSED but the first with is_char = false and the second with
> is_char = true.
> 
> This makes the sequence of events received by the views::NativeWidgetAura
> similar to that received by views::NativeWidgetWin - first a keydown, then a
> char. So there would need to be an InputMethodAura that handles the
> aura::KeyEvents (char and non-char) and figure out what to dispatch.
> 
> -Ben
> 
> On Wed, Oct 19, 2011 at 7:36 AM, <mailto:sadrul@chromium.org> wrote:
> 
> > On 2011/10/19 14:21:38, Ben Goodger (Google) wrote:
> >
> >> So, I am beginning to think DesktopHostLinux should send two key events,
> >> just like Windows ends up doing. One for keypress and one for char. We
> >> would
> >> have to add a flag to aura::KeyEvent |is_char| to disambiguate. I don't
> >> know
> >> if we can introduce a new event type without breaking the universe.
> >>
> >
> >  In NativeWidgetAura, we would discard the |is_char| ones.
> >> In RWHVA, we'd fwd them just like keypress.
> >>
> >
> >  The reason for doing this is so that we propagate an identical event
> >> sequence on windows and linux. This makes it possible for the IME folk to
> >> write an InputMethodAura that actually processes the char events properly
> >> in
> >> NativeWidgetAura.
> >>
> >
> > This sounds like a good idea to me. However, I am not sure if this will
> > make it
> > easier to get the current IME stuff to work with aura (InputMethodAura), or
> > if
> > this will make it even more difficult. Added Peng in the CC list. Peng,
> > what do
> > you think?
> >
> >
> >  Sadrul, WDYT?
> >>
> >
> >  -Ben
> >>
> >
> >  On Wed, Oct 19, 2011 at 6:05 AM, <mailto:sadrul@chromium.org> wrote:
> >>
> >
> >  > Quite interesting. The change to make keyevents work in X11 is here:
> >> >
> >>
> >
> > http://codereview.chromium.****org/8353005/%253Chttp://coderevi**
> > ew.chromium.org/8353005/ <http://codereview.chromium.org/8353005/>>
> >
> >> >
> >> >
> >>
> >
> > http://codereview.chromium.****org/8342026/%253Chttp://coderevi**
> > ew.chromium.org/8342026/ <http://codereview.chromium.org/8342026/>>
> >
> >> >
> >>
> >
> >
> >
> >
>
http://codereview.chromium.**org/8342026/%3Chttp://codereview.chromium.org/83...>
> >

[sadrul, 2011-10-19 20:24:56]

LGTM

[James Su, 2011-10-20 01:45:51]

On 2011/10/19 14:21:38, Ben Goodger (Google) wrote:
> So, I am beginning to think DesktopHostLinux should send two key events,
> just like Windows ends up doing. One for keypress and one for char. We would
> have to add a flag to aura::KeyEvent |is_char| to disambiguate. I don't know
> if we can introduce a new event type without breaking the universe.
> 
> In NativeWidgetAura, we would discard the |is_char| ones.
> In RWHVA, we'd fwd them just like keypress.
> 
> The reason for doing this is so that we propagate an identical event
> sequence on windows and linux. This makes it possible for the IME folk to
> write an InputMethodAura that actually processes the char events properly in
> NativeWidgetAura.

On Windows, keyboard layouts and IMEs are handled in a unified (well, almost)
way. All KEYDOWN/KEYUP events are dispatched to the system IME or keyboard
layout engine (by USER.EXE/IMM)  before being dispatched to the application. And
the CHAR events are actually synthesized by the keyboard layout/IME layer
(unlike KEYDOWN/KEYUP, which are kind of hardware events). So InputMethodWin is
merely for receiving results from the system keyboard layout/IME. It's actually
nothing to do with keyboard event handling.

But on Linux, keyboard layouts and input methods are handled differently. All
keyboard events are sent to the application directly, it's the application's
responsibility to forward them to the system input method, and receive results
from it. If there is no input method or the input method cannot handle the key
events, then the application needs to query the X server by itself to get the
correct characters according to the current keyboard layout configuration.
In chrome, InputMethodIBus/InputMethodGtk takes the responsibility for both
sending the keyboard events to the system input method and querying the X server
for characters if a keyboard layout is used instead.

So if we need to synthesize CHAR events in DesktopHostLinux, then IMHO the most
appropriate way is to incorporate InputMethodIBus/InputMethodGtk into
DesktopHostLinux and handle this logic there. We also need to do the same thing
on Windows that incorporate InputMethodWin into DesktopHostWin to handle IME
related logics.

Then we can have InputMethodAura as a fake input method implementation which
simply receives results from the real input method implementation owned by the
DesktopHost class.

But whether or not to use a CHAR event (is_char in aura::Event) to transfer the
character results from InputMethodIBus/Win/Gtk to the InputMethodAura is still
debatable, because we need to handle not only results from keyboard layouts
(usually CHAR events on Windows), but also results from input methods. I think
the easiest way for us is to let InputMethodAura implement
views::TextInputClient interface, so that both of these results can be
transfered via this interface from InputMethodIBus/Gtk/Win to InputMethodAura
and then to RenderWidgetHostViewAura. Then we can synthesize Webkit::Char events
inside TextInputClient::InsertChar() method implemented by either
InputMethodAura or RenderWidgetHostViewAura.

With this approach, we should not need to dispatch Char events down to the
aura/views widget tree at all. Just like what we have done in the original pure
views implementation (NativeWidgetWin does not dispatch Char events down to
views tree at all).

> 
> Sadrul, WDYT?
> 
> -Ben
> 
> On Wed, Oct 19, 2011 at 6:05 AM, <mailto:sadrul@chromium.org> wrote:
> 
> > Quite interesting. The change to make keyevents work in X11 is here:
> >
>
http://codereview.chromium.**org/8353005/%3Chttp://codereview.chromium.org/83...>
> >
> >
>
http://codereview.chromium.**org/8342026/%3Chttp://codereview.chromium.org/83...>
> >