532912 - Modifier key events need to be passed

Status: RESOLVED INVALID

(Core :: Widget: Gtk, defect)

Description

[Tero Turtiainen]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.9.1.4) Gecko/20091016 Firefox/3.5.4
Build Identifier: 

/widget/src/gtk2/nsWindow.cpp drops bare modifier key events.

In Maemo we must not drop those events as they are needed to keep GtkImContext in sync. This is needed to get for example keyboard input in Flash player fully working.

Reproducible: Always

Steps to Reproduce:
1. Open some page with Flash content where you can write text
2. Press shift key, release shift key
3. Press a key
Actual Results:  
Lower case a appears

Expected Results:  
Upper case a appears.

As the shift key pressing in step 2 is dropped, GtkImContext does not know that we should be now producing an upper case letter.

Comment 1

[Tero Turtiainen]

Attachment: Don't drop modifier keys

Patch proposal

Comment 2

[Karl Tomlinson (:karlt)]

I'm not following where GtkImContext is involved here.  Is that created by the plugin?

Windowless plugin events come from NS_KEY_DOWN events, which means that the plugin does get the notification of change in state of the Shift key.
Also, each XKeyPress event contains the modifier state.

We don't send events to windowless plugins based on NS_KEY_PRESS.
This causes the problem reported in bug 507811 (but if we did send events to plugins based on NS_KEY_PRESS we would have this problem).

Could this bug be a result of using MOZ_COMPOSITED_PLUGINS?

Comment 3

[Tero Turtiainen]

(In reply to comment #2)
> I'm not following where GtkImContext is involved here.  Is that created by the
> plugin?

Yes, it is created by the plugin.

> Windowless plugin events come from NS_KEY_DOWN events, which means that the
> plugin does get the notification of change in state of the Shift key.
> Also, each XKeyPress event contains the modifier state.

The modifier state in XKeyPress is not enough. See test sequence step 2 and 3. In step 3, the modifier state is basically zero as the shift has been released. But the clever input method should give the user an upper case letter as the user had pressed shift in step 2.

> We don't send events to windowless plugins based on NS_KEY_PRESS.
> This causes the problem reported in bug 507811 (but if we did send events to
> plugins based on NS_KEY_PRESS we would have this problem).

Well, we are using the "Possible fix" patch from bug 507811.

Comment 4

[Tero Turtiainen]

I'm sorry but I don't agree with RESOLVED/INVALID.

In Maemo Browser we need to pass the NS_KEY_PRESSes for modifier keys to for example the flash plugin. But currently nsWindow::OnKeyPressEvent drops these. 

See the "steps to reproduce" and "expeceted results". In the test sequence flash plugin should get the events for "NS_KEY_PRESS for shift-key" "NS_KEY_RELEASE for shift-key", "NS_KEY_DONN for 'a', modifier state 0". Even though the modifier state is 0 this would produce upper case 'A' as the flash plugin had received the press for shift-key and had updated the IM context status.

But now as we are not sending the NS_KEY_PRESS for shift (or any modifier keys), IM context does not know about the previously pressed shift-key and produces lover case 'a'.

Comment 5

[Karl Tomlinson (:karlt)]

I thought this bug was describing a situation that only happened with attachment 392087 [details] [diff] [review] applied, but didn't happen with unmodified Gecko.  Did I misunderstand?

The plugin only gets an X KeyPress event.  (There is no X KeyDown event.)  Why would it matter that this is sent during NS_KEY_DOWN (instead of NS_KEY_PRESS) for modifier keys?  Is there some reason why the KeyPress event for the shift key is not sent during NS_KEY_DOWN?

Comment 6

[Tero Turtiainen]

Actually now I think you are correct. Sorry for the confusion and thanks for making me think about this yet again!