18321 - [webshell] Javascript frame history doesn't work well with SH

Status: VERIFIED FIXED

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

Description

[terry]

Mozilla M10 (1999100808) unable to handle frames on this site.  Can't pin it to
anything specific yet.  Navigator 4.7 and IE 4 able to handle this fine.

Comment 1

[karnaze (gone)]

Reassigning to Eric.

Comment 2

[Eric Pollmann]

Please include a description of the problem.  I visited this site and it looks
fine, except for a few extra scrollbars (bug 14827)  Clicking on links update
frames as it should.

Comment 3

[terry]

Sorry - was at work when this was posted and time was short - had a better
description typed but Mozilla crashed.  Backed off to the more general case when
I noticed that the site was flakey in general as I went back to recheck it.
Browser version:  Mozilla M10 linux version running on Redhat 6.1
The offending link:
http://www.cdmag.com/Home/home.html?article=/articles/024/040/armada_preview.html
Problem description:  Browse there, pick any of the screenshots in the frame
with the article - this should replace the contents of the frame with a larger
screenshot.  Use the browser's back button.  In my case the frame did not change
back to the article - the screenshot remained in the frame.  Hit back again, and
the frame *still* didn't change, but the frame at the top (where the banner ad
is) changed to the page that originally brought me here (www.bluesnews.com).
Typing the Url http://www.bluesnews.com into the location textbox at the top
successfully reloaded the page and replaced the stuck frame.  This behavior was
reproducible every time I tried (4 or 5 times).
If you really want to reproduce the path I took, start at bluesnews and find the
topic "Star Trek Previews" in the stories for Nov 9 and follow the link from
there.

Comment 4

[Eric Pollmann]

Radha, is this a frameset navigation problem with Session History?  Can you take
a look?  Thanks!

This is what he is seeing:

 [A]  _click link in frame B__\  [A]  _click "Back" button__\  [A]
 [B]                          /  [C]                        /  [C]

Should be:

 [A]  _click link in frame B__\  [A]  _click "Back" button__\  [A]
 [B]                          /  [C]                        /  [B]

Comment 5

[Radha on family leave (not reading bugmail)]

Post webshell landing. The SH design is going to change .

Comment 6

[rickg]

Seems like a must have for Beta.

Comment 7

[Radha on family leave (not reading bugmail)]

This  page seems to use Javascript to load pages with in a subframe. This is
broken and a known issue. Javascript frame history and Session History are not
tied up right now. However, once the new webshell lands, it will be and I'll be
address this then.

I tried several internal QA test sites for frames in general and it seems to
work OK.

Changing summary to reflect the javascript discrepancy. adding [webshell] to
summary.

Comment 8

[don]

Move to M13 ...

Comment 9

[Radha on family leave (not reading bugmail)]

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

Comment 10

[don]

Re-summarized.

Comment 11

[Bill Law]

Moving all to M14 (for webshell changes that are being deferred till M14 rather
than destabilize M13).

Comment 12

[Bill Law]

Marking this as beta1.

Comment 13

[leger]

Putting on PDT+ radar for beta1.

Comment 14

[Michael La Guardia]

Removing PDT+.  Radha's not here and there's no indication of when this will be
fixed.  It's obviously a bad bug, but it's not going to affect a large
proportion of our users often.  You have to work to reproduce this (go to page

Comment 15

[leger]

Putting on PDT- radar for beta1. 

Comment 16

[Radha on family leave (not reading bugmail)]

Moving to M15...

Comment 17

[Radha on family leave (not reading bugmail)]

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

Comment 18

[don]

Move to M16 for now ...

Comment 19

[Chris Petersen]

Added keyword nsbeta2.

Comment 20

[leger]

Need info...is this still occuring with latest build?

Comment 21

[Chris Petersen]

With the May 12 Mac build (2000051208), I can still reproduce this problem as 
described. Since I have no forward/back arrows on my win 98 build I don't have a 
way to test this yet.

Steps to reproduce:

1) Got the url specified:http://www.cdmag.com/Home/home.html?article=/articles/
024/040/armada_preview.html

2) In the center frame, click on one of the thumbnail images to open larger 
version.

3) After image is displayed in this frame, click the back arrow on toolbar.

4) Instead of displaying the previous page in the frame, it goes to the  last 
site visited.

Comment 22

[Gervase Markham [:gerv]]

Looks like it. Going to:
http://www.cdmag.com/Home/home.html?article=/articles/024/040/armada_preview.htm
l
clicking a link, then pressing Back has no effect.

Gerv

Comment 23

[leger]

Thanks guys.  Putting on [nsbeta2+] radar for beta2 fix. 

Comment 24

[don]

Move to M18.

Comment 25

[Costi Karastamatis]

makingtest [e-mail]

Comment 26

[don]

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

Comment 27

[Radha on family leave (not reading bugmail)]

This involves more changes to nsIWebNavigation interface. cc'ing respective 
people.

Comment 28

[Radha on family leave (not reading bugmail)]

Rick has all the info to address this. I think  he can handle this.

Comment 29

[rpotts (gone)]

-> nisheeth

I'm leaving on vacation and nisheeth has kindly agreed to look at my session 
history related bugs :-)

-- rick

Comment 30

[Nisheeth Ranjan]

Adding radha to the cc list.  javascript: urls currently get added to the 
session history.  This should not be happening, right?  CCing radha for an 
answer...

I need to leave right now for a W3C meeting.  I'll be back on Wednesday night.  
Radha, I'll give you a brief description of what I've uncovered so far so if you 
have some free cycles till then, you can take a look.

First, let me attach the reduced test case...

Comment 31

[Nisheeth Ranjan]

Attachment: fs.html

Comment 32

[Nisheeth Ranjan]

Attachment: left.html

Comment 33

[Nisheeth Ranjan]

Attachment: right.html

Comment 34

[Nisheeth Ranjan]

Attachment: right2.html

Comment 35

[Nisheeth Ranjan]

If you save fs.html, left.html, right.html, and right2.html in the same 
directory and load fs.html in the browser you'll see a page with two frames.  
Initially, the left frame contains left.html and the right frame contains 
right.html.  The left frame has a javascript: link that loads right2.html into 
the right frame.

If you click on the link, right2.html loads in the frame and the text 
right2.html appears in the left frame.  The bug is that the back button does not  
let you go back - it stays disabled.

I set a breakpoint in nsDocShell::OnNewURI() and recorded the aURI parameter 
when I clicked on the link in the left frame.  First, right2.html showed up on 
the right frame's docshell.  Then, the javascript: url showed up in the left 
frame's docshell.  I manually set the updateHistory flag to false in the 
debugger so that the JS url would not get added to the SH and continued loading 
right2.html into the right frame.  Once the load completed, the back button did 
not get enabled as it should have, but clicking on the back button did the right 
thing - right.html got loaded up in the right frame.  So, it seems that ensuring 
that JS urls don't get added to the SH gets us part way to the solution.  I 
haven't figured out, yet, why the back button does not get enabled.

So, that was the flow for the case where I manually set the updateHistory flag 
in OnNewURI().

When I don't interfere with the page load after clicking on the link in the left 
frame, the back button stays disabled as before and does not initiate page 
loads when I click on it.

Hope that explanation helps in tracking this down further while I'm away...

Comment 36

[Kathleen :Brade]

looks like nisheeth intended to reassign this bug to radha but didn't; reassign
Radha--if you don't think you are the right person to fix this, reassign it back 
to nisheeth or a more appropriate person

Comment 37

[Radha on family leave (not reading bugmail)]

Nisheeth,

There is more to this bug than what you have discovered. Basically, we need to 
re-wire the JS history dom/src/base/nsHistory.cpp, nsLocation.cpp to the new 
session History. Some explanation for the behaviors you saw:

1) When you do a location.href = "some url" it gets passed in to docshell as 
loadtype "loadNormalReplace". loadNormalReplace urls do not get added to SH, 
instead they just replace the url in the previous entry with the current url. 
That's why the back did not get enabled when you loaded the right frame. 

2) We also should make sure that JS urls do not get added to SH, this will 
prevent JS alert dialogs, redirects, reloads etc.. from reappearing when you 
click back.

3) history.go() is not wired to the new SH. This may involve some changes in 
nsIWebNavigation.

4) JS history.back() and history.forward() are hooked to session history. But it 
its well behavior depends on how we fix history.go(). 

5) Finally the controversy of to kill OR not to kill JS frame history/dom 
history. I will send you some email discussions I had with Ian/vidur regarding 
this. The changes we make to fix this bug may break some benign dom history 
features that people have come to expect in a browser. 

I'm giving this bug back to you, since I think you understand the dom/JS 
ramifications on this, better than I do.

Comment 38

[Chris McAfee]

updating status

Comment 39

[leger]

Re-assigning to radha.  nisheeth is out....can you contact jst and vidur for 
some help on this?  Thanks!

Comment 40

[racham]

Adding Alec and myself to the cc list.

Comment 41

[Radha on family leave (not reading bugmail)]

More comments:

when I load your test page, fs.html and click in the left frame, I see that 
right2.html is loaded in the right frame as well in the left frame. 

1) I think loading right2.html in hte left frame is wrong

2) While loading right2.html in the right frame, if I change the loadtype to 
loadNormal instead of loadNormalReplace in the debugger, then it gets added to 
session history.

3) Then the actual "javascript:top.right....." comes for loading in the left 
frame. First of all, I think this load should not happen, since the javascript 
has already been executed and the result of the execution has already been 
displayed in the right frame. Now that the load has come thro', I set 
updateHistory to false to prevent this from getting into SH. When I do this, 
right2.html gets loaded in teo left frame, but does not get in to SH. So, 
clicking back/forward from now on behaves right. 

So, I think we need to 

