444165 - iframe onload event does not fire if the src URL is invalid

Status: NEW

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

Description

[Jaymz]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9) Gecko/2008052906 Firefox/3.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9) Gecko/2008052906 Firefox/3.0

When a page contains an iframe with its src attribute set to an invalid URL [e.g. a URL for which DNS look-up failed], the iframe's onload event does not fire.

Reproducible: Always

Steps to Reproduce:
View the attached testcase in Firefox, which includes the following line of HTML:

<iframe src="http://invalid-url" onload="alert('iframe loaded!')"></iframe>
Actual Results:  
The iframe's onload event does not fire; the alert does not show.

Expected Results:  
The iframe's onload event should fire, even though the domain cannot be found.

Comment 1

[Jaymz]

Attachment: iframe onload test case

Comment 3

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

What is the regression range for this? Did implementing about:neterror cause this?
(IMO error event should be fired, not load) 

Comment 4

[Jaymz]

I have verified that this problem exists in the most recent versions of both Firefox 2 and 3. Unfortunately, I cannot test any other versions for myself.

Comment 5

[Jaymz]

In case somebody is curious... I implemented a workaround for this bug by handling Gecko's DOMFrameContentLoaded event on the document that contains the iframe.

Comment 6

[Bjarne (:bjarne)]

Attachment: Testcase

While writing a mochitest for bug #461413 I crashed into this issue. The attached test behaves rather different in Opera and FF. Would someone try it on MSIE and other relevant UAs?

On FF, the sequence of alerts are : 1,2,3 (4 never happens)

On Opera 9.6 on my Ubuntu-system, the sequence is : 2,3,3,4 (i.e. onload on the iframe is called after the body-onload)

Comment 7

[Bjarne (:bjarne)]

(In reply to comment #5)
> In case somebody is curious... I implemented a workaround for this bug by
> handling Gecko's DOMFrameContentLoaded event on the document that contains the
> iframe.

I am curious...  :)

Feel free to attach your workaround or email it to me.

Comment 8

[Bjarne (:bjarne)]

