Here's a bit of JS that I use to load bugs. It's in my bookmarks as the property of the bookmark. When the resulting url is loaded the back button isn't highlighted and it hasn't been added to the history as near as I can tell. You have to do this with a newly opened browser window. <DT><A HREF="javascript:id=document.getSelection();if(!id){void(id=promp t('Show Mozilla bug no.',''))}if(id)location.href='http://bugzilla.mozilla.org/s how_bug.cgi?id='+escape(id)+' '" ADD_DATE="943845841">Show Bug...</A>

session history --> radha. and this is either a dupe or a recent regression.

Does this JS code in a frameset page or create a frameset page? There's already a bug that says, if the first page loaded on a browser window is a frameset page, back button fails to work. This bug is assigned to mscott.

No, this happens with any javascript: URL, where ``result'' means ``URL load from the setting of document.location during the execution of a javascript: URL that doesn't return a non-void result''. The bookmarklet on the front page of bugzilla is a great example, and this bug is insanely annoying, for the record.

This bug is caused by the checkin for bug 39938 in js/dom/src/nsLocation.cpp. When the location.href is processed, docshell is still busy with the alert dialog, which makes the new url to replace the existing one, instead of a normal load.

If a javascript: bookmark (bookmarklet) or javascript: link loads a URL with [document.]location[.href], the new URL replaces the current page in session history. This happens even if the link/bookmarklet doesn't call alert(). Additional URLs with bookmarklets: http://www.google.com/options/netscape6.html (previously linked to on Google's front page) http://www.squarefree.com/bookmarklets/pagedata.html#validate

The attached patch takes care of bookmarklets, onLoadHandlers and simple script tags that have a location.href in them.

sr=alecf

Radha, who needs to review this?

I would like rpotts to take a look at this. I will talk to him today.

Hi Radha, I talked with jst about this one and he suggested that you add a IsExecutingEventHandler(PRBool *aResult) to nsIScriptContext. It turns out that event handlers are executed by calling CallEventHandler(...) while other scripts are executed by calling EvaluateString(...) So, inside on nsLocation you can check nsIScriptContext::IsExecutingEventHandler(...) to determine the replacement policy. This seems to limit the number of dependencies and keeps knowledge of script evaluation out of the DocShell (and nsDocument) :-) How does this sound? -- rick

That does sound better. As we discussed in the morning, I was trying to use nsDocument for maintaining the script execution status and it was more messier than the patch I have attached here. I will try jst's suggestion.

one nit - please use GetIsExecutingScript() rather than IsExecutingScript() so that we can use IDL attributes to describe this. (readonly attribute isExecutingScript;)

Since nsIScriptContext will never be a [scriptable] interface is there any advantage to using a 'readonly attribute isExecutingScript' rather than 'boolean IsExecutingScript' method? It seems like GetIsExecutingScript() just makes it harder to understand the code...

oh, I didn't realize it would never be scriptable...

I tried out the suggestion that jst made. But it doesn't work in all situations. The original test case posted to this bug by blizzard on 3/16 for the bookmarklet fails. In this case, the location.href is executed by EvaluateString(), even though clicking OK in the alert dialog triggers the location.href. Also, I have to make a special case checking for onLoad handlers, since we want to do LoadReplace if a onLoadHandler had a location.href in it.

The latest patch is a combination of first and the sugegstion jst made. Th idea is to isolate the <script> tag from the rest and set a flag in nsIScriptContext whenever we are in the middle of processing a script tag. This flag is used bynsLocation to distinguish a regular load from a replace load. So, all location.href will lead to a normal load except for the ones inside a script tag. We do not use nsDocument or nsDocShell to pass flags around.

Hi Radha, This looks good to me (r=rpotts) The only comment I have is to change my mind about what I said earlier :-) In this instance, I think that it would be better to have an attribute called 'processingScriptTag' rather than the IsProcessingScriptTag and SetProcessingScriptTag methods... So, for consistancy the methods should be called GetProcessingScriptTag and SetProcessingScriptTag I admit that I was wrong about the names - and now it's come back to bite me :-)

sr=blizzard

Whoa, what's up with the new argument to EvaluateScript() in the HTML content sink? Is the idea that external vs. inline scripts should be treated differently here, that seams really really wrong to me. If EvaluateScript() is called in the sink, you know if's because a <script> tag was seen in the HTML that's loaded, is that not all you're interested in? Also, the attached patch only fixes the problem for HTML, not for XHTML, you need similar changes in the XML content sink, having the two work differently doesn't seem like a good idea to me. Also, to add to the fun, Vidur is rewriting the script loading code and taking it out of the content sinks all together, so depending one who's checking in first, changes will be needed in both these diffs, and in Vidur's diffs. Cc:ing Vidur, please don't check this in yet until we really know what we wanto do here.

Hmm, I see this was checked in already, please don't close this before dealing with the differences between inline vs. external scripts, which I guess is something Vidur will haveto fix now. Oh well...

jst, the addition argument to EvaluateScript() got added because, in the original patch, it was getting called with a TRUE flag by ProcessScriptTag() and with a FALSE flag by OnStreamComplete(). Just minutes before I checked in, I saw your reply to the email from rpotts that it needs to be TRUE for both the cases. So, now, like you said, since both the callers pass a TRUE flag, the additional argument to EvaluateScript() is not needed. I can take that out and make similar changes to nsXMLContentSink. Vidur, What is the status of your rewrite? Will you be checking in RSN? Thanks,

vidur's changes has the patch attached to this bug. Markign fixed.

mass-verifying claudius' Fixed bugs which haven't changed since 2001.12.31. if you think this particular bug is not fixed, please make sure of the following before reopening: a. retest with a *recent* trunk build. b. query bugzilla to see if there's an existing, open bug (new, reopened, assigned) that covers your issue. c. if this does need to be reopened, make sure there are specific steps to reproduce (unless already provided and up-to-date). thanks! [set your search string in mail to "AmbassadorKoshNaranek" to filter out these messages.]

This checkin caused bug 178729 -- broken history when the script is running in one window and setting location in a _different_ window.