1) make some changes to dom/src/base/nsLocation.cpp to fix loadTypes
2) see how we can avoid javascript: urls coming in as urls and thereby executed 
twice. (Maybe there are some JS/dom/layout gotchas here that I don't know about)
3) See how we can deal with history.go()

Comment 42

[racham]

Please note that this bug also blocks bugscape bugs 1231 and 1637. Both of them 
are failing on history.back().

bug 1231 is marked as dup of this one. Bug 1637 (like a tracking bug on 
bugscape) is kept open so that QA can keep track of the all possible 
side-effects of this bug.

As we do not have a system that we can use to link bugzilla and bugscape bugs, 
please do update bugscape bug 1637 when this bug is fixed.

Comment 43

[Johnny Stenback (:jst)]

Radha, the testcase is a bit confusing but clicking on the link in the left
frame doesn't cause the file 'right2.html' to be loaded into the left frame, in
stead the content of the left frame is replaced with the value of the expression
in the javascript: URL, which happens to be the same as the content of the file
'right2.html'. To fix this you can insert "undefined;" (without the quotes) at
the end of the javascript: URL ('...="right2.html"; undefined;'.

Comment 44

[Radha on family leave (not reading bugmail)]

Nisheeth is back. He is looking into this.

Comment 45

[bijals (gone)]

This is an important dependency that Activation has on Seamonkey.  End users can 
not hit the back button if they made an error during registration.

How are things going on this bug for PR2?

Comment 46

[Nisheeth Ranjan]

Updating ETA...

Comment 47

[Nisheeth Ranjan]

OK, here's an update on this bug.

This bug is tracking four different problems (which is bad, but I'm not going 
to rant about it at this time :-) ).  I'll list out the status on each one of 
them below:

1) The back button was not getting enabled when the user clicked on a javascript 
url that caused a new page load in another frame.  I've checked in a fix for 
this into the branch.

2) JS urls should not get added to Session History.  I spoke to Radha about this 
and we don't want to make this change for beta 2.  This particular problem is 
not causing any beta 2 blockers and we want to keep changes to beta 2 at a 
minimum.  Filed bug 46890 to track this problem.  If we find that this bug 
causes beta 2 blockers, we can raise its severity.

3) Activation is blocked by the problem described in bugscape bug 1231.  I've 
attached a patch to that bug that should, hopefully, fix the problem.  Bhuvan 
Racham is testing the patch on a commercial branch build. Thanks, Bhuvan!

4) history.go() needs to get hooked up to the new session history.  Radha is 
working on re-implementing nsHistory::Go() using the new session history APIs.  
Thanks, Radha!

Comment 48

[Nisheeth Ranjan]

Attaching a patch that makes us behave exactly like Nav 4x and IE on 
http://www.cdmag.com/Home/home.html?article=/articles/024/040/armada_preview.htm
l.

AddToSessionHistory() was creating session history entries even for a load type 
of loadNormalReplaced.  The attached code prevents that from happening.  Radha, 
please code review it.  Thanks.

Comment 49

[Nisheeth Ranjan]

Attachment: Proposed fix for session history behavior on cdmag page

Comment 50

[Nisheeth Ranjan]

Radha has reviewed the attached fix (attachment 12163 [details] [diff] [review]) and I've checked it into 
the beta 2 branch.

Comment 51

[leger]

Ok to mark Fixed?  or do you need to check into Trunk too?

Comment 52

[leger]

Actually not in branch yet per nisheeth.  Will do a partial fix for history.go 
to be checked in TODAY!

Comment 53

[Radha on family leave (not reading bugmail)]

Changes checked in to the branch. 

Comment 54

[Blake Ross]

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

Comment 55

[Radha on family leave (not reading bugmail)]

All the 4 items nisheeth listed above are fixed. but we still have some problems
with cdmag.com. In general JS hook up to SH is DONE. So marking this fixed and
opening a new bug for cdmag.

Comment 56

[Jan Carpenter]

I can get Nisheeth's testcases (modified using the suggested "='right2.html'; 
undefined;") to run correctly on Win98 & Mac when I click the link only once.  
However if I click the link twice and then use the back button twice I do not go 
back to the original page in the right frame.  I'm going to consider this bug 
not fixed, but if this is instead a new bug let me know and I'll enter a new bug 
for it.

I'm having trouble with the Linux builds, but will update again when I get that 
working. 

Comment 57

[Radha on family leave (not reading bugmail)]

janc, nisheeth's test case is little confusing. you may want to try some other 
similar page where clicking in a link in left frame, will load something in 
right  frame. However, this bug was primarily for hooking up JS history calls to 
session History. THis work has been done. So, please open a new bug for any 
problem you see with JS history. Please look thro' my bug list (I have about 20 
Session History bugs) before  filing a new one. It is possible that you could 
find one already open for the probl;em. Thanks. Marking this as fixed.

Comment 58

[Jan Carpenter]

verified
2000-08-07-12-M17 : win98
2000-08-07-12-M17 : linux
2000-08-07-12-M17 : mac