(In reply to comment #6)
> Created an attachment (id=349159) [details]
> Testcase 
> 
> While writing a mochitest for bug #461413 I crashed into this issue. The
> attached test behaves rather different in Opera and FF. Would someone try it on
> MSIE and other relevant UAs?
> 
> On FF, the sequence of alerts are : 1,2,3 (4 never happens)
> 
> On Opera 9.6 on my Ubuntu-system, the sequence is : 2,3,3,4 (i.e. onload on the
> iframe is called after the body-onload)

Hmm...  I suddenly realized that my test is slightly different than the original one since it dynamically changes the content of the iframe. Furthermore, it may also be the case that when  dynamically loading new content into an iframe, handlers of the iframe is reset, and would thus not fire. Have to look into this more closely...

Comment 9

[Bjarne (:bjarne)]

Attachment: Clearer and simpler testcase

This test does not change the iframes dynamically but rather loads three separate with an onload-handler for each.

On FF, the sequence is 1,3,4 (skipping 2 with the error-page)
On Opera the sequence is 4,1,2,3
On MSIE 7.0 on Windows XP Home the sequence is 1,2,3,4

Comment 10

[Bjarne (:bjarne)]

W should try to coordinate behaviour with MSIE to simplify life for developers building fault-tolerance into their Ajax-apps. Requesting this to be fixed for 3.1...

Comment 11

[Jaymz]

Attachment: Gecko-specific workaround for iframe onload bug.

(In reply to comment #7)
> I am curious...  :)
> 
> Feel free to attach your workaround or email it to me.

Sorry so late... I haven't been checking this thread! Workaround attached.

Comment 12

[Bjarne (:bjarne)]

(In reply to comment #11)
> Created an attachment (id=354851) [details]
> Gecko-specific workaround for iframe onload bug.
> 
> (In reply to comment #7)
> > I am curious...  :)
> > 
> > Feel free to attach your workaround or email it to me.
> 
> Sorry so late... I haven't been checking this thread! Workaround attached.

Better late than never. :) Thanks - I'll see what can be done.

Comment 13

[Bjarne (:bjarne)]

This one has been completely forgotten for quite some time...  but I still see this issue in my trunk-build. Is this something we should consider fixing for 2.0?

Comment 14

[Boris Zbarsky [:bzbarsky]]

Imo, no.

Comment 15

[Benjamin Smedberg]

Not blocking or wanted for Firefox 4.

Comment 16

[Bjarne (:bjarne)]

-> default owner

Comment 17

[Staś Małolepszy :stas]

I ran into this bug while working on the localization of the neterror page in Gaia (it's all client side).  The page doesn't have a valid URL and the load event never fires.  Is there any chance this could be easily fixed?

Comment 18

[Staś Małolepszy :stas]

bsmedberg, do you know anyone who could work on this bug?

According to my numbers, there's a win of around 30ms in the startup time for the default language if we can delay the localization of the page until the load event compared to DOMContentLoaded.  (In Gaia, the HTML is pretranslated on build time into the default language, so we don't need to run the client side localization right away.)

Comment 19

[Benjamin Smedberg]

stas, I'm not sure that this is actually a bug. We shouldn't be firing load events when the page fails to load, should we?

And also the bug summary doesn't really match comment 0: are we talking about a case where the URL is actually invalid? "htp://foo" or just one where the URL is valid but DNS fails to resolve "http://foobarmissingnothing.orgvoa"?

Why does any of this affect gaia l10n or the choice of whether you use DOMContentLoaded or the load event?

Comment 20

[Staś Małolepszy :stas]

Thanks for taking a look at this bug!

There has been some work in bug 882186 to make the neterror page look nice in Gaia.  The effect of this work is that what the users see is no longer the page that Gecko and Toolkit have in docshell/resources/content/netError.xhtml, but instead an HTML document displayed by the Gaia system app.

Before, in order to localize the neterror page, we'd just rely on Toolkit's DTD files and Gecko's localization process would take care of it entirely.  The downside was that the strings required for localization of this page needed to be built into B2G.  So we had two places in which to configure the list of available locales:  for B2G, we'd use things like all-locales, just like we do in desktop Firefox, to provide the localization of the neterror pages.  For pretty much everything else (all other Gaia apps), the localization happens on the client side via the shared/js/l10n.js library, using .properties files.  So bug 882186 made this look better and also made things easier to configure.

Now, why this load event matters for performance?  In Gaia, we do a bunch of build time optimizations to launch apps faster.  One such optimization is pretranslating the HTML into the default locale that the given Gaia build has been set to.  For instance, with GAIA_DEFAULT_LOCALE=fr, all apps that go into the zip packages will have their HTML pretranslated into French.  This way, as long as the user's preferred locale is the same as Gaia's default one, we don't have to 1) do any IO for the localization files nor to 2) modify and reflow the DOM by injecting translations into it during the app's launch.

With all these optimizations we want to reduce the time between the tap on the icon and the moment something appears on the screen, localized.  Many apps still wait for the 'localized' event fired by l10n.js in order to run their own initialization functions.   For instance, the settings app will check the status of the SIM card only after 'localized' is fired.  This all happens asynchronously, and well after the user has a chance to see the UI of the freshly-launched app, so it doesn't matter much for performance.

In fact, it matters so little that we could further delay the initialization of the localization for the default locale (until after 'load' fires).  The benefit is that we run fewer things while the page is still being rendered, and consequently the launch is faster.  Everything still looks okay, because the HTML has been pretranslated to the default locale (again, all of this only applies to the case where the Gaia's default locale is also the user's preferred locale; if it's not, there are other optimizations in place to reduce the loading time).

Waiting until after the 'load' event fires works well for all apps, but it doesn't work for the neterror page.  I don't want to special-case it in the l10n library (although I could be convinced otherwise), so the naive solution is to wait for DOMContentLoaded everywhere.  This, however, slows launch times for the default locale by approximately 30ms.

I'm not sure if the fact that the neterror page doesn't fire the 'load' event is caused by this bug or not.  In Gaia, it's a page like all others (although heavily restricted by CSP), so it would make sense to have it fire the event.  However, I can also see your point about this not being semantically correct, so I'm just looking for ideas at this point.

One other I came across is this comment here:

  "This is because error pages are loaded as LOAD_BACKGROUND, which means that onload handlers will not be executed."
  http://hg.mozilla.org/mozilla-central/file/a57801cec8be/docshell/resources/content/netError.xhtml#l374

If this bug is not valid, maybe the above comment is a clue?

Comment 21

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Assigning P2 based on 30ms global slowdown. 

I'm a little unclear whether that's proper behavior on the neterror page or not, even after digesting comment 20, but I think we might want to explore special casing as a possibility to remove the "lowest common denominator" issue.

Comment 22

[Zibi Braniecki [:zbraniecki][:gandalf]]

I retested this today, and being able to use 'complete' readyState in Gaia would give us avg. 50ms firstPaint improvement.

Can we get an evaluation of how hard would it be to get this fixed?

Comment 23

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

Fixing this is not entirely risk-free.

Do you get pageshow event?
http://mxr.mozilla.org/mozilla-central/source/layout/base/nsDocumentViewer.cpp?rev=e50544d1d32d&mark=957-957,1019-1019#956