20785 - [DOGFOOD] dialog controls bust 2nd time dialog is displayed

Status: VERIFIED FIXED

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

Description

[Chris Waterson]

For example, open the prefs dialog, click "ok" to close it. Re-open, "ok" and
"cancel" don't work.

This is either related to my master document brutal sharing that I landed
tonight, or the recent script sharing stuff that brendan has landed. Setting
the "Disable XUL Cache" pref allows you to work aroun the problem.

Comment 1

[Chris Waterson]

Ok, it looks like what happens here is that the function object forgets that
it's a function the second time we try to use it; ie., the sharing case when it
is simply "bound" rather than compiled.

Updating test case. To reproduce:

1. Open "preferences" dialog
2. Close with the "cancel" button.
3. Re-open "preferences" dialog
4. Close with the "cancel" button.

Expected behavior: dialog closes.
Actual behavior: either...

1) Nothing happens. I've debugged this case a bit and discovered that
JS_TypeOfValue() != JSTYPE_FUNCTION on the shared function object the second
time the "cancel" onclick handler is invoked.

2) A bizarre JS Exception is thrown, indicating a parameter mismatch. I've
debugged this a bit as well, and what's happening here is that the function
object is -still- a function object, but is pointing to some completely
different native function, whose sanity checking code detects a parameter
mismatch.

brendan: are we failing to properly root the function object for the garbage
collector?

Comment 2

[Chris Waterson]

brendan, my guess is that we need add a GC root in
nsXULElement::SetCompiledEventHandler(), just like we do in
nsXULPrototypeScript::Compile().

I'm gonna try that...

Comment 3

[Chris Waterson]

Attachment: proposed fix

Comment 4

[Chris Waterson]

So the attached patch adds a named root for each prototype event handler.
Unfortunately, I think there are still some more binding problems. The second
time we bring up the prefs window, the JS interpreter can't resolve
'doCancelDialog()' from the onclick handler :-(

http://lxr.mozilla.org/seamonkey/source/xpfe/global/resources/content/win/platf
ormDialogOverlay.xul#39

debugging that now...

Comment 5

[Chris Waterson]

brendan: in nsJSContext::BindCompiledEventHandler(), is JS_DefineProperty
"enough"? Don't we need to still need to do some work to set up scoping?

Comment 6

[Brendan Eich [:brendan]]

Exactly right -- need to clone the function object if its parent isn't the event
handling object!  D'oh.  Patch coming up.

/be

Comment 7

[Brendan Eich [:brendan]]

Attachment: my part of the fix

Comment 8

[Jan Carpenter]

verified - WinNt, Win98, Linux, Mac