nsJSEventListener::HandleEvent potentially is using an incorrect script context when executing an event handler. A comment to this effect was first introduced in http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/dom/src/events&command=DIFF_FRAMESET&file=nsJSEventListener.cpp&rev2=1.17&rev1=1.16 [Look for the comment starting '// XXX This doesn't seem like the correct context'] The DOM_AGNOSTIC landing has moved some of this code - look for a FIXME comment referencing this bug in nsJSEventListener.cpp. See also bug 339649 for another place where the context may be incorrect.

Seems like we should figure out what's going on here for 1.9. Assigning to smaug for now.

Smaug - have you had a chance to look into this?

Haven't thought about this too much yet, but currently I think using mContext is the right thing to do.

Can anyone think of reason why the context could be wrong? When nsJSEventListener::HandleEvent is called there is a context pushed to stack. That context is taken from the scriptglobalobject of the ownerdocument of the currentTarget. Similarly when nsEventListenerManager::AddScriptEventListener creates nsIJSEventListener, it gets the context from the object to which the listener is registered. So the context should be the same. The only case when context taken from the stack and mContext might differ seems to be the case that bug 339649 describes (and hopefully it will be fixed there). So I'd just remove the comment in nsJSEventListener.

Comment on attachment 271992 [details] [diff] [review] Remove XXX but add an assertion Yeah, makes sense to me.