From Bugzilla Helper: User-Agent: Mozilla/5.0 (X11; U; Linux i686; ; rv:0.9.5) Gecko/20011012 BuildID: 2001101202 The navigation back and forward is a bit broken, when the JS command location.replace() is used on a page to initiate a load of a new page in an other frame. Reproducible: Always Steps to Reproduce: 1. Visit any site you like, or simply enter 'about:'2. Now enter http://www.oeamtc.at/ 3. Choose any story you like from the main frame 4. Go back -> this brings you 2 pages back5. Go forward -> brings you to the homepage as expected -> Now the forward button is disabled, to go forward to the article Actual Results: I just explained this in the "Steps to Reproduce": * Back brings you 2 pages back * Forward is disabled, even though it should be possible to click on it Expected Results: I think that's clear ;-) * Back should bring you one page back * Forward should bring you one page forward The problem seems to be the JS part which is responsible to load the ad banner in the other frame when the page gets loaded: <!-- Start of ad loader script --> <script LANGUAGE=JavaScript> <!-- hide from older browsers var fn = 'werbung'; var pn = '/netautor/pages/ads/ad.php?data=oeamtc/HP/homepage/index.ivw'; if (navigator.appName == 'Microsoft Internet Explorer' && parseInt (navigator.appVersion) < 4) { window.setTimeout ('window.open(fn,pn)', 1); } else if (window.parent.frames[fn]) { window.parent.frames[fn].location.replace(pn); } // --> </script> <!-- End of ad loader script --> The real problem is the location.replace(pn) command, which seems to confuse mozilla a bit. It should simply replace the currently displayed page in the frame 'werbung' with the one which is provided for this page in the main frame.

Over to session history.

is this what bug 103978 is about?

I have similar problem on : www.bytbil.se In the first screen, after login, select any city to login,you should set some search criterias. Now if you click one of the items found, i.e. look at a car for sale and want to go back, you cant. THERE is a special button on the page named "Tillbaka". It works sometimes but not always.

I have similar problem on : www.bytbil.se In the first screen, after login, select any city to login,you should set some search criterias. Now if you click one of the items found, i.e. look at a car for sale and want to go back, you cant. THERE is a special button on the page named "Tillbaka". It works sometimes but not always.

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

(Please change the OS to ALL...I don't know if I'm allowed to.) Please fix this. It is annoying and unexpected behavior, that makes a user unproductive. Test on MY.YAHOO.COM: Start out NOT logged in to a Yahoo account. 1. Go to my.yahoo.com and log in 2. Click to a news story 3. Use Back button REsult: No longer logged in Expected: Remain logged in (as worked in earlier browser versions) NOTE: If I log into Yahoo mail, then go to my.yahoo I *do not* get logged out when going to a news story then using Back button to get back to my.yahoo. Occurs in Gecko/20011128 as well as 2002011103 ______________________________ WWW.AMAZON.COM - Yesterday I experienced the same problem on a store that affiliates.amazon.com has. Unfortunately I can't replicate it today but here is what happened: 1. login at affiliates.amazon.com 2. add items to query in a text field, that will be used to generate affiliate ID codes. [i attached source for this page] 3. Submit, and the results are generated 4. Use Back button to edit/add items to the query field. Result: Takes you go to login page. The page that had the data entry field is gone. Expected: To be able to go Back and Forward Did not occur in IE.

Additional info re. replicating Yahoo problem: -I do not have "Remember my ID & Password" checked -I have my.yahoo listed as a "Passwords never saved" in Password Manager -If I log in, then use the Back button I go to the website that I was on *before* my.yahoo. So to replicate you don't actually need to go to a page within Yahoo.

added nsbeta1

Changed platform to ALL. I'm using Win2K.

I've been able to reproduce this, and do have "Remember my ID & Password" checked (but i had signed out, prior to signing back in again). I am adding regression, because this has worked for ME in previous releases. Of note, the lock icon is RED and broken when i log into yahoo, and unlocked when i go to news story.

