46004 - shift key is not being set for keypress events

Status: VERIFIED FIXED

(Core :: DOM: UI Events & Focus Handling, defect, P3)

Description

[Kathleen :Brade]

I noticed today that when I pressed alt-shift-n to open a new composer window, I
got a browser window instead.  Upon further investigation (and turning on the
debugging information in nsGTKEventHandler.cpp), I saw that we aren't setting
the shift flag for keypress events (possibly other key events as well?).

Keybindings are currently broken if they rely on the user pressing shift to use
them.  I'm not positive exactly how many keybindings this affects.

Assigning to blizzard since he made changes to nsGtkEventHandler.cpp most recently.

Comment 1

[Kathleen :Brade]

reassign to ftang; I think he added isShift=false though I'm not yet sure why.

Comment 2

[Akkana Peck]

Kathy and I talked about this and thought that a good solution might be to make
the code a little smarter, so that it disables isShift only if control or alt
are not pressed (i.e. only for printable keys).  Also, let's put in a comment
explaining why isShift is disabled (which I guess is that we're trying to use
"no modifier keys are set" as a way of indicating "this key event represents a
printable character, insertable by an editor" since the event system gives us no
other way of representing that concept).

Comment 3

[Kathleen :Brade]

Attachment: proposed patch (comment may need some polishing)

Comment 4

[Kathleen :Brade]

I have attached a patch which works for me.  I can now do the following
successfully:
  * type capital letters in Composer and edit fields,
  * type lower case letters, numbers, shift-numbers,
  * tab and shift-tab to navigate a form,
  * use alt-shift-n to open a Composer window,
  * use arrows, shift-arrows, control-shift arrows to affect selection/caret

Can anyone think of anything else that should be tested?

Comment 5

[Frank Tang]

r=ftang and reassign back to brade. 

Comment 6

[Jay Patel [:jay]]

Putting on [nsbeta2+] radar for beta2 fix. 

Comment 7

[Kathleen :Brade]

fix checked in for today's builds.

Comment 8

[Kathleen :Brade]

*** Bug 38591 has been marked as a duplicate of this bug. ***

Comment 9

[ckritzer (gone)]

Mass update:  changing qacontact to [e-mail]

Comment 10

[Sean Richardson]

Testing with 2000-08-01-04-M17 on WinNT:
1. All printable characters, shifted and unshifted, can be typed into edit
   fields.
2. tab and shift-tab, and all combinations of shift, ctrl, and arrowkeys
   do the right thing.
3. ctrl-shift-n brings up a different window than ctrl-n, but for all
   other alpha, ctrl-shift-alpha brings up the same window/function as
   the corresponding ctrl-alpha, and for all alpha corresponding to main 
   menu names, alt-shift-alpha brings up the same window/function as the 
   corresponding alt-alpha. 

Comment 11

[ckritzer (gone)]

On Linux6 2000-080204-M17 Commercial build:

The only instance of alt-shift-ALPHA which seems to work is for bringing up a
composer window (alt-shift-n).  Every other instance of alt-shift-ALPHA I've
encountered does what it's alt-shift sibling is supposed to do.  Not a big deal,
you say?  Try hitting alt-shift-z.  Instead of 'Redo' you get 'Undo'.

Reopening.

Comment 12

[Sean Richardson]

First, remembering that shortcut keys on Linux do not match those on Win & Mac:
bug 22515, "Shortcut keys in Linux Mozilla not in line with standards for 
KDE/Gnome", nsbeta3+... meaning that once it is fixed, a new composer wiondow
will come up when ctrl-shift-n is typed, not alt-shift-n.

Second, on Windows, undo is ctrl-z, and redo is ctrl-y, not ctrl-shift-z.
I don't see *any* ctrl-shift-alpha keys on Windows, but you'll want
to consult http://gooey/client/5.0/specs/keyboard/kybdnav2.htm to verify that.

Comment 13

[Christopher Blizzard (:blizzard)]

Hey, is there a chance we can get that page onto mozilla.org?  ( a spec?  no
way! )

Comment 14

[leger]

Putting on [nsbeta2-] radar.  Agree with Sean...not critical to beta2.

Comment 15

[rubydoo123]

rremoved nsbeta2 from keywords -- added nsbeta3 and nsbeta3+. Kathy -- any input 
on level of priority

Comment 16

[Kathleen :Brade]

re-resolving this bug as fixed.  I don't think it has been broken since I last 
resolved it.  Using a Linux build from today, I see that alt-shift-n does 
appropriate bring up composer instead of navigator.  Also, alt-shift-b in 
composer does *NOT* set bold so it is working correctly also.  This bug is fixed 
since the shift key is now being set for keypress events.  If it weren't fixed, 
alt-shift-n wouldn't work.  I'm unaware of any problems with any particular 
keybindings not working.  If there are bugs, they should be filed as separate 
(keybinding) bugs; not as a widget bug (which is what this bug is).  This bug 
should be reopened only if *all* shift keybindings are broken on Linux.

Comment 17

[Kathleen :Brade]

note to Sean Richardson:  this is a Linux bug; if there is a problem on NT, it 
should be filed as a separate bug

Comment 18

[ckritzer (gone)]

Wow.  This bug got convoluted in a hurry.

This bug is fixed.
Marking VERIFIED FIXED on:
- LinuxRH62 2000-09-07-08-M18 Commercial
- Win98     2000-09-07-08-M18 Mozilla
- MacOS86   2000-09-07-04-M18 Commercial