630817 - The value of native keyevent's state doesn't refer to the event

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

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

The value of native keyevent's state doesn't refer to the event.

E.g., when shift key is pressed, (state & GDK_SHIFT_MASK) isn't true. On the other hand, when shift key is released, (state & GDK_SHIFT_MASK) is still true.

On Win and Mac, we set the modifier key state as expected.

For keypress event, this can fix easily. However, it's not so for keyrelease event because both shift keys may be pressed same time and the release event may be for one of them.

Comment 1

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

Attachment: Patch v1.0

Comment 2

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

Comment on attachment 509376 [details] [diff] [review]
Patch v1.0

This will be landed after Fx4, so, please do your work for Fx4 rather than this review if you have.

Comment 3

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

Karl: Would you review the patch? That is first step of keyboard handling code refactoring on Linux.

Comment 4

[Karl Tomlinson (:karlt)]

(In reply to comment #0)
> E.g., when shift key is pressed, (state & GDK_SHIFT_MASK) isn't true. On the
> other hand, when shift key is released, (state & GDK_SHIFT_MASK) is still true.

I assume it is a keydown (not keypress) event involved here, as I don't expect a keypress event from the shift key.

The state seems quite reasonable to me as the shift modifier was not down at the time when the shift key was pressed.

Is there a specification that says that the state should represent the state *after* the key press?

Comment 5

[Karl Tomlinson (:karlt)]

I guess this example indicates that expectation.
http://www.w3.org/TR/2006/WD-DOM-Level-3-Events-20060413/keyset.html#Modifiers

Comment 6

[Karl Tomlinson (:karlt)]

Comment on attachment 509376 [details] [diff] [review]
Patch v1.0

I'm not keen on using gdk_keyboard_get_modifiers() here.  As you indicate in the comment, it returns the state at a later time than the key event and any number of key events may have happened between the two times, and so this is an unreliable and unpredictable indicator.  e.g.  A shift key may have already been pressed again for the next character.

Is it important that the keyup event indicate that the modifier state is unset when it is the last such modifier key being released?

If so, the simplest implementation might be to count how many times a modifier key is pressed while the corresponding modifier state on the native event is set (and count how many times such a key is released).

This would miss key events that happened while focus was in another app, but the counter could be reset when the modifier is pressed and the modifier state on the native event is not set.

Comment 7

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

(In reply to comment #6)
> Is it important that the keyup event indicate that the modifier state is unset
> when it is the last such modifier key being released?

I think that keyup event's state isn't as important as keydown event's because probably, most applications are handling keydown event or keypress event. So, I think that we can put off the fix with your approach to another bug. I think that we should do it in bug 600117. The bug needs to manage the all key's state.

Comment 8

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

Attachment: Patch v1.1

Comment 9

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

Attachment: Patch rv1.1 (mq)

Comment 10

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

http://hg.mozilla.org/mozilla-central/rev/9bdade790c52