http://www.mehndi-tempel.com/shop/index.html Going to this site, and clicking on any link (for example the top links with a green background), it does not create an session history entry, and going back goes to the page you were one prior to visiting said site. Filed originaly as a bugscape german topsite issue - http://bugscape.mcom.com/show_bug.cgi?id=18199

This happens in both 1.0.1 branch and 1.1beta btw.

when you click on the green link, most of the times the throbber never stops, indicating that the browser is expecting more data from the site. This prevents the back and forward buttons from updating. Sometimes however, the throbber stops, and in those cases sessio history entries are created and the back forward buttons are enabled. When this happens, the zuruck button works. I think the problem is with the site.

Dup of bug 160869?

open frameset.html on a new window. Click some links and you will notice that history doesn't change. It has something to do with javascript (specialy with location.replace)

on the sample above, when you view source of central frame, you see contenido1.html although you are on contenido2.html. You ever see the first page loaded.

Marking as nsbeta1+/topembed+ per EDT triage.

There are couple of things going on here. looking thro' the testcase, 1) Doing a location.replace right on the "src=" for the subframe, confuses docshell and it does not create a entry for the subframe in history. 2) The left frame links.html itself does not load because javascript fails to identify a relative url over a blank document. Changing location.replace('links.html') to a absolute url makes the left frame show the document. But due to the problem described in 1) back forward navigation stil won't work. Looking thro' how these 2 issues can be resolved....

This patch specifically looks for about: as the base uri. Other way to fix this bug is to simply look for success of NS_NewURI() in SetHrefWithBase() and redo it with an alternate base url if the first one fails.

Comment on attachment 113749 [details] [diff] [review] Initial patch This approach looks reasonable to me, but this diff is broken, looks like a chunk of the diff is missing, the beginning of the file says nsDocShell.cpp, but most of the changes are to LocationImpl. Please attach a non-broken diff.

Comment on attachment 113810 [details] [diff] [review] patch 1.1 +// Checks if the baseurl is an about: url. If so, we use the parent document's url +// as the base url. +nsresult +LocationImpl::CheckBaseURL(nsIURI * aBaseURI, nsIURI ** aAlternateBaseURI) How about renaming this to soemthing like FindUsableBaseURI()? You're not just checking something here, you're looking for a usable base URI if one isn't given... And rename aAlternateBaseURI to something like aUsableURI, and for consistency's sake, use URI all over, no URL's here. { + if (!aBaseURI || !mDocShell) + return NS_ERROR_FAILURE; + PRBool aboutProtocol = PR_FALSE; This file uses 2-space indentation, stick with that. + // If the current base url is not a "about:" url, we have nothing to do. + if (NS_SUCCEEDED(aBaseURI->SchemeIs("about", &aboutProtocol)) && !aboutProtocol) + return NS_OK; Set *aAlternateBaseURI to aBaseURI here. >+ if (!docShellAsTreeItem) >+ return NS_ERROR_FAILURE; >+ nsCOMPtr<nsIDocShellTreeItem> parentDS; No tabs! + docShellAsTreeItem->GetSameTypeRootTreeItem(getter_AddRefs(parentDS)); I don't think we want to get the root directly here, imagine a nested frameset case, the base should not necessarily be the root's URI, but in stead maybe the immediate parent's URI. Write a loop that walks up the parent chain to the same type parent and use the first base that's a usable base URI. + if (webNav) + rv = webNav->GetCurrentURI(aAlternateBaseURI); I say we want to check that aAlternateBaseURI here is actually something that can be used as the base URI, if not, don't bother passing it back to the caller... + nsCOMPtr<nsIURI> newUri, alternateBaseURI; + + // Make sure the base url is something that will be useful. + result = CheckBaseURL(aBase, getter_AddRefs(alternateBaseURI)); + if (!alternateBaseURI) + alternateBaseURI = aBase; How about naming alternateBaseURI simply baseURI here? I'll stamp my r/sr on this if you make those changes, but I want to see the new patch first...

Comment on attachment 114238 [details] [diff] [review] Patch 1.2 +// Walk up the docshell hierarchy and find a usable base URI. Basically +// anything other than a "about:" I just realized that about: isn't the only type of URI that won't work as a base, what about a javascript: URL, or a data: URL? Any others? - In LocationImpl::FindUsableBaseURI(): +{ + if (!aBaseURI || !aParent) + return NS_ERROR_FAILURE; + NS_ENSURE_ARG_POINTER(aUsableURI); ... Pull all of this method in two spaces, don't mix 4-space and 2-space indentation. + while(NS_SUCCEEDED(rv) && baseURI) { + PRBool aboutProtocol = PR_FALSE; + // If the current base uri is a valid one,(anything other than about:), return it. + if (NS_SUCCEEDED(baseURI->SchemeIs("about", &aboutProtocol)) && !aboutProtocol) { + *aUsableURI = baseURI; + NS_ADDREF(*aUsableURI); + return NS_OK; + } Make this check for javascrip: and data: URL's too. r=jst with that change.

jst, I thought about other urls that could not be usable base urls. But I wasn't sure I would have a complete list or I will be able to test them well. That's why I restricted to about: for now, which I know doesn't work. I believe that data: and javascript: can fall into this category, but I'm not going to spend more time on other protocols. I will take care of the spacing. (I'm so used to the 4-space indentation in docshell). Thanks.

Comment on attachment 114238 [details] [diff] [review] Patch 1.2 >+ // If the current base uri is a valid one,(anything other than about:), return it. >+ if (NS_SUCCEEDED(baseURI->SchemeIs("about", &aboutProtocol)) && !aboutProtocol) { >+ *aUsableURI = baseURI; >+ NS_ADDREF(*aUsableURI); >+ return NS_OK; >+ } yeah, would be good to include data: and javascript: here. hmm.. could you check nsIProtocolHandler::protocolFlags for the URI_NORELATIVE flag? that should tell you if the current baseURI can be used to make the href absolute, right? that would be much better than some sort of whitelist.

Darin, I have attached patch that uses the protocol flags to check on validity of a base uri instead of a white list. One question for you is, is there a default value for protocol flags that can be assigned to the local variable. + PRUint32 pFlags; // Is there a default value for the protocol flags? It looks like 0 would not make a good a default value here.

Comment on attachment 114738 [details] [diff] [review] Patch v 1.3 Darin, Please see my previous comment. The latest patch is pertty much the same as the previous one, except that it uses (as you suggested) the protocolhandler flags to identify possibility of relative uris in nsLocation ::FindUsable BaseURI(). Please provide your super review comments. Thanks.

Comment on attachment 114738 [details] [diff] [review] Patch v 1.3 this looks right to me. sr=darin radha: the protocol flags almost always include either URI_STD or URI_NORELATIVE. 0 is not a common value, but i don't think it's anything to worry about :)

Comment on attachment 114738 [details] [diff] [review] Patch v 1.3 + nsCOMPtr<nsIIOService> ioService(do_GetService(NS_IOSERVICE_CONTRACTID, &rv)); + + while(NS_SUCCEEDED(rv) && baseURI && ioService) { Loose the double error check here, if ioService is non-null, do_GetService() *did* succeed. ... + protocolHandler->GetProtocolFlags(&pFlags); + if (!(pFlags & nsIProtocolHandler::URI_NORELATIVE)) { + *aUsableURI = baseURI; + NS_ADDREF(*aUsableURI); + return NS_OK; + } + + // Get the same type parent docshell + nsCOMPtr<nsIDocShellTreeItem> docShellAsTreeItem(do_QueryInterface(parentDS)); + if (!docShellAsTreeItem) + return NS_ERROR_FAILURE; + nsCOMPtr<nsIDocShellTreeItem> parentDSTreeItem; + docShellAsTreeItem->GetSameTypeParent(getter_AddRefs(parentDSTreeItem)); + nsCOMPtr<nsIWebNavigation> webNav(do_QueryInterface(parentDSTreeItem)); + + // Get the parent docshell's uri + if (webNav) { + rv = webNav->GetCurrentURI(getter_AddRefs(baseURI)); + parentDS = do_QueryInterface(parentDSTreeItem); + } + else + return NS_ERROR_FAILURE; Fix the indentation of the later part of this loop, indent it to line up with the first part... + } // while r/sr=jst if you change that.

Fix checked in.

verified on today's windows trunk, works fine. thanks!

The networking issue (never stops loading the page) has been filed as http://bugzilla.mozilla.org/show_bug.cgi?id=196259

This caused regression bug 196404.

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