362340 - error event handler in invisible frames is not called

Status: NEW

(Core :: DOM: Events, defect, P5)

Description

[Wladimir Palant]

If I load a document into a hidden frame (display:none) the JavaScript executes correctly. However, if this frame has window.onerror defined this handler never gets called. Making the frame visible fixes the problem.

This came up when writing a testcase for bug 252326.

Comment 1

[Wladimir Palant]

Attachment: Testcase

Open this testcase. If you don't see an alert message pop up then the error handler hasn't been triggered. Removing "display:none" from the frame fixes it. Tested in:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20061117 Minefield/3.0a1
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20061010 Firefox/2.0

Comment 2

[Wladimir Palant]

Found the reason for this behavior. Script errors are handled by the function NS_ScriptErrorReporter in nsJSEnvironment.cpp which requires a valid presContext to dispatch the event. In the case of an invisible iframe the presContext is null.

Comment 3

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

PresContext shouldn't be needed, I think.
Wladimir, will you post a patch? If not, assign this bug to me.

Comment 4

[Wladimir Palant]

Attachment: Possible solution

Yes, I also cannot see why we depend on presContext here. I looked and there is in fact some code calling nsEventDispatcher::Dispatch without checking the presContext parameter first (there is some other code that does - these event probably won't be delivered in invisible frames either). I tried removing the presContext check and it fixed the problem for me without causing any obvious problems. I don't know the implications of this change however, maybe jst can tell...

Comment 5

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

Comment on attachment 247071 [details] [diff] [review]
Possible solution

There shouldn't be need to get the prescontext from docshell at all, so you could simplify the code even more.

Comment 6

[Wladimir Palant]

Smaug, I don't know what this presContext is used for so I better wait for jst's comments. There is at the very least the possibility now to make NS_ScriptErrorReporter use NS_HandleScriptError as suggested in the comment before the function - the only concern isn't any more with this change.

The other code that checks presContext before dispatching events does in fact show the same behavior. Invisible frames don't receive DOMFrameContentLoaded events (nsDocument::DispatchContentLoadedEvents) and HTMLInputElement.click() in invisible frames doesn't create click events (nsHTMLInputElement::Click). I have testcases, can attach them here if requested. There is probably more of this but I don't have time right now to check everything.

Comment 7

[Johnny Stenback (:jst)]

Comment on attachment 247071 [details] [diff] [review]
Possible solution

Yeah, this is what we want. AFAIK event dispatch is always ok to do even if there is no pres shell/context around.

r+sr=jst

Comment 8

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

but :
> There shouldn't be need to get the prescontext from docshell at
> all, so you
> could simplify the code even more.

Comment 9

[Joachim Rousseau]

Dup of bug #598438 ?

Comment 10

[Wladimir Palant]

How so? These two bugs seem unrelated to me.

Ouch, I left a reviewed patch bitrot here for four years...

Comment 11

[Joachim Rousseau]

Oups, yes, very old one. But invisible frames /iframes ... sounded similar?
At least I reminded you about this one ;)

Comment 12

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1472046

Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.