Closed Bug 630817 Opened 9 years ago Closed 9 years ago

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

Categories

(Core :: Widget: Gtk, defect)

x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla5

People

(Reporter: masayuki, Assigned: masayuki)

References

()

Details

Attachments

(2 files, 1 obsolete file)

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.
Attached patch Patch v1.0 (obsolete) — Splinter Review
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
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.
Attachment #509376 - Flags: review?(karlt)
Karl: Would you review the patch? That is first step of keyboard handling code refactoring on Linux.
(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 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.
(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.
Attached patch Patch v1.1Splinter Review
Attachment #509376 - Attachment is obsolete: true
Attachment #521742 - Flags: review?(karlt)
Attachment #509376 - Flags: review?(karlt)
Attachment #521742 - Flags: review?(karlt) → review+
http://hg.mozilla.org/mozilla-central/rev/9bdade790c52
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.2
You need to log in before you can comment on or make changes to this bug.