227466 - Clicking "Back" button inhibits subsequent clicks until the server responds

Status: RESOLVED INCOMPLETE

(Core :: DOM: Navigation, defect, P3)

Description

[hades_depths]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; fr-FR; rv:1.6b) Gecko/20031202
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; fr-FR; rv:1.6b) Gecko/20031202

This bug happens when the remote server does not respond immediately. When you
click on the "Back" button once, clicking another time on the button (in order
to view the previous page) may be ignored.

This is particularly annoying when you want to go to the page before the
previous one and you want to press Backspace twice rather than go and take your
mouse to click the tiny arrow near the "Back" button (FYI, Internet Explorer
handles this correctly).

Reproducible: Always

Steps to Reproduce:
1. Load a dummy webpage
2. Then load a page on a laggy server (dynamically-generated pages are better
because they force Mozilla to reload the page)
3. Load another dummy page
4. Now press your Back button (Mozilla should start downloading webpage 2), wait
a while (half a second) so that Windows does not believe you double-clicked on
the button, and press it again.
Actual Results:  
Mozilla won't get you to page 1 (it remains loading page 2).

Expected Results:  
It should be displaying page 1.

Comment 1

[Hixie (not reading bugmail)]

Yeah, this is annoying.

Comment 2

[hades_depths]

Attachment: Patch for chrome://browser/content/browser.js

Quick-fix that addresses the problem.

Comment 3

[hades_depths]

I forgot to mention the side-effects of my patch : it behaves the intended way
in most cases, but if there is a script on a page that performs
"history.back()", then the next click on the Back button will have no effect.

This is a real mess, and I don't think there's any ideal way to do this (if you
play around with IE 6, you'll notice that management of such stuff is really
buggy : try and press Back several times, and press Refresh before the page has
loaded -- the session history will be completely messed up).

The cleanest way would probably be to create a nsIWebNavigationListener, which
would be notified whenever
goBack()/goForward()/gotoIndex()/any_function_possibly_modifying_the_history are
invoked... But I haven't had a look at the low-level implementation of
nsIWebNavigation, so I would be unable to do it myself at the moment.

Comment 4

[hades_depths]

Attachment: Patch for chrome://browser/content/browser.js

Now this is the real thing. It's compatible with Javascript, plugins and other
sorts of stuff (I used a nsISHistoryListener). I did not find any bug, but
there is a place which I did not manage to write very cleanly (the
QueryInterface method), though it works just fine with existing versions of
Mozilla or FireFox and should not normally be broken in future releases.


Does anyone know how I can get the patch reviewed, and eventually merged into
the main branch ?

Comment 5

[hades_depths]

Attachment: Patch for chrome://browser/content/browser.js

Oops, forgot one line in the last patch.
Sorry for the spamming...

Comment 6

[hades_depths]

Attachment: Patch for chrome://browser/content/browser.js

Fixed the dirty hack in QueryInterface. I think the patch is now OK for
merging.

Comment 7

[jag (Peter Annema)]

Shouldn't this functionality be built into our default session history component
so that other users of it (e.g. Camino) will get this bug fix for free?

Comment 8

[hades_depths]

(In reply to comment #7)
> Shouldn't this functionality be built into our default session history component
> so that other users of it (e.g. Camino) will get this bug fix for free?

Well, you might be right. I know close to nothing about Mozilla's architecture,
I just hacked this out after looking at the location where the Back/Forward
commands were issued (that is, browser.js under Firefox).

But, after a quick look, there are several problems :
- when pressing Back/Forward, browser.js doesn't call sessionHistoryUI.js (which
is used, as far as I can see, for displaying the sessionHistory, not for
handling related commands). I believe that WebNavigation's goBack()/goForward()
are handled by C++ code, so this might be harder to modify
- anyway, goBack() and goForward() just _have to_ behave the way they do :
imagine a script saying it wants to go back one page. Typically, you do want the
browser to load the page previous to that page, which is fairly different from
the user pressing Back once again... So, this inherently has to be done by UI
code, not internal stuff.

But perhaps it would be cleaner to define it somewhere else than browser.js,
though a bit of extra interfacing would be needed (we need to catch Back/Forward
clicks, listen to Session History events, and catch location change events
[which occur when you change tabs, for example] ). I don't think I know other
platforms (e.g. Camino) well enough for doing it myself, so would you care doing it?

Comment 9

[jag (Peter Annema)]

(In reply to comment #8)
> But, after a quick look, there are several problems :
> - when pressing Back/Forward, browser.js doesn't call sessionHistoryUI.js (which
> is used, as far as I can see, for displaying the sessionHistory, not for
> handling related commands). I believe that WebNavigation's goBack()/goForward()
> are handled by C++ code, so this might be harder to modify

Yeah, that's the session history code I meant:
http://lxr.mozilla.org/seamonkey/source/xpfe/components/shistory/src/nsSHistory.cpp#463

> - anyway, goBack() and goForward() just _have to_ behave the way they do :
> imagine a script saying it wants to go back one page. Typically, you do want the
> browser to load the page previous to that page, which is fairly different from
> the user pressing Back once again... So, this inherently has to be done by UI
> code, not internal stuff.

I'm not quite sure what you're trying to say here. The goBack() code should take
you back a page (this supports calls from JS) but should also allow us to
interrupt a load (from a previous back or forward) in progress and take us back
to the next one.

Unless perhaps you're afraid that when someone clicks the back button, the
page's onunload could call history.go(-1) (evil, regardless!) and thus you'd end
up going back two pages. We'll have to guard against this.

Comment 10

[Robin Monks]

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