1) Load the URL in Mozilla. 2) When it has finished loading, press reload. 3) Observe. Sorry I can't be more detailed, but I don't even know how to describe this. I will attach a screenshot.

use the menus there for a while, and the left menu frame will also be filled with little white/olive colored pieces of "corner" gifs. Saw that in last sundays build. Frame bug perhaps.

Dividing up Claytons bugs to triage

I see the bad behavior on WINNT with todays build. It only happens on reload. hitting return in the URL bar works fine. Is this bug cache related? Reassigning to buster.

I just pulled and build (debug) on WinNT, and I don't see this at all. Kevin, did you test commercial or debug build? Chris P: can you verify on Win2k or any other windows platform?

Fun things to try: - Cover the page with another window - Drop down the dropdown listbox: when I remove the listbox, I see parts of the tabs but with the colors messed up.

ok, I see the bug now. nice! will investigate

cc-ing eric because the document is a big frameset. have you seen anything like this before?

changed summary, added RTM nomination, assigning to pollmann. After the initial successful load, what seems to be happening is urls are being loaded into frames somewhat randomly. Hit the reload button multiple times, and the url in any given frame may eventually change. I don't know if this is an HTML Frames issue, a session history issue, or a cache issue (unlikely.) I'm not 100% sure we need this for RTM, but we really need to understand it better. Is this a rare case, or a common case? What's special about this frameset?

I'll take a look at this - there are a number of major regressions in form submission and frameset loading assigned to me right now :S

See also bug 49857

Eric: I'm available to help. Let me know if there is anything I can do to help you track down these regressions.

bug 46646 and bug 49857 sure sound like duplicates. based on the frequency of occurance and the lack of a workaround (once the page is horked, there's no obvious way to unhork it), and the fact that it is a regression, I strongly recommend rtm++.

At least some of these could be related to bug 53708 / bug 53975. In those cases, session history is detecting that the load should come from history when it shouldn't and pre-empting the load with a previous URL. I've duped 53975 on 53708, and assigned that one to Nisheeth, since he's working on bug 48382, which is almost assuredly also related. It would be a developer's dream-come-true if one fix took care of all of these.. :)

See bug 53708 for a reduced test case - the problem there lies in the logic in nsDocShell::LoadURI where it detects if the load should come from session history or not (false detection that it should come from session history)

This looks like a problem with the order frames are stored in session history. Initial page: http://www.securityfocus.com/ a) http://www.securityfocus.com/frames/empty.html?&_ref=119562203 b) http://www.securityfocus.com/frames/?&_ref=119562203 0) http://www.securityfocus.com/frames/logo.html 1) http://www.securityfocus.com/frames/ad.html?group=home 2) http://www.securityfocus.com/frames/top.html?focus=home 3) http://www.securityfocus.com/focus/home/menu.html 4) http://www.securityfocus.com/frames/upper_left.html 5) http://www.securityfocus.com/frames/upper_edge.html 6) http://www.securityfocus.com/frames/upper_right.html 7) http://www.securityfocus.com/frames/left_edge.html 8) http://www.securityfocus.com/focus/home/home.html 9) http://www.securityfocus.com/frames/right_edge.html 10) http://www.securityfocus.com/frames/lower_left.html 11) http://www.securityfocus.com/frames/lower_edge.html 12) http://www.securityfocus.com/frames/lower_right.html Reload (URLs come from session history): http://www.securityfocus.com/ a) http://www.securityfocus.com/frames/empty.html?&_ref=119562203 b) http://www.securityfocus.com/frames/?&_ref=119562203 0) http://www.securityfocus.com/frames/logo.html 1) http://www.securityfocus.com/frames/ad.html?group=home 2) http://www.securityfocus.com/frames/top.html?focus=home 3) http://www.securityfocus.com/focus/home/menu.html 4) http://www.securityfocus.com/frames/upper_left.html 5) http://www.securityfocus.com/frames/upper_edge.html 6) http://www.securityfocus.com/frames/upper_right.html 7) http://www.securityfocus.com/frames/left_edge.html 8) http://www.securityfocus.com/frames/lower_left.html 9) http://www.securityfocus.com/frames/right_edge.html 10) http://www.securityfocus.com/focus/home/home.html 11) http://www.securityfocus.com/frames/lower_edge.html 12) http://www.securityfocus.com/frames/lower_right.html Notice how the URLs for frames 8 and 10 are swapped in the session history load. I stepped though this in a debugger, and the mChildOffset seems to be okay for the frame when loading the page from session history, so it must be when the page is initially loaded that we store the wrong URL at the wrong offset in session history. Handing this to Radha...

