1265590 - Can we get rid of GetContextForEventHandlers?

Status: RESOLVED DUPLICATE of bug 1449631

(Core :: DOM: Events, defect)

Description

[Boris Zbarsky [:bzbarsky]]

With the patches in bug 1265072, I believe there are only two _callers_ of GetContextForEventHandlers:

1)  The ifdef DEBUG block in EventDispatcher::Dispatch.  Not sure what this is trying to assert and whether we really care about it.

2)  DataTransfer::GetDataAtInternal which is simply using it to get to nsIGlobalObject; it can do that just as well by doing GetWindowIfCurrent() or GetOwner() (possibly with CheckInnerWindowCorrectness, depending on how it wants to handle non-current inners). 

Am I missing something about how this function should be used?

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

(1) is just trying to get some sane global or check whether we used to have a global.
If there hasn't been any global ever, like in XBL documents case, we bypass the warning/assertion.
IIRC C++ XBL code uses load event listener in an unsafe way, if that was exposed to scripts, but it is not, at least not in any trivial way.

2) could indeed use something like GetWindowIfCurrent, whatever it is actually called.



The method was added in Bug 435656 to deal with nsCxPushing consistently, so that we'd fail fast in case there wasn't a global to use (but the event target used to be bound to some global in some way).

Comment 2

[Boris Zbarsky [:bzbarsky]]

Hmm.  I guess GetWindowIfCurrent() is on DOMEventTargetHelper, not on EventTarget, with bug 1265072.  I guess we could try pushing up virtual things like that to EventTarget...

Basically, it seems silly to go through the scriptcontext to get the global, since the "get the scriptcontext" stuff gets the global first...

Comment 3

[Boris Zbarsky [:bzbarsky]]

Fixed in bug 1449631 (part 4 of that bug).