Closed Bug 18321 Opened 25 years ago Closed 24 years ago

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

Categories

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

defect

Tracking

()

VERIFIED FIXED

People

(Reporter: terry, Assigned: nisheeth_mozilla)

References

()

Details

(Whiteboard: [nsbeta2+][PDT-] ETA 8/01)

Attachments

(5 files)

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.
Assignee: karnaze → pollmann
Reassigning to Eric.
Status: NEW → ASSIGNED
OS: Linux → All
Hardware: PC → All
Whiteboard: Can't reproduce problem
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.
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.
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]
Assignee: pollmann → radha
Status: ASSIGNED → NEW
Summary: Broken frames navigation → Broken frameset navigation
Whiteboard: Can't reproduce problem
Status: NEW → ASSIGNED
Target Milestone: M12
Post webshell landing. The SH design is going to change .
Summary: Broken frameset navigation → [dogfood]Broken frameset navigation
Whiteboard: [PDT-]
Seems like a must have for Beta.
Summary: [dogfood]Broken frameset navigation → [dogfood][webshell]Javascript frame history doesn't work well with SH
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.
Target Milestone: M12 → M13
Move to M13 ...
*** Bug 21373 has been marked as a duplicate of this bug. ***
Summary: [dogfood][webshell]Javascript frame history doesn't work well with SH → [webshell] Javascript frame history doesn't work well with SH
Whiteboard: [PDT-]
Re-summarized.
Target Milestone: M13 → M14
Moving all to M14 (for webshell changes that are being deferred till M14 rather than destabilize M13).
Marking this as beta1.
Keywords: beta1
Putting on PDT+ radar for beta1.
Whiteboard: [PDT+]
Depends on: 28434
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
Whiteboard: [PDT+]
Putting on PDT- radar for beta1.
Whiteboard: [PDT-]
Moving to M15...
Target Milestone: M14 → M15
*** Bug 21373 has been marked as a duplicate of this bug. ***
Move to M16 for now ...
Target Milestone: M15 → M16
Added keyword nsbeta2.
Keywords: nsbeta2
Need info...is this still occuring with latest build?
Whiteboard: [PDT-] → [NEED INFO][PDT-]
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.
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
Thanks guys. Putting on [nsbeta2+] radar for beta2 fix.
Whiteboard: [NEED INFO][PDT-] → [nsbeta2+][PDT-]
Move to M18.
Target Milestone: M16 → M18
*** Bug 21373 has been marked as a duplicate of this bug. ***
This involves more changes to nsIWebNavigation interface. cc'ing respective people.
Whiteboard: [nsbeta2+][PDT-] → [nsbeta2+][PDT-]ETA 7/20
Rick has all the info to address this. I think he can handle this.
Assignee: radha → rpotts
Status: ASSIGNED → NEW
-> nisheeth I'm leaving on vacation and nisheeth has kindly agreed to look at my session history related bugs :-) -- rick
Assignee: rpotts → nisheeth
Status: NEW → ASSIGNED
Whiteboard: [nsbeta2+][PDT-]ETA 7/20 → [nsbeta2+][PDT-]Under analysis
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...
Attached file fs.html (deleted) —
Attached file left.html (deleted) —
Attached file right.html (deleted) —
Attached file right2.html (deleted) —
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...
Whiteboard: [nsbeta2+][PDT-]Under analysis → [nsbeta2+][PDT-]Away on W3C mtg. Will return 7/26 night. Radha is the best person to work on this till then.
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
Assignee: nisheeth → radha
Status: ASSIGNED → NEW
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.
Assignee: radha → nisheeth
No longer blocks: 20283
Blocks: 20283
Blocks: 30030
updating status
Whiteboard: [nsbeta2+][PDT-]Away on W3C mtg. Will return 7/26 night. Radha is the best person to work on this till then. → [nsbeta2+][PDT-] Nisheeth away at W3C mtg. back 7/26 pm.
Re-assigning to radha. nisheeth is out....can you contact jst and vidur for some help on this? Thanks!
Assignee: nisheeth → radha
Adding Alec and myself to the cc list.
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()
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.
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;'.
Nisheeth is back. He is looking into this.
Assignee: radha → nisheeth
Whiteboard: [nsbeta2+][PDT-] Nisheeth away at W3C mtg. back 7/26 pm. → [nsbeta2+][PDT-] Nisheeth working on this
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?
Updating ETA...
Status: NEW → ASSIGNED
Whiteboard: [nsbeta2+][PDT-] Nisheeth working on this → [nsbeta2+][PDT-] ETA 7/31
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!
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.
Radha has reviewed the attached fix (attachment 12163 [details] [diff] [review]) and I've checked it into the beta 2 branch.
Ok to mark Fixed? or do you need to check into Trunk too?
Actually not in branch yet per nisheeth. Will do a partial fix for history.go to be checked in TODAY!
Whiteboard: [nsbeta2+][PDT-] ETA 7/31 → [nsbeta2+][PDT-] ETA 8/01
Changes checked in to the branch.
*** Bug 34937 has been marked as a duplicate of this bug. ***
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.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
Status: REOPENED → RESOLVED
Closed: 24 years ago24 years ago
Resolution: --- → FIXED
verified 2000-08-07-12-M17 : win98 2000-08-07-12-M17 : linux 2000-08-07-12-M17 : mac
Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard
Component: Layout: HTML Frames → Layout: Images
Product: Core Graveyard → Core
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: