301089 - Pressing a key while another key held down does not generate keydown event

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Mark Robson]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.9) Gecko/20050711 Firefox/1.0.5
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.9) Gecko/20050711 Firefox/1.0.5

If you use onkeydown to take keyboard events, pressing a second key while
another is held down, does not generate a keydown event.

This works fine in Mozilla 1.7.8

Reproducible: Always

Steps to Reproduce:
1. visit http://www.gensortium.com/~mark/eventtest.html
2. Hold down one key
3. Press another key

Actual Results:  
Only the first key registers a keydown even

Expected Results:  
Both key pushes register a keydown event

Comment 1

[pq]

(In reply to comment #0)
> User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.9)
Gecko/20050711 Firefox/1.0.5
> Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.9)
Gecko/20050711 Firefox/1.0.5
> 
> If you use onkeydown to take keyboard events, pressing a second key while
> another is held down, does not generate a keydown event.
> 
> This works fine in Mozilla 1.7.8
> 
> Reproducible: Always
> 
> Steps to Reproduce:
> 1. visit http://www.gensortium.com/~mark/eventtest.html
> 2. Hold down one key
> 3. Press another key
> 
> Actual Results:  
> Only the first key registers a keydown even
> 
> Expected Results:  
> Both key pushes register a keydown event

Yes it does this to me too but only seems to happen in Linux, within windows it
seems to work tho

Comment 2

[Nickolay_Ponomarev]

Is it a problem with 1.5 beta as well?

Comment 3

[pq]

(In reply to comment #2)
> Is it a problem with 1.5 beta as well?
> 

yep it sure does - I'm creating a javascript webpage editor that lets you enter text directly onto the webpage and its very hard to type with this bug - I have to singlely hit every key by itself or the following key does not register.
eg.
typing a sentence like: Hello there how are you
ends up looking like: Hlo thre hwae yu 

Comment 4

[Boris Zbarsky [:bzbarsky]]

I can reproduce this in a GTK2 build, but NOT a GTK1 build.  So sounds like a widget issue. 

Can someone narrow down a regression range using builds from archive.mozilla.org?   I only have old nightlies of GTK1 builds...

That said, the only patch I see that touches GTK2 code on branch between 1.7.8 and 1.7.9 is for bug 289940.  But that really shouldn't have affected this.

Comment 5

[Darin Fisher]

Not going to block 1.8.1 for this bug, but we'll happily consider a baked-on-trunk patch.

Comment 6

[Boris Zbarsky [:bzbarsky]]

It really would help to get an idea of when this regressed in the GTK2 builds, on either the 1.7 branch or on trunk...

Comment 7

[Andrew Schultz]

The oldest gtk2 trunk build I have is from Dec 2, 2004 and that build has this bug.  I also see the bug with Firefox 1.0 (gecko 1.7.5) and 1.0.4 (gecko 1.7.8) and Firefox 0.8 (gecko 1.6)
So... I don't believe comment 0.

Comment 8

[Andrew Schultz]

ah, so "This works fine in Mozilla 1.7.8" doesn't mean this regressed between 1.7.8 and 1.7.9, but rather that the Mozilla 1.7.8 gtk1 didn't have the bug and the Firefox 1.0.5/1.7.9 gtk2 did (so comment 0 isn't wrong).  But that's due to gtk1 vs. gtk2.  So this is just a bug that gtk2 has had since forever (or at least for a long time)

Comment 9

[Kathryn]

I'd just like to add that it is only restricted to key press events, the key releases are always reported.  For example:

Presses: ths satest
Releases: this is a test

Comment 10

[O. Atsushi (Torisugari)]

Attachment: Patch v1

nsWindow should remember all keys the user is pressing.

Comment 11

[O. Atsushi (Torisugari)]

Attachment: Patch v2

I'm sure my patch fixes this bug, while I don't understand why GTK1 is
free from this problem at all. This part seems almost identical to that
of GTK2.

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/widget/src/gtk/nsGtkEventHandler.cpp&rev=1.192&mark=487-494#487

Comment 12

[O. Atsushi (Torisugari)]

> I'm sure my patch fixes this bug, while I don't understand why GTK1 is
> free from this problem at all. This part seems almost identical to that
> of GTK2.

Never mind. Now I'm aware of the difference. To many release evnets
saved onKeyDown supression system, though a bit inaccurately.

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

+    /*
+       XXX Probably we should not call GdkKeyCodeToDomKeyCode(...)
+           3 times per 1 gtk key press event. See also implemetaion
+           of InitKeyEvent(...).
+     */

It doesn't matter. Key events can't happen often enough for this to be a problem.

+    PRBool              mKeyRepeatStatus[256];

Use PRPackedBool. PRBool is 4 bytes, PRPackedBool is one byte.

Actually, given that we create quite a few nsWindow objects in some cases, I think it's worth using one bit each. Can you implement that? Unfortuantely I don't think we have a bitset type.

Comment 14

[O. Atsushi (Torisugari)]

Attachment: Patch v3

(In reply to comment #13)
> Actually, given that we create quite a few nsWindow objects in some cases, I
> think it's worth using one bit each. Can you implement that? Unfortuantely I
> don't think we have a bitset type.

Sure, addressed. Will you review this patch?

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

+    /* Though we know it's not likely that aKeyCode is over 0xFF. */
+    aKeyCode &= 0xFF;

Should probably include an NS_ASSERTION that aKeyCode <= 255, in all three methods.

+    return (mKeyDownFlags[(aKeyCode >> 6)] & (1 << (aKeyCode & 0x3F)))?
+               PR_TRUE : PR_FALSE;

Just write "... != 0"

Also I think your "1 <<" should be "PRUint64(1)" otherwise things are going to break when you shift by 32 or more.

Maybe use a helper method in nsWindow
  PRUint64* GetFlagWord(PRUint32 aKeyCode, PRUint64* aMask)
that does the assertion and returns &mKeyDownFlags[aKeyCode >> 6] and stores
PRUint64(1) << (aKeyCode & 0x3F) in *aMask?

Comment 16

[O. Atsushi (Torisugari)]

Attachment: Patch v4

(In reply to comment #15)
> Also I think your "1 <<" should be "PRUint64(1)" otherwise
> things are going to break when you shift by 32 or more.

There's no specific reason to use 64 bit flag in the first place,
I chopped it to 32bit.

Comment 17

[O. Atsushi (Torisugari)]

Oops,

>-    PRUint32            mTransparencyBitmapWidth;
>-    PRUint32            mTransparencyBitmapHeight;
>+    PRInt32             mTransparencyBitmapWidth;
>+    PRInt32             mTransparencyBitmapHeight;

was just to suppress gcc warnings, so it has nothing to do with this bug.

Comment 18

[Daniel Veditz [:dveditz]]

Using signed values for widths is a good way to get into trouble in the future. Please do not do that, find another way to suppress the warnings.

Comment 19

[O. Atsushi (Torisugari)]

Attachment: Patch v5

(In reply to comment #18)
> Using signed values for widths is a good way to get into trouble in the future.
> Please do not do that, find another way to suppress the warnings.

I'm sorry for that. I removed those lines.

Comment 20

[Nickolay_Ponomarev]

Checked in, thanks for the patch.

mozilla/widget/src/gtk2/nsWindow.cpp  1.215
mozilla/widget/src/gtk2/nsWindow.h    1.72