59455 - Browser crashed when hit "Ctrl+Shift+(Tab/Esc/Return/F#/->)key.

Status: RESOLVED FIXED

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

Description

[Shirley Liu]

In Netscape 6.0 test blitz,

Browser crashed when hitting Ctrl+Shift+(Tab/Esc/Return/Copy/Del/F#/->/..) keys.

It was fine when hitting Ctrl+Shift+(Numbers/Letters).

Comment 1

[Cyril Bortolato]

See also bug 59152 "Browser crashes when you press certain key combinations".
Per that bug, over to keyboard navigation.

Comment 2

[Ann Adamcik]

With an 11/8 build, I can no longer get it to crash with
"Ctrl+Shift+(Tab/Esc/Return/F1-10)".

I do see the crash with "Ctrl+Shift+(F11/F12/NumPad1-9)" *and* with just
"Shift+(F11/F12/NumPad1-9)".  It looks like those keys are returning odd keysym
values when shifted.

Comment 3

[Ann Adamcik]

Okay, it turns out that Sun uses some non-standard keysyms for some keys.  So,
mozilla needs to either have a mechanism to recognize vendor-specific keysyms,
or simply ignore unrecognized ones (as opposed to crashing!).


The following cause mozilla to crash:

F11 and F12 (with or without any modifiers)


If NumLock is "off", the following crash:

KP1, KP3, KP5, KP7, and KP9 (with or without modifiers)
Shift+(KP1-KP9)

KP2, KP4, KP6, and KP8 unshifted are okay.


If NumLock is *on*, the following will cause a crash:

Shift+(KP1/KP3/KP5/KP7/KP9)


The keysyms returned in these cases are as follows:

F11
	SUNXK_F36	0x1005FF10
F12
	SUNXK_F37	0x1005FF11
KP1
	XK_F33		0xFFDE
Shift+KP2
XK_F34
	0xFFDF
KP3
	XK_F35		0xFFE0
Shift+KP4
XK_F30
	0xFFDB
KP5
	XK_F31		0xFFDC
Shift+KP6
XK_F32
	0xFFDD
KP7
	XK_F27		0xFFD8
Shift+KP8
XK_F28
	0xFFD9
KP9
	XK_F29		0xFFDA

Comment 4

[Ann Adamcik]

Attachment: Fixes core dump when pressing keypad keys on Solaris

Comment 5

[Ann Adamcik]

The patch fixes the crash when the keypad keys are pressed.  F27-F35 were
already defined; we just needed to check for them in the Solaris case.  The
crash when pressing F11 and F12 will require adding F36 and F37 definitions to
gdk...

Comment 6

[timeless]

If I use x11 or xdmcp from a solaris box where a linux box is running mozilla, 
would pressing these keys crash mozilla? If so I'd prefer for this patch to not 
use #if defined(SOLARIS)
also, shouldn't this be #ifdef SOLARIS instead [if you intend to keep the 
#ifdef]

Akkana/Pavlov could you review this patch?

Comment 7

[Ann Adamcik]

Attachment: Fixes crash for numpad keys and F11/F12 keys

Comment 8

[Ann Adamcik]

I wasn't able to reproduce the crash with a linux box.  The latest attachment
fixes the crash with both the numpad keys and F11/F12 keys.

Review requested.

Comment 9

[Christopher Blizzard (:blizzard)]

Two comments:

o You are making gdk changes here which means that every solaris build will
break that uses stock gdk.  I can't approve this.

o You are adding DOM VK symbols.  Are those legitimate symbols?

Comment 10

[Akkana Peck]

nsIDOMKeyEvent is generated from MouseKeyEvent.idl (see the "AUTO-GENERATED. DO
NOT EDIT!!!" comment near the top).  New VK_ key symbols should be added in the
idl, not in the generated .h file.  See
http://www.mozilla.org/editor/changeidl.html for info on how to generate .h
files from DOM idl files (unfortunately, it requires Windows :-( ).  And we
unfortunately have another set of keybindings in nsGUIEvent.h -- any new key
symbols added in one place have to be added in the other place. :-(

There is no policy at this point on whether it's kosher to add keycodes that
only work on one platform.  Currently, I don't believe we have any such symbols,
and we've been avoiding adding any hoping that the dom event spec folks will
come up with some way of dealing with this issue (don't hold your breath, though).

I'm also confused about why the comment says they're F11 and F12 keys, but we're
using DOM_VK_F36 and _37 instead of DOM_VK_F11 and _F12.  Can't we change
nsGtkEventHandler to return DOM_VK_F11 when the user presses the key marked F11,
instead of adding a new symbol DOM_VK_F36 as a "second" F11?

Comment 11

[timeless]

Akk, "F11" and "F12" typically don't get the numeric values after F10 even on 
pc keyboards. Instead you usually have F1..F10, gF1..gF10 [the ones on the 
left], and then F11 F12 or something.  This is really getting stuck near the 
other bug about vendor specific bindings.  We need some answers from a dom spec 
writer.

Comment 12

[Akkana Peck]

Timeless: I'm not sure I understood your comment, or what the "gF*" symbols are.
 I should clarify that I'm not worried one way or the other about what the
numeric values are; I'm just saying that if the key on the keyboard is marked as
F12, then our event system should map a press of that key to the keycode
corresponding to symbol DOM_VK_F12, regardless of what the numeric value of
DOM_VK_F12 is or of what code the underlying platform-specific widget code
(linux/gtk or solaris/gtk or windows or whatever) may apply to that key.

Comment 13

[timeless]

ok, well. assuming we try to be true to that.
dom_vk_f1..dom_vk_f12 [top fkeys]
do we do
dom_vk_lf1..dom_vk_lf10 [left fkeys]

For keyboards with both? [Especially Sparcs] [Actually right now we have some 
mapping for those keys, which i think conflicts with your suggestion]

Comment 14

[Ann Adamcik]

This seems to have been fixed in the trunk by the patch for 53667.  Need to add
it to the OEM branch for Solaris.

Comment 15

[Viswanath Ramachandran]

Since Don has left, Vishy is taking his bugs in bulk, pending reassignment.
thanks,
	Vishy

Comment 16

[Chris McAfee]

marking fixed for trunk.  Sun OEM branch should file another reminder bug, or
reassign and reopen if you want this as a placeholder.

Comment 17

[sairuh (rarely reading bugmail)]

if someone else with solaris could verify this issue, that'd be great. thx.

Comment 18

[Ann Adamcik]

This was checked into the OEM branch last month.

Comment 19

[Tracy Walker [:tracy]]

mass remove verifyme requests greater than 4 months old