204364 - [FIX]Browser loses track of previous scroll position when going back to URL with anchor

Status: RESOLVED FIXED

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

Description

[Per Ångström]

If I have followed a URL with an anchor and I then follow a link and return back
again, Mozilla will lose track of where I was on the page - I will always end up
at the top of the page, having to reload the page or manually my previous position. 

If I had followed the same link from a non-anchored URL Mozilla would have
returned to my previous position. 

Seen in Mozilla Firebird/0.6-20030429 and Mozilla 1.4b-20030502 on Linux, and
Mozilla 1.4b-20030502 on Windows 98. I think this is a quite recent regression.
Netscape 7.02 doesn't have this annoying behaviour.

Instructions on how to reproduce the bug will be posted in a comment.

Comment 1

[Jo Hermans]

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

Comment 2

[Jo Hermans]

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

Comment 3

[Per Ångström]

How to reproduce:
1) Follow this link: http://bugzilla.mozilla.org/show_bug.cgi?id=204364#c0
2) Scroll down as far as possible.
3) Now follow this link: <http://www.mozilla.org/> and go back to the previous
page (using the back button or equivalent).
4) Note where the browser returns to.

Expected result:
The browser should return exactly to where I was before following the link to
another page.

Actual result:
The browser returns to the top of the page.

The problem is 100 % reproducible.

(Sorry about the duplicates - I can't see how they happened.)

Comment 4

[Jo Hermans]

At the first look, it seems bug 59774 again, but I think it depends on the
length of the page.

I can't repeat it if I go to
http://bugzilla.mozilla.org/show_bug.cgi?id=204184#c7 and click on the "comment
4" link. When I click the back-button, I'll be correctly transported to comment #7.

The test-case for this is
http://mozilla.org/quality/browser/front-end/testcases/history/anchor-history.html
and it works perfectly in build 2003050208 on Mac OS X 10.2.5

Comment 5

[Mitch]

I am seeing this too - Linux with mozilla cvs 20030503

Comment 6

[Felix Miata]

cf bug 47350

Comment 7

[Per Ångström]

Re comment #4: Yes, it looks suspiciously like bug #59774. As for the testcase
you mention, it uses links that refer to the same page - that works for me too.

Comment 8

[Boris Zbarsky [:bzbarsky]]

This works in build 2003-04-15-08 but is broken in a current CVS build.  I can't
test nightlies newer than 2003-04-15-08, so someone else should really narrow
the regression window here....

Comment 9

[Jo Hermans]

I just saw it in a forward-movement :

- go to http://www.mozillazine.org/talkback.html?article=3136&message=8#8
- page is now scrolled to one of the replies in the mozillazine-forum
- go backwords with the back-button
- and forwrad again
- page is now at the top of the talkback-page, not scrolled to the correct
position anymore

Comment 10

[Calum Mackay]

I've just checked my nightly builds, and narrowed the regression to the 24-hour
period between 2003042505 & 2003042605 (checkout hour times in GMT). The former
works, the latter doesn't.

Comment 11

[Michiel van Leeuwen (email: mvl+moz@)]

Can it be the checkin for bug 186149 then? bz?

Comment 12

[Boris Zbarsky [:bzbarsky]]

It could.  In fact, it's likely.

Comment 13

[Boris Zbarsky [:bzbarsky]]

I'd much appreciate it if someone could test whether reverting that patch helps
this bug.

Comment 14

[Michiel van Leeuwen (email: mvl+moz@)]

I found it a bit hard to reproduce, but i could reproduce with the steps in
comment 3.
After backing out the patch, i couldn't reproduce anymore.
Can someone else confirm this?

Comment 15

[Boris Zbarsky [:bzbarsky]]

Yeah, comment 3 is the only set of steps that let me reproduce this....

Which part of the patch is problematic? The generic element part, or the
presshell part?

Comment 16

[Calum Mackay]

Confirmed that removing both calls to FlushPendingNotifications() fixes the
problem. I was reproducing using:

1. go to http://www.hacksrus.com/~ginda/venkman/faq/venkman-faq.html#2.7
2. go back, and then forward.

Linux/x86.

Comment 17

[Calum Mackay]

I tried 2 builds:

o With the call to FlushPendingNotifications() in GenericHTMLElement, but not in
PresShell. This build worked; I could not reproduce this bug.

o With the call to FlushPendingNotifications() in PresShell, but not in
GenericHTMLElement. This build did not work; I could reproduce this bug.

Comment 18

[Boris Zbarsky [:bzbarsky]]

Attachment: This "fixes" the bug....

Comment 19

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 122511 [details] [diff] [review]
This "fixes" the bug....

So... I suppose we can just do this, but I'd dearly like to know why that flush
screws up framestate restoration....  Note that the GoToAnchor call is coming
in from the HTML content sink's DidBuildModel, so this is basically _right_
before EndLoad() is called on the presshell...

Comment 20

[Boris Zbarsky [:bzbarsky]]

I guess I should take this...

Comment 21

[Asa Dotzler [:asa]]

I think we want this for 1.4b. Thanks for the fix bz. 

Comment 22

[John Keiser (jkeiser)]

Comment on attachment 122511 [details] [diff] [review]
This "fixes" the bug....

I remember another flush we had to mess around with that had to do with the
anchor scroll problem; this probably has to do with the fact that we don't
store a separate session entry for anchors.  We *are* now representing it on
the url bar multiple times, oddly enough since the pages are always the same.

Given that this will still accomplish the purpose for which it was added,
r=jkeiser

Comment 23

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 122511 [details] [diff] [review]
This "fixes" the bug....

sr=dbaron, although I suspect you could also just move the call down a few
lines.

Comment 24

[Asa Dotzler [:asa]]

Comment on attachment 122511 [details] [diff] [review]
This "fixes" the bug....

a=asa (on behalf of drivers) for checkin to 1.4b.

Comment 25

[Boris Zbarsky [:bzbarsky]]

Checked in. 

I'm still sorting out whether reflow can trigger a frame reconstruct (eg with
CantRenderReplacedElement or some such), so I'm not sure whether GPFF would
return the same thing before and after the flush... hence the placement of the code.