Yep, here's the bug: NS_IMETHODIMP nsSHEntry::AddChild(nsISHEntry * aChild, PRInt32 aOffset) { NS_ENSURE_TRUE(aChild, NS_ERROR_FAILURE); NS_ENSURE_SUCCESS(aChild->SetParent(this), NS_ERROR_FAILURE); PRInt32 childCount = mChildren.Count(); if (aOffset < childCount) mChildren.InsertElementAt((void *) aChild, aOffset); else mChildren.AppendElement((void *)aChild); NS_ADDREF(aChild); return NS_OK; } This method assumes that the frames come in in order, which is not guaranteed to be the case. Set a breakpoint in the above method right after childCount is set with the condition (aOffset > childCount). Notice a bad thing will happen in this case... If the load order is: 0 3 2 1 The frames will be stored in session history as: 0 1 3 2 (If they come in really mixed up order, as happens when you step through this in a debugger, things get really scrambled) One was of fixing this (may not be the most efficient as I haven't though about it a lot) would be to grow the list if an entry comes in before the list is long enough, then set the entry at the correct index instead of inserting.

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

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

this one must be rtm+ we can't ship with reload/back whacking framesets. bug 46828 might be a dup.

Radha, do you have a fix in mind for this? Should it be re-assigned?

I find that if I visit http://www.securityfocus.com/, and then click on the "Vulnerabilities" link, a click on one of the links returned in the inside frame fails to load anything. This is with a local build from CVS (Build ID: 2000092905).

This must be RTM+'d. Is more info really still needed? BTW, www.nerve.com shows this problem at times too (it has 5-9 frames, depending).

Working on a fix. Please note the [RTM NEED INFO] is a way of tracking for the Navigator triage team on whether this can be marked rtm+(once a patch is available). Please don't remove it from the status whiteboard.

Radha, is a fix still on the way here?

may be tomorrow

Sorry, with no fix in site, I have to minus this for now.

FWIW, I've attached a patch that does what I think might fix this bug, but I haven't tested it or anything yet. If this is still being considered for RTM, it could even be made a one liner: Index: nsSHEntry.cpp =================================================================== RCS file: /cvsroot/mozilla/xpfe/components/shistory/src/nsSHEntry.cpp,v retrieving revision 1.11 diff -u -r1.11 nsSHEntry.cpp --- nsSHEntry.cpp 2000/10/13 20:49:40 1.11 +++ nsSHEntry.cpp 2000/10/16 18:51:16 @@ -213,6 +213,7 @@ NS_ENSURE_SUCCESS(aChild->SetParent(this), NS_ERROR_FAILURE); PRInt32 childCount = mChildren.Count(); + while (aOffset > childCount++) mChildren.AppendElement(nsnull); if (aOffset < childCount) mChildren.InsertElementAt((void *) aChild, aOffset); else

Renominating (removing [rtm-]) now that there's a one-liner fix in hand.

Eric, Thanks for the patch. I will try out the patch later today. I'm still wpring on 46828.

FWIW: This patch did not seem to clear up the problem on Win2k for me.

I should add that it seems to make the first one or two reloads work, but continue reloading and everything gets horked the same as before. I am waiting for each reload to finish completely before reloading again.

*blush* I was still calling "Insert" so it would cause inputs like 0 3 2 1 to come out as 0 1 null 2 null 3. This latest patch should fix that. (2 liner)

Okay, I just tested this latest patch on FreeBSD and it works for me! :)

Jerry makes a good point - what if the reload button is pressed before the load completes? Radha, with my patch it's possible that some of the entries in the session history representation of the frame hierarchy could be null - would that cause problems? (worse than already exist) I noticed that sometimes if I reload before the page finishes, some frames won't load at all. However, this is also a problem before the patch.

Now that fixes it :-)

Moving this back up to "need info" status now that we have a patch. Radha, should we send this bug over to Eric?

Eric, I think you should own the bug. I have tried out the patch. It looks good. r=radha if you need it. Let's get a super review from rpotts or nisheeth and try to get it into PDT's query today before 4:30.

sr=alecf Also, whoever ends up checking this in, can you just add a comment explaining why you need to extend the array in this way? I talked to radha over AIM and she explained, but I think it would be useful to have in there.

Some explanation on the patch: Old code assumed that the order in which subframes come for addition in to SH will be the same as the order in which they actually get created. But that assumption is wrong. So, the new patch extends the array (by appending nsnull)to as many numbers required when the list is not long enough and then just replaces the nsnull with the SHEntry.

rtm+

Can I get one final r= and sr= to present this most recent patch to PDT? Thanks!

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

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

The latest patch looks good to me r=radha

I have just received sr= from Rick Potts on the latest patch. Sending back to PDT for evaluation. This is a very small change that fixes a session history corruption problem. It is low risk small, and contained. They payoff is that any complex nested frameset will not get mangled when clicking reload or going back/forward through history. There are real world test cases (this specific one has been reported several times independantly)

PDT: Real world test cases are: http://www.securityfocus.com (Reported twice independantly) http://gopconvention.com http://cadcamonline.com

rtm++

Fix checked in to branch and trunk. To verify: 1) Go to www.securityfocus.com (wait for the load to complete) 2) Click the reload button The page should loook the same as before the reload button was pressed. Thanks!

I can verify that this works on WinNT with build 2000101920. Thanks!

Marking verified with the Oct 19th branch build.

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

Mass removing self from CC list.

Now I feel sumb because I have to add back. Sorry for the spam.