SORRY - THE BUG 114897 WAS NOT A DUPE, AND I REMOVED KEYWORDS FROM THIS BUG AND PASTED ALL RELEVANT INFO INTO THE OTHER ONE :-( Sorry for the mess.

The problem here is 2 page loads happen when a news story in the right frame is clicked. The news is loaded in the right frame, and then an ad is replaced in another subframe(using location.replace). When going back, session history recognises the latest load which is the change in the ad, and loads it. But not the previous load, which is the change in the news story. This is the same problem as informer2.comdirect.de described in bug 88984.

This patch takes care of this bug and the site informer2.comdirect.de referred in bug 88684

Comment on attachment 70401 [details] [diff] [review] Patch to nsSHistory.cpp this looks fine, but the indenting is all messed up in CompareFrames (just look at the patch itself, you'll see what I mean) I find it a little disturing that CompareFrames is causing a page load - maybe it should be called LoadEntryInMatchingFrame() or something? sr=alecf with the indenting cleaned up - your call on the function name.

Comment on attachment 70781 [details] [diff] [review] patch with indents cleaned up per alec's suggestion. r=adamlock

Before we approve this, can we hear what it does to Tp numbers?

The patch adds additional cycles to the for loop, but only numeric comparison is eventually done. No string comparisons or allocations are done. This patch also partially fixes my other 0.9.9 bugs 88684 and 124427.

OK. I used the nsITimelineService to time the changes I specifically made to the function. As mentioned earlier, this change only affects back/forward/Go with in a frameset. For example, if you a) visit warp.mcom.com b) Click on QA in the left frame c) click back. The code comes in to effect only in c). The code does not become effective, if back/forward leads to a change in the root docshell or while loading a frameset page fresh. To specifically time the effect of additional cycles through the for loop, I created a test case which has 3 framesets and 2 out of 3 framesets have 2 additional children. I loaded a new page inone of the earlier frames. Previously, (without my patch) the function would be called recursively 3 times. The total time taken by the recursive function + a call to nsDocShell::LoadURI() took 10 ms. (average of 3 runs). With my patch the function will be called recursively for 6 times, The total time taken by the recursive function + a call to nsDocShell::LoadURI took only about 6.66 ms. (average of 3 runs). Ironically, I find that the latter run(with my patch) is faster than the current code. I discussed this with dp. Me and him tend to believe that the following could be the reason. But it is hard for either of us to explain. a) The current function has 2 out parameters that are interfaces. nsSHistory::CompareSHEntry(nsISHEntry * aPrevEntry, nsISHEntry * aNextEntry, nsIDocShell * aParent, nsIDocShell ** aDSResult, nsISHEntry ** aSHEResult) The 2 out parameters aDSResult and aSHEResult are addrefed with in the function, before returning. This has been avoided in the new implementation. The new function looks like this, nsSHistory::CompareFrames(nsISHEntry * aPrevEntry, nsISHEntry * aNextEntry, nsIDocShell * aParent, long aLoadType, PRBool * aIsFrameFound) The operation that was done outside the function (a call to nsDocShell::LoadURI()) using the return values in the former case is now being done inside the function. I believe that the addreff could be contributing to the additional time in the former case. But when I tried to specifically time the addref part, it turned out to be less than 10 ms, so I didn't get a exact number from nsITimelineService. (nsITimelineService provides a value of 0.00 if the time taken by the call is < 10 ms). I hope this data will help the drivers make a decision in approving the patch, Thanks, Radha

That's great analysis, and I'm heartened that you put it in, but the description of the change (fully recur in frameset hierarchies) makes it pretty clear that it'll have different effects on deeper framesets. I don't understand why you're not just running the pageload tests and reporting back. You're inside the firewall, so the instructions at http://www.mozilla.org/performance/tinderbox-tests.html should work fine for you. It's quick, it's easy, and it's what drivers asked for. Is there some reason that you can't run them, or do you just not want to?

I thought, specific performance evaluation of my code will give better data about my patch than running the tp tests. I don't believe running the tp tests will help anyone in deciding the performace effects of my code, since my code will not get invoked by the tests at all. Deeper larger framesets may result in additional iterations through the loop. But there will be a difference between current code and my patch *only if* the url change happened in one of the frames that are on the top of the tree. If the url change happend in the last child of the last frame, there will be no difference between current code and my patch. It looks like there is no way I can get a a= unless I run the tp tests. I will post that data when I'm in the campus tomorrow.

Pageload tests are at http://cowtools.mcom.com/page-loader/report.pl?id=3C7B2EFA35

Pageload tests for optimised linux build is at http://cowtools.mcom.com/page-loader/report.pl?id=3C7B3A5BFF. The previous chart reported in comment #22 was for an optimised windows build.

Comment on attachment 70781 [details] [diff] [review] patch with indents cleaned up per alec's suggestion. a=shaver for 0.9.9. BTW: providing only _one_ set of page load tests doesn't really help us figure out what the performance _change_ from a patch is, does it?

Fix checked in.