Closed Bug 20283 Opened 25 years ago Closed 25 years ago

Infinite loop when anchor has javascript:history.go(0)

Categories

(Core :: DOM: Core & HTML, defect, P3)

x86
Windows NT
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: pcurtis, Assigned: radha)

References

()

Details

(Keywords: testcase, Whiteboard: [nsbeta2-][TESTCASE]ETA 7/20[NEED INFO])

Attachments

(3 files)

Steps to Reproduce: 1. Go to http://developer.netscape.com/docs/manuals/dirsdk/jsdk30/index.htm 2. In the left pane, click the arrow icon next to "Step 4" (or 1 or 2 or 3) 3. *crash* Actual Results: *crash* Expected Results: Javascript menu expands Build Date &Platform: 1999111520 Windows NT Additional Platforms: None, at this time. Additional Information: none.
Assignee: mccabe → leger
Component: Javascript Engine → Browser-General
Summary: Browser Crashes on Javascript Menu → Browser Crashes on collapsable menu
Thanks for finding this bug. Any page we crash on is a problem. I see no evidence yet that this is actually a problem with the JavaScript engine. Reassigning to browser-general and changing in the hopes that somebody from the net can look at it and produce a narrower testcase. pcurtis - if you can produce a smaller testcase that causes the problem, it'll be a great help towards actually finding a fix. (QA - please don't reassign it back to JavaScript without some proof that something's wrong about how we process JavaScript. If it's a problem with a DOM api exposed to JavaScript, file it against 'DOM level 0'. )
Build 1999111520 Linux 2.2.5/glibc 2.1.1 also crashes.
Severity: normal → critical
Assignee: leger → don
Component: Browser-General → XPApps
QA Contact: rginda → claudius
claudius, can you reproduce this?
Assignee: don → vidur
Component: XPApps → DOM Level 0
Re-assinging to the owner of "DOM Level 0" since this has nothing to so with XPApps. Any idea who should get this?
Attached file reduced test case. (deleted) —
This line of html is causing the problem: <A HREF="javascript:history.go(0)">Hello</A> I attached a test case. This bug shows up all over developer.netscape.com it's Danny Goodman's html menu code. I think this is related to javascript: urls not working properly, especially because this html works fine, which also calls history.go(0): <html> <body> <SCRIPT> history.go(0) </SCRIPT> <A HREF="http://www.yahoo.com/">Hello</A> </body> </html>
stack trace using build from this morning (winnt). First I assert (follows). k1 seems bogus (0x0003308c), and thus my guess is the v1 val passed into js_CompareStrings is bum. JS_STATIC_DLL_CALLBACK(intN) js_compare_atom_keys(const void *k1, const void *k2) { jsval v1, v2; v1 = (jsval)k1, v2 = (jsval)k2; if (JSVAL_IS_STRING(v1) && JSVAL_IS_STRING(v2)) return !js_CompareStrings(JSVAL_TO_STRING(v1), JSVAL_TO_STRING(v2)); if (JSVAL_IS_DOUBLE(v1) && JSVAL_IS_DOUBLE(v2)) { ASSERT STACK js_compare_atom_keys(const void * 0x0003308c, const void * 0x00dbcc14) line 132 + 6 bytes JS_HashTableRawLookup(JSHashTable * 0x014c6560, unsigned long 1567, const void * 0x0003308c) line 178 + 28 bytes js_AtomizeString(JSContext * 0x022b1ba0, JSString * 0x00033088, unsigned int 2) line 488 + 17 bytes js_AtomizeChars(JSContext * 0x022b1ba0, const unsigned short * 0x01fd47a0, unsigned int 2, unsigned int 0) line 560 + 19 bytes js_GetToken(JSContext * 0x022b1ba0, JSTokenStream * 0x01fd44e0) line 703 + 45 bytes MemberExpr(JSContext * 0x022b1ba0, JSTokenStream * 0x01fd44e0, JSTreeContext * 0x000334b4, int 1) line 2271 + 13 bytes UnaryExpr(JSContext * 0x022b1ba0, JSTokenStream * 0x01fd44e0, JSTreeContext * 0x000334b4) line 2182 + 19 bytes MulExpr(JSContext * 0x022b1ba0, JSTokenStream * 0x01fd44e0, JSTreeContext * 0x000334b4) line 2042 + 17 bytes AddExpr(JSContext * 0x022b1ba0, JSTokenStream * 0x01fd44e0, JSTreeContext * 0x000334b4) line 2024 + 17 bytes ShiftExpr(JSContext * 0x022b1ba0, JSTokenStream * 0x01fd44e0, JSTreeContext * 0x000334b4) line 2009 + 17 bytes RelExpr(JSContext * 0x022b1ba0, JSTokenStream * 0x01fd44e0, JSTreeContext * 0x000334b4) line 1977 + 17 bytes EqExpr(JSContext * 0x022b1ba0, JSTokenStream * 0x01fd44e0, JSTreeContext * 0x000334b4) line 1952 + 17 bytes BitAndExpr(JSContext * 0x022b1ba0, JSTokenStream * 0x01fd44e0, JSTreeContext * 0x000334b4) line 1940 + 17 bytes BitXorExpr(JSContext * 0x022b1ba0, JSTokenStream * 0x01fd44e0, JSTreeContext * 0x000334b4) line 1929 + 17 bytes BitOrExpr(JSContext * 0x022b1ba0, JSTokenStream * 0x01fd44e0, JSTreeContext * 0x000334b4) line 1918 + 17 bytes AndExpr(JSContext * 0x022b1ba0, JSTokenStream * 0x01fd44e0, JSTreeContext * 0x000334b4) line 1907 + 17 bytes OrExpr(JSContext * 0x022b1ba0, JSTokenStream * 0x01fd44e0, JSTreeContext * 0x000334b4) line 1896 + 17 bytes CondExpr(JSContext * 0x022b1ba0, JSTokenStream * 0x01fd44e0, JSTreeContext * 0x000334b4) line 1856 + 17 bytes AssignExpr(JSContext * 0x022b1ba0, JSTokenStream * 0x01fd44e0, JSTreeContext * 0x000334b4) line 1797 + 17 bytes Expr(JSContext * 0x022b1ba0, JSTokenStream * 0x01fd44e0, JSTreeContext * 0x000334b4) line 1734 + 17 bytes Statement(JSContext * 0x022b1ba0, JSTokenStream * 0x01fd44e0, JSTreeContext * 0x000334b4) line 1441 + 17 bytes js_Parse(JSContext * 0x022b1ba0, JSObject * 0x00ddd2b0, JSTokenStream * 0x01fd44e0, JSCodeGenerator * 0x0003347c) line 283 + 20 bytes CompileTokenStream(JSContext * 0x022b1ba0, JSObject * 0x00ddd2b0, JSTokenStream * 0x01fd44e0, void * 0x01fd44e0, int * 0x00000000) line 2263 + 21 bytes JS_CompileUCScriptForPrincipals(JSContext * 0x022b1ba0, JSObject * 0x00ddd2b0, JSPrincipals * 0x02b26b04, const unsigned short * 0x02c72a30, unsigned int 13, const char * 0x00000000, unsigned int 0) line 2342 + 23 bytes JS_EvaluateUCScriptForPrincipals(JSContext * 0x022b1ba0, JSObject * 0x00ddd2b0, JSPrincipals * 0x02b26b04, const unsigned short * 0x02c72a30, unsigned int 13, const char * 0x00000000, unsigned int 0, long * 0x000335f8) line 2702 + 33 bytes nsJSContext::EvaluateString(nsJSContext * const 0x022b1d30, const nsString & {...}, void * 0x00ddd2b0, nsIPrincipal * 0x02b26b00, const char * 0x00000000, unsigned int 0, const char * 0x00000000, nsString & {...}, int * 0x00033958) line 228 + 53 bytes nsEvaluateStringProxy::EvaluateString(nsEvaluateStringProxy * const 0x02c71050, nsIScriptContext * 0x022b1d30, const char * 0x02c72c40, void * 0x00000000, nsIPrincipal * 0x02b26b00, const char * 0x00000000, int 0, char * * 0x00033954, int * 0x00033958) line 99 + 54 bytes XPTC_InvokeByIndex(nsISupports * 0x02c71050, unsigned int 3, unsigned int 8, nsXPTCVariant * 0x02c72b40) line 139 EventHandler(PLEvent * 0x02c72bf0) line 424 + 41 bytes nsProxyObject::Post(unsigned int 3, nsXPTMethodInfo * 0x00f2cd80, nsXPTCMiniVariant * 0x0003377c, nsIInterfaceInfo * 0x02b6b970) line 361 + 9 bytes nsProxyEventObject::CallMethod(nsProxyEventObject * const 0x02c72dc0, unsigned short 3, const nsXPTMethodInfo * 0x00f2cd80, nsXPTCMiniVariant * 0x0003377c) line 391 + 55 bytes PrepareAndDispatch(nsXPTCStubBase * 0x02c72dc0, unsigned int 3, unsigned int * 0x00033830, unsigned int * 0x0003381c) line 100 + 31 bytes SharedStub() line 125 Then I crash after "Goto 2" prints out a zillion times in the console. The crash is in printf :-/. Here's the crash stack. _write_lk(int 1, const void * 0x0003125c, unsigned int 1) line 129 + 10 bytes _write(int 1, const void * 0x0003125c, unsigned int 1) line 79 + 17 bytes _flsbuf(int 71, _iobuf * 0x1025a848) line 183 + 17 bytes write_char(int 71, _iobuf * 0x1025a848, int * 0x00031304) line 1083 + 75 bytes _output(_iobuf * 0x1025a848, const char * 0x00376c65, char * 0x00031568) line 393 + 21 bytes printf(const char * 0x00376c64) line 60 + 18 bytes nsWebShell::GoTo(nsWebShell * const 0x022a6bb0, int 2) line 2243 + 15 bytes HistoryImpl::Go(HistoryImpl * const 0x02b27fe4, JSContext * 0x022b1ba0, long * 0x00db328c, unsigned int 1) line 189 + 28 bytes HistoryGo(JSContext * 0x022b1ba0, JSObject * 0x00dbcc78, unsigned int 1, long * 0x00db328c, long * 0x0003187c) line 342 + 24 bytes js_Invoke(JSContext * 0x022b1ba0, unsigned int 1, unsigned int 0) line 665 + 26 bytes js_Interpret(JSContext * 0x022b1ba0, long * 0x00032154) line 2226 + 15 bytes js_Execute(JSContext * 0x022b1ba0, JSObject * 0x00ddd2b0, JSScript * 0x02c73f70, JSFunction * 0x00000000, JSStackFrame * 0x00000000, unsigned int 0, long * 0x00032154) line 838 + 13 bytes JS_EvaluateUCScriptForPrincipals(JSContext * 0x022b1ba0, JSObject * 0x00ddd2b0, JSPrincipals * 0x02b26b04, const unsigned short * 0x02c72040, unsigned int 13, const char * 0x00000000, unsigned int 0, long * 0x00032154) line 2705 + 27 bytes nsJSContext::EvaluateString(nsJSContext * const 0x022b1d30, const nsString & {...}, void * 0x00ddd2b0, nsIPrincipal * 0x02b26b00, const char * 0x00000000, unsigned int 0, const char * 0x00000000, nsString & {...}, int * 0x000324b4) line 228 + 53 bytes nsEvaluateStringProxy::EvaluateString(nsEvaluateStringProxy * const 0x02c72680, nsIScriptContext * 0x022b1d30, const char * 0x02c72250, void * 0x00000000, nsIPrincipal * 0x02b26b00, const char * 0x00000000, int 0, char * * 0x000324b0, int * 0x000324b4) line 99 + 54 bytes XPTC_InvokeByIndex(nsISupports * 0x02c72680, unsigned int 3, unsigned int 8, nsXPTCVariant * 0x02c72150) line 139 EventHandler(PLEvent * 0x02c72200) line 424 + 41 bytes nsProxyObject::Post(unsigned int 3, nsXPTMethodInfo * 0x00f2cd80, nsXPTCMiniVariant * 0x000322d8, nsIInterfaceInfo * 0x02b6b970) line 361 + 9 bytes nsProxyEventObject::CallMethod(nsProxyEventObject * const 0x02c723d0, unsigned short 3, const nsXPTMethodInfo * 0x00f2cd80, nsXPTCMiniVariant * 0x000322d8) line 391 + 55 bytes PrepareAndDispatch(nsXPTCStubBase * 0x02c723d0, unsigned int 3, unsigned int * 0x0003238c, unsigned int * 0x00032378) line 100 + 31 bytes SharedStub() line 125
I have no clue what's going on here. Not sure if I'm helping... But my guess is that JS reflection/stub routines are breaking????
Summary: Browser Crashes on collapsable menu → [WIP+rods]Browser Crashes on collapsable menu
Assignee: vidur → mscott
Summary: [WIP+rods]Browser Crashes on collapsable menu → Infinite loop when anchor has javascript:history.go(0)
mscott, Buster thought you already had this, I can't find your bug. So maybe its a dup or not. But it goes into an infinite loop becuase of a failure in nsWebShell::DoLoadURL
Attached file here a small test case (deleted) —
Whiteboard: [TESTCASE] javascript:history.go() broken.
Bulk moving [testcase] code to new testcase keyword. Sorry for the spam!
Keywords: testcase
Target Milestone: M14
We can see crash only if we use history.go(0) in Anchor. If we use it onClick of some button then it does not crash. Same crash is happening on bug# 22393 too. May be they are related. And bug is in Anchor.
Here's the problem: we run a javascript url for goto anchor(0). when the url is run, the session history adds the javascript url to it's history list. Then the javascript url is executed which says go to anchor 0. Session history is telling us the url for anchor 0 is the javascript url for going to anchor 0. hence the infinite loop of loads. I wonder if the session history shouldn't be adding the current url to the session history list until after that url is loaded. Actually I would think it has to do that. Maybe the problem is that the dom JS code proxies an event for actually running the javascript url. By the time this proxied event gets executed, the original javascript url has marked itself as done so it gets added to the sesion history. Then the proxied event gets handled and we read the same url out of the session history!! Now that I think about it, I'm sure that's what's actually going wrong. Just a matter of figuring out how to fix this the right way with session history.
Status: NEW → ASSIGNED
I'm not sure what you decided here but please note that indeed Session History MUST add urls before they are necessarily finished loading. Otherwise, if you follow a link on a page b4 it's done and then click back you get bad things, including a dozen or so more bugs that were fixed because SH adds the url before it's done loading.
Apparently session history adds the url to the session history BEFORE we run the url. So running a JS url like history.go(0) will always return the same url back out of the session history causing an infinte loop. One potential fix is to make sure js urls don't go into session history. I don't know if we would ever want JS urls going into session history? Another alternative is to move session history so that urls are only added AFTER they have been loaded instead of before.
claudius, thanks for mentioning the problems with moving session history after the url is loaded. I won't pursue this avenue then. =)
no prob. In general, I believe session history is somewhat over-inclusive at this juncture. There are open bugs that say _______url's should not be added to session history. I think law@netscape.com is riding those bugs.
I should clarify that it isn't session history per say that's causing the problem. It's something called history in webshell. I'm not sure how mHistory is different from session history. But before we run a url every URI gets added to mHistory in PrepareToLoadURI. so javascript:history.go(0) gets added to mHistory then when that script executes it extracts itself back out and re-runs itself.
Adding radha to the cc list in case she knows. Radha, do you know what mHistory is used for in webshell? How does it differ from the session history? mHistory is adding the javascript:history.go(0) to itself before we run the url. so when the script gets executed it gets the javascript url out of mHistory and runs it again. hence the infinite loop. I can't figure out what mHistory is and how it differs from the session history which is another variable in webshell.
I think mHistory is *global* history.
Thanks for the info Bill. Properly fixing this bug means changing when urls get added to *global* history. They should be added after the document has successfully loaded the url instead of before we load the url. This also makes the most sense from a design point of view because we don't want to be adding urls that failed to load or were aborted the global history. we should only add urls that successfully were loaded. I think session history should work the same way and it did in mozilla classic. claudius mentioned that we added the url before it was loaded to session history in order to fix some problems. I wonder if we shouldn't try to fix those problems individually instead of moving when the url gets added to the session history. but that should be another bug.
There may be some duplication of effort here in the webshell. I currently see a mHistory which Bill suggested is probably the global history. But there is also another variable supposedly tracking the global history called mHistoryService which is an implementation of nsIGlobalHistory. Ughhh....it almost looks like webshell is implementing 2 separate global histories + session history.
*** Bug 28671 has been marked as a duplicate of this bug. ***
Sorry about the late response. Catching up from sabbatical... mHistory in Webshell is *NOT* global History. This is the old Session History code that was used before I wrote nsISession History. It is used by viewer's back/forward buttons and by javascript for frame history support. It has not been removed from webshell, because removing it meant hooking viewer and javascript to nsISH. There was not a lot of interest then in getting rid of this old code in the webshell world, because we didn't want to hook viewer with nsISH for simplicity reasons and we didn't want to break the frame history that javascript maintains. But NOW with redesigned webshell, Brendan has agreed to hook up javascript with nsISH and this code will be changing soon.... In short, mHistory is used only by viewer and all javascript history calls. apprunner doesn't use mHistory.
Which Brendan has agreed to do what? I'm on sabbatical (your bug update generated mail that got through my filters by using "JS" and "brendan"), and not too likely to fix this soon, or even when I return. /be
Brendan, If you recall, you, I and travis had a meeting few months ago and this issue of hooking up Javascript history with SH came up. It was decided that we will hook up Javascript history with the window's SH though this may break javascript's frame history. I didn't mean to say that *You* were going to hook up. I meant that you agreed for the hookup of javascript history with SH. Radha
Moving my remaining M14 bugs to M15 which is the next targeted milestone.
Target Milestone: M14 → M15
not a m15 stopper.
Target Milestone: M15 → M16
Fixing JS to use the new session history instead of mHistory may fix this bug because urls aren't added to the new session history until after we fire the on load handler. So we won't have this problem where loading the 0th entry in the on load handler will infinitely cause us to reload the original url. Radha, do you know who owns the work for moving JS urls to use the session history instead of mHistory?
I have seen JS's history object to be wired to call docshell which uses the new session hsitory. But I ran in to some problems because JS urls are parsed as nsSimpleURLs. Look at http://bugzilla.mozilla.org/show_bug.cgi?id=40160. I'll be looking in to this later this week. I'll keep you posted.
Some good news: 1) I made some changes to docshell so we don't actually add the session history entry to session history until the on load handler is fired in the OnEndDocumentLoad notification. This fixes this problem. 2) I also have a fix for the assertion problem covered in Bug #40160. The Bad news: Right now JS urls which change the window location are badly broken. This is documented in Bug #37463. We enter a dead lock situation between the file transport which is blocked waiting for the JS to execute to change the location and the ui thread which is trying to destroy the file transport channel because we are going to load a new location. In short, I have a fix for the infinite loop in this case where the on load handler tries to load the first entry in session history. But then we run into Bug #37463. Marking the dependency.
Depends on: 37463
Keywords: nsbeta2
I haven't tried your changes yet. But few things that comes to my mind are, what happens if the page is partially loaded? If the user stopped a load or clicked on a link before the page is completely loaded, then it gets in to SH. If the url could not be resolved, then it doesn't. Is this taken care of? I'm not sure if it is a good idea to add the SHentry from webshell. There is more work to be done in docshell w.r.t. to SH that myself and the embedding team are working on right now. I think docshell w'd like to have control of interactions with SH. Docshell is also specific to each frame, so in a page with frameset, I think this will be a ugly interaction between webshell and docshell.
To answer your questions: 1) The OnEndDocumentLoad call is called for loads that get aborted and interrupted as well. So the url is still correctly added. >There is more work to be >done in docshell w.r.t. to SH that myself and the embedding team are working >on right now. I think docshell w'd like to have control of interactions with >SH. There is a one to one mapping between webshell and docshell right now. webshell currently inherits from docshell. Everything you can do in docshell you can do in webshell. Eventually the OnEndDocumentLoad handler will be moved to docshell but this hasn't been done yet. In other words, I don't believe this changes docshell's use of session history at all. I'm using the same variable mSessionHistory and everything. It just happens to be that this method is in the derived nsWebShell class. >Docshell is also specific to each frame, so in a page with frameset, I think >this will be a ugly interaction between webshell and docshell. I don't think in this case there is any ugly interaction. Docshell's implemtation of OnEndDocumentLoad is currently implemented by a sub class: nsWebshell. The implementation will eventually be moved up to Docshell but that hasn't happened yet. I don't think "interaction" between webshell and docshell is an issue here. I hope this makes sense and alays your fears about the suggested change.
Putting on [nsbeta2+] radar for beta2 fix.
QA Contact: claudius → desale
Whiteboard: [TESTCASE] javascript:history.go() broken. → [nsbeta2+][TESTCASE] javascript:history.go() broken.
M16 has been out for a while now, these bugs target milestones need to be updated.
My proposed fix above is still the only idea I have for a fix (that is not actually adding the page to the session history until after we have fired an on load event for the document in the OnEndDocumentLoad). Radha, I know you are doing lots of new session history bug work for nsbeta2. Should I re-assign this bug to you or are you okay with the above fix? I'm currently looking to load balance my nsbeta2 bug list which is currently at 15 so if you have some cycles and want a different fix than the one above I'd appreciate any help.
taking this from mscott.
Assignee: mscott → radha
Status: ASSIGNED → NEW
I would not hold nsbeta2 for this bug. I think we can take this off the beta2+ list. Looks like radha has a lot of other more serious bugs to look at anyway. What do you think Don?
I have fixes for most of my other bugs. This bug does need some research. I will hold on to this until tuesday or wednesday and then move out if it looks impossible. Thanks for helping out, mscott :)
Status: NEW → ASSIGNED
Whiteboard: [nsbeta2+][TESTCASE] javascript:history.go() broken. → [nsbeta2+][TESTCASE] javascript:history.go() broken. ETA 7/20
note: there's no more infinite loop. So this isn't going to cause a freeze or crash anymore.
can you change the summary to whatever symptoms you are seeing right now?
If there's no more infinite loop or crash, then I'm making this "nsbeta2-".
Whiteboard: [nsbeta2+][TESTCASE] javascript:history.go() broken. ETA 7/20 → [nsbeta2-][TESTCASE] javascript:history.go() broken. ETA 7/20
Depends on: 18321
nav triage team: [NEED INFO] QA please test original bug and close if you can. If you need to create a new bug for how this bug has evolved, please do.
No longer depends on: 18321
Whiteboard: [nsbeta2-][TESTCASE] javascript:history.go() broken. ETA 7/20 → [nsbeta2-][TESTCASE]ETA 7/20[NEED INFO]
Depends on: 18321
M16 is long gone
Target Milestone: M16 → M18
*** Bug 33311 has been marked as a duplicate of this bug. ***
Depends on: 30942
This got fixed with my checkins yesterday. THe mnu does come up. Fixed in branch too.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Verified with 2000-08-11-05.
Status: RESOLVED → VERIFIED
No longer depends on: 30942
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: