nsISHistory::requestedIndex is not reset after page reload through nsSHistory::Reload

RESOLVED FIXED in mozilla8

Status

()

Core
Document Navigation
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: khuey, Assigned: khuey)

Tracking

(Blocks: 1 bug, {regression})

unspecified
mozilla8
x86
Windows 7
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Start on page A, navigate to page B, then page C.  Click back.  Refresh (what is now page B).  The navigation buttons will be greyed out when they should not be.
I couldn't reproduce on a few sites I tried.  Can you try a more detailed str?
Are you trying a build with the patch from bug 622315?
Ah, nope.
Yeah, this is a regression from that.  I backed it out since I didn't have any clue how to fix this.
So the issue, presumably, is that nothing actually resets the requested index when the navigation completes?
Should we be comparing requested index to current index as well, not just to -1?
Created attachment 537785 [details] [diff] [review]
Patch

This fixes this behavior for me and passes the tests in docshell/

I'm going to try to write a mochitest for this behavior.
Attachment #537785 - Flags: review?(bzbarsky)
(In reply to comment #5)
> So the issue, presumably, is that nothing actually resets the requested
> index when the navigation completes?

Yes, this is correct.  The attached patch does that.

(In reply to comment #6)
> Should we be comparing requested index to current index as well, not just to
> -1?

I don't think so.  I think we should avoid navigating whenever there is a pending navigation.
Summary: Back and then reload results in grayed out navigation buttons → nsISHistory::requestedIndex is not reset after page reload.
Comment on attachment 537785 [details] [diff] [review]
Patch

Hrm...  Changing this stuff is scary as hell, but ok...
Attachment #537785 - Flags: review?(bzbarsky) → review+
Yeah, I'm terrified too :-)
So, fun thing, this only occurs if the reload is triggered through nsSHistory.  If the reload goes through nsDocShell we do an internal load and never muck with history.
Summary: nsISHistory::requestedIndex is not reset after page reload. → nsISHistory::requestedIndex is not reset after page reload through nsSHistory::Reload
Patch:

http://hg.mozilla.org/projects/cedar/rev/580ddd2f1267

Regression test:

http://hg.mozilla.org/projects/cedar/rev/ea5ed0f2a952
Whiteboard: fixed-in-cedar
http://hg.mozilla.org/mozilla-central/rev/580ddd2f1267
http://hg.mozilla.org/mozilla-central/rev/ea5ed0f2a952
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: fixed-in-cedar
Target Milestone: --- → mozilla7
Depends on: 670318
This was backed out of Aurora to fix Bug 670318.

http://hg.mozilla.org/releases/mozilla-aurora/rev/cd5b516e9a39
http://hg.mozilla.org/releases/mozilla-aurora/rev/5cf936e830cf
Target Milestone: mozilla7 → mozilla8

Updated

6 years ago
Depends on: 680344
Depends on: 678872
No longer depends on: 678872
You need to log in before you can comment on or make changes to this bug.