47365 - SH totally confused and misbehaves in this page.

Status: VERIFIED FIXED

(Core :: DOM: Navigation, defect, P1)

Description

[Radha on family leave (not reading bugmail)]

I went to www.cinescope.com and all hell broke loose. These two sites are very
similar in structure - yet one works and the other
doesn't. I just followed the steps outlined above.(here they are again)

1. Surf to http://www.dmoz.org 2. Type http://www.cinescope.com 3. Click
Left-hand frame - 'Forums'
4. Click RHS 'Guest Access'
5. Click any from the list of forums.
6. Click 'Back'

I went at least two pages back(instead of the one). You don't even have to do
Step 5. I clicked back after Step 4 and went back
to dmoz.org

Comment 1

[Claudius Gayle]

sorry to do this at so late a juncture but mozilla.org's formating broke my html and it took me this long to get the min testcase 
working right. I tried to tell PDT before, anyway...

At the url above is the 'standard' min testcase for this specific feature. It fails horribly. frankly, i don't know why it works on _any_ 
sites if this doesn't work.

The test case contains steps to repro.
Does not work on all platforms with the 2000080204/5 m17 branch builds.

Comment 2

[shrirang khanzode]

I recreated this problem on all m17 builds today. Seems like session history 
for frames is broken. This works just fine if I visit a few pages and just go 
BACK and FORWARD and pages load in the proper order.

Comment 3

[leger]

Not sure where we left off with this one after PDT today.  It is ugly, but 
thought I heard pass for hold..jar? do you remember?

Comment 4

[Jim Roskind]

I think the call for beta2 at this point is that the risk is too great to try to
repair frame history (on short notice, and with little/no cook time possible).

I believe I'm playing back the concensus of the discussion on Friday... so I'm
going to go ahead and beta2-minus this bug.

Comment 5

[leger]

Putting on relnote2 radar.

Comment 6

[Paul Chen]

nav triage team:
too important to minus

Comment 7

[Radha on family leave (not reading bugmail)]

cc'ing mscott & rick,
The problem here is, docshell does not have proper loadtype. In the mozilla.org 
test case url, provided above,

a) Follow the steps to reproduce.
b) After step # 2, the loadtype in the subframe "main"(the bottom one) is 
loadHistory (due to the back/forward action)
c) When you click on the  link "Third" in step #3, nsDocShell::InternalLoad() is 
called in the top frame with loadtype "loadLink" which passes the control to the 
uriloader for target "main".
d) uriLoader then calls the nsDocShell::OnLoadingSite() on the target 
frame("main"). But it does not pass any loadtype to it. So, in 
nsDocShell::OnNewUri() for the targer subframe, we continue to use the loadtype 
set by previous loads which in this case happens to loadHistory. When loadType 
is loadHistory, it does not get added to SH. therefore the misbehavior. 

UriLoader should pass a loadtype in its callback, OnLoadingSite().

Comment 8

[leger]

PR2 is out...adding [nsbeta2-]  Already has [nsbeta3+]

Comment 9

[Dan Nunn]

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

Comment 10

[Scott MacGregor]

Radha, do you mind if I take this from you? If the only problem is what you
mentioned above with the load type not being propogated properly than I have a
fix for that.

Unfortunately, www.cinescope.com is in korean! So I can't perform Claudius's
tests on that page to verify if I really do have it fixed.

However, general testing on warp.mcom.com (which uses frames) with my changes
seemed to see session history working much better!! There was one caveat. I am
able to get into a case where I assert in nsDocShell::GetChildAt because index
wasn't in a valid range. So you get stuck on a certain page.

This happens most often when:
1) browsing through some frame pages on warp. Click on a link from the TOC frame
that leaves warp (so you lose the TOC frame). i.e. click on User Experience
which loads gooey.mcom.com. The document loads correctly, the frames are torn
down. Then hit the back button...Oops, we get this assertion and we fail to go
back to the warp.mcom.com pages.

That was the only problem I could get myself into when mixing session history
with frames on warp.mcom.com after the patch I'm about to attach.

Comment 11

[Scott MacGregor]

Attachment: proposed fix.

Comment 12

[Radha on family leave (not reading bugmail)]

scott, the changes look OK to me. i will look in to the assertion once we we 
have this in place.

Comment 13

[Scott MacGregor]

this is now fixed.

Comment 14

[Blake Ross]

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

Comment 15

[Claudius Gayle]

VERIFIED Fixed with 2000091312 builds
(using testcase in url field of this bug)