175220 - Complex frames/framesets does not work in V1.01 upwards

Status: RESOLVED FIXED

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

Description

[Charles de Swardt]

complex frames/framsets does not work properly from version 1.01 upto and
including version 1.2b. This includes Linux, Windows and OS/2. 

This used to work in versions before 1.01. The bug is also reproduced in the
latest IBM webbrowser and also Phoenix

Comment 1

[Charles de Swardt]

Attachment: Frameset testcase that works in IE and Mozilla 1.0.0 but not in 1.0.1 upwards

Comment 2

[Boris Zbarsky [:bzbarsky]]

To reproduce, save the testcase zipfile, unzip it, rename "Test.html" to
"test.html" so it actually works, then open up 0.html.  Notice the blank space
in the bottom right.  Load 1.html (loaded by 0.html).  Notice the same problem.
 Load 2.html (loaded by 1.html).  Now the bottom right shows CNN.  So the bug is
that 0.html and 1.html do not show the CNN page...

This is due to our limit on the frame-nesting depth (see bug 126466 and bug 8065
for motivation/discussion/patches).  There's no good solution here,
unfortunately....

Comment 3

[Mike Kaply [:mkaply]]

any idea why they picked 8?

That seems awfully low.

Comment 4

[Mike Kaply [:mkaply]]

*** Bug 175227 has been marked as a duplicate of this bug. ***

Comment 5

[Boris Zbarsky [:bzbarsky]]

See bug 126466 comment 12.

Comment 6

[Mike Kaply [:mkaply]]

Attachment: Remove code that limits nested frames

Since we limit the total number of docshells, we shouldn't need to limit nested
frames.

Comment 7

[Adam Lock]

This code was put in there to catch runaway JS and other bustage. 8 frames deep
was  picked as cutoff to catch such content but leave sane content alone. See
also bug 136580.

Where is the limit on the number of docshells that makes this code obsolete?

Comment 8

[Mike Kaply [:mkaply]]

I had this chat with jkeiser on the IRC and he swore it was there.

Comment 9

[Adam Lock]

John, where is this max docshell limit that makes it okay to remove this nesting
check? I had a search for 'max' in nsDocShell.cpp & nsWebShell.cpp and didn't
see anything relevant.

Comment 10

[John Keiser (jkeiser)]

The limit is right there next to the nested frames code.  #define
MAX_NUMBER_DOCSHELLS 100

http://lxr.mozilla.org/mozilla/source/content/base/src/nsFrameLoader.cpp#76

Comment 11

[Johnny Stenback (:jst)]

Comment on attachment 108414 [details] [diff] [review]
Remove code that limits nested frames

I never liked this limit in the first place, so I'm glad to see it go.

sr=jst

Comment 12

[John Keiser (jkeiser)]

Fix checked in.  Does this need to be in 1.0?  If so, please go through the
approval process and such.  It doesn't seem important enough to me, but I'm not
the one with the customer who wants it :)

Comment 13

[Boris Zbarsky [:bzbarsky]]

This is fixed.