66742 - [pp]Application crashes after closing a onkeydown event dialog

Status: VERIFIED FIXED

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

Description

[Chris Petersen]

Build: 2001012611
Platform: Mac only
Expected Results: Closing dialog should not crash the application.
What I got: Event handler is processed and dialog is displayed. Closin the
dilaog causes the crash.

Steps to reproduce:

1) Open http://slip/projects/marvin/html/a_onkeydown.html

2) Place focus on link by mousing down on it. Press a key on the keyboard.

3) The event onkeydown will be processed. Mouse up to remove focus off link.

4) Close the alert dialog. A crash should occur.

Comment 1

[Chris Petersen]

Attachment: A macsbug report of the crash

Comment 2

[Chris Petersen]

Attachment: A macsbug report of the crash

Comment 3

[Hixie (not reading bugmail)]

Reassigning QA Contact for all open and unverified bugs previously under Lorca's
care to Gerardo as per phone conversation this morning.

Comment 4

[joki (gone)]

I'm gonna need some Mac help on this one.  I'll set milestone to 0.9.1 since its 
a crash and see if we can get it taken care of.

Comment 5

[Marcia Knous [:marcia]]

nominating for dogfood (from sdagley's list of bugs that are good candidates for 
our next release) 

Comment 6

[bsharma]

There is a javascript code that should open the Alert dialog box. But, this 
dialog is not opened.

Comment 7

[Kathleen :Brade]

Attachment: double-check before sending keypress

Comment 8

[joki (gone)]

Looks good to me.

r=joki

Comment 9

[Kathleen :Brade]

The attached patch fixes two problems.
First, it ensures that a keypress event goes to the same widget/window that the
keydown event goes to.  (note: joki says it's ok if the keyup goes to a
different window)

Second, this patch ensures that we don't use a focused widget which might have
been closed (and therefore deleted which could lead to a crash).  It does this
by relying on a 2nd call to gEventDispatchHandler.GetActive() to not return a
bad window/widget.

Comment 10

[Simon Fraser [no longer active]]

sr=sfraser if you fix the spacing.

Comment 11

[Kathleen :Brade]

spacing adjusted to match code immediately around it; fix checked in

Comment 12

[Dave Alverson]

This change does not build under gcc on Mac OSX:

nsMacEventHandler.cpp: In method `PRBool 
nsMacEventHandler::HandleKeyEvent(EventRecord &)':
nsMacEventHandler.cpp:1075: jump to case label
nsMacEventHandler.cpp:1060:   crosses initialization of `class nsWindow * 
checkFocusedWidget'

You need to move the declaration of checkFocusedWidget to a better place.

Comment 13

[Kathleen :Brade]

Dave Alverson--could you please apply a patch or a code snippet that does 
compile?

Comment 14

[Dave Alverson]

Attachment: patch that builds under gcc/MacOSX

Comment 15

[Kathleen :Brade]

new patch checked in; thanks!

Comment 16

[Madhur Bhatia]

Tested bug on win2000 BuildID: 2001061304
and macOS 8.6 BuildID:2001061308

The crash does not occur anymore.
Marking bug as verified.