Closed Bug 20283 Opened 25 years ago Closed 24 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.
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
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: 24 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: