315056 - [FIX]Dynamically creating an iframe within another, causes a neverending page load

Status: RESOLVED FIXED

(Core :: Layout: Images, Video, and HTML Frames, defect, P3)

Description

[hulebjoern]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051103 Firefox/1.5
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051103 Firefox/1.5

Dynamically creating an iframe in the body of another iframe or frame of a frameset, be it using appendChild or innerHTML, causes Firefox to never stop loading the page. This does not happen when the iframe is NOT being created within another frame. IE6 does not have this problem.

Reproducible: Always

Steps to Reproduce:
1. Dynamically create an iframe in the body of another iframe or frame from a frameset

Actual Results:  
Firefox never shows the page as being completely loaded, even though it is.

Expected Results:  
Firefox should show the page as having finished loading.

Comment 1

[Remco]

Attachment: local testcase creates iframe inside iframe

Hi,

I've come up with an simple example that shows this problem.
It works fine in firefox 1.0.x but not in any of the 1.5 rc's.
The file is called 'iframe_testcase.html' and it calls itself in a nested iframe until the iframe's parent is an iframe. You should get a document with an iframe in an iframe.

Also notice what happens when you right-click inside the iframe!!

Regards,
Remco

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Attachment: working testcase

Comment 3

[:Gavin Sharp [email: [e-mail]]]

Maybe the testcase I've added isn't correct, but it seems to work for me. Do you see the issue with my testcase?

Comment 4

[Remco]

If you'll just save my attachment locally and open it in firefox 1.5 you'll see that it doesn't dynamically create an iframe WITHIN an iframe like it does in firefox 1.0.x


(In reply to comment #3)
> Maybe the testcase I've added isn't correct, but it seems to work for me. Do
> you see the issue with my testcase?
> 

Comment 5

[Remco]

(In reply to comment #3)
> Maybe the testcase I've added isn't correct, but it seems to work for me. Do
> you see the issue with my testcase?
> 

Hi Gavin,

Thanx for your reply.

The issue doesn't show in your testcase becouse isn't just the created iframe, but the page loaded in that iframe that creates a new iframe.

I've put a testcase on our website for convenience:
http://www.smartology.nl/testcase/testcase.html

Comment 6

[:Gavin Sharp [email: [e-mail]]]

I can confirm that there's been a change in behavior since 1.0.x. Could you try and narrow down a regression range using the builds at http://archive.mozilla.org/pub/firefox/nightly/ ? If not, I will do so later today.

Comment 7

[Remco]

(In reply to comment #6)
> I can confirm that there's been a change in behavior since 1.0.x. Could you try
> and narrow down a regression range using the builds at
> http://archive.mozilla.org/pub/firefox/nightly/ ? If not, I will do so later
> today.
> 

2005-03-09 seems to be OK
2005-03-10 has the bug (so it would seem on my system)

Comment 8

[:Gavin Sharp [email: [e-mail]]]

Which builds were you testing, mozilla1.8 or trunk?

Comment 9

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #8)
> Which builds were you testing, mozilla1.8 or trunk?

Oops, silly question, 1.8 didn't exist then.

Comment 10

[:Gavin Sharp [email: [e-mail]]]

Ah, this would be from a change introduced in bug 228829, then. That bug intentionally limited the number of same-url nested frames to 1, matching IE's behavior.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

That being said, is this bug's summary correct? I don't see a "neverending page load" with your testcase.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Yep, like Gavin I don't see anything like "neverending page load", either with current trunk or 2005-03-10 builds.  Could you please clearly explain what the problem is?

Comment 13

[hulebjoern]

(In reply to comment #12)
> Yep, like Gavin I don't see anything like "neverending page load", either with
> current trunk or 2005-03-10 builds.  Could you please clearly explain what the
> problem is?
> 

Okay, I've created the test case that shows the problem perfectly. However, it does not appear that I can attach two (2) files as one (1) test case, so I didn't upload it. But here are the two pages' source code:

---------------------------------------------------------------
PAGE 1: test.html
---------------------------------------------------------------
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Frameset//EN">

<html>

	<head>
		<meta http-equiv="content-type" content="text/html;charset=utf-8">
		<title>Test</title>
	</head>

	<frameset rows="*" border="0" frameborder="no" framespacing="0">
		<frame name="topFrame" src="testframe.html" noresize="noresize" scrolling="yes">

	</frameset>

</html>
---------------------------------------------------------------
PAGE 2: testframe.html
---------------------------------------------------------------
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">

<html>

	<head>
		<meta http-equiv="content-type" content="text/html;charset=utf-8">
		<title>Untitled Page</title>
		<script type="text/javascript">
		function createFrame() {
			var iframe = document.createElement("iframe");
			document.body.appendChild(iframe);
		}
		</script>
	</head>

	<body onload="createFrame()">
	</body>

</html>

Comment 14

[Boris Zbarsky [:bzbarsky]]

> However, it does not appear that I can attach two (2) files as one (1) test
> case

Right.  So you attach "page 2" first, change "page 1" to point to the attachment version of "page 2", and attach it.

Pasting into the bug is no good -- very often the formatting changes from the copy/paste roundtripping change things such that bugs either appear or disappear.

Comment 15

[hulebjoern]

Attachment: testcase that dynamically creates an iframe inside a frameset (page 2)

Comment 16

[hulebjoern]

Attachment: testcase that dynamically creates an iframe inside a frameset (page 1)

Comment 17

[Gabriel Sjoberg]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060301 Firefox/1.6a1 ID:2006030120

I can confirm this behavior in the above build.  Does this bug exist on Linux as well?

Comment 18

[David Wainwright]

Attachment: creates an iframe with src="test2.html" (my other attachment)

Opening this page causes an infinite reload in firefox. Platform is windows XP Pro.
This attachment depends on the other file test2.html

Comment 19

[David Wainwright]

Attachment: test1.html creates an iframe sourcing this file

When this file is specified as the src value for an iframe in another page, firefox reloads indefinitely.

Comment 20

[David Wainwright]

Attachment: causes an indefinite reload if another page uses an iframe whose src is this file

Comment 21

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed patch

Comment 22

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 234932 [details] [diff] [review]
Proposed patch

So the basic problem here is that the <frame> is done loading before the <iframe> child starts loading, but the latter happens before we call DocLoaderIsEmpty() on the frameset docloader.  The frameset doesn't fire onload then because there is some descendant docloader that's busy (even though it's a descendant of a docloader that no longer has mIsLoadingDocument set).  But the problem is that when the iframe finishes loading it calls DocLoaderIsEmpty() on the <frame>, which is ignored (because mIsLoadingDocument is false).

This patch fixes the problem by just having the frameset fire onload right after the frame in this case -- loads still happening in children of docloaders that no longer have mDocumentLoading are ignored, which is consistent with what we do for other loads in such docloaders (that is, this makes <iframe> behave like <img> already did).

The other option, if we want to delay the frameset onload in this case until after the <iframe> finishes loading, is to keep IsBusy() as-is, but change DocLoaderIsEmpty() to call DocLoaderIsEmpty() on the parent even if mIsDocumentLoading is false.  I could do that, but this approach seemed more reasonable to me (again, because it makes <iframe> behave the same way as <img>).

Comment 23

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 234932 [details] [diff] [review]
Proposed patch

ok... makes sense. I think.

Comment 24

[Boris Zbarsky [:bzbarsky]]

Fixed.