83673 - Scroll position not being remembered when using relative refs (#s)

Status: VERIFIED DUPLICATE of bug 59774

(Core :: DOM: Navigation, defect)

Description

[gabriel]

This bug is a follow up to bug 46877. If you click the above URL and follow one
of the links in the 'Method Summary', then click back, the scroll postion is lost.

I believe this is due to the link being inside a table, since if you click a
link on the same page which is outside a table, the scroll position is restored
correctly.

Comment 1

[Radha on family leave (not reading bugmail)]

pollmann may have a clue here.

Comment 2

[Radha on family leave (not reading bugmail)]

pollmann/evaughan may know what's going on here.

Comment 3

[Miloslaw Smyk]

I think I know. :)

First, this issue is NOT related to tables. All links in this table are relative
ones pointing to the same document, and those are relative references that are
causing problems here. I am updating summary line accordingly.

I've been chasing this bug all day and it looks like fix won't be an one-liner.
Basically, while with normal links mLayoutHistoryState of nsSHEntry is used for
preserving information about location on a page and at the right moment is
restored in PresShell::EndLoad() with a call to
FrameManager::RestoreFrameStateFor(), this isn't the case with relative links.
Moving back and forward in history of these uses PresShell::GoToAnchor()
instead, which is called by nsDocShell::ScrollIfAnchor() and looks almost like
side effect of using ScrollIfAnchor for checking if given URI and nsSHEntry are
relative or not.

I still have two-three hours of spare time, so I'll try to create something
reasonable to fix it, but a person better acquainted with the code may be needed.

Comment 4

[gabriel]

Is that because we assume there is only one mLayoutHistoryState allowed per page
? (So when we scroll to a different part of it, we don't store the current
scroll position ?). Isn't the fix then (guessing without looking at the code):

if going from page to page#anchor, store mLayoutHistoryState before calling
PresShell::GoToAnchor()

if going from page#anchor to page, call PresShell::GoToAnchor(), restore
mLayoutHistoryState for the page, and then call
FrameManager::RestoreFrameStateFor()

(I am assuming, maybe wrongly, that you can store mLayoutHistoryState without
destroying it).

Comment 5

[Miloslaw Smyk]

Well, I'm not sure if there's only one mLayoutHistoryState per page, as this
pointer is stored in nsSHEntry and you can have as many of these as you wish.
Other than that, I've implemented something quite close to what you describe,
unfortunately without much success.

I suspect my understanding of FrameManager::CaptureFrameStateFor() and friends
is not good enough and the stuff I've done purely by intuition is not working as
it should. Unfortunately this is going to be too busy week at work to allow me
learn this part of the code, so it'd be nice if someone else could fix it.

Comment 6

[James Paige]

This happens on Windows also. I dont have a Mac to test it on, but I am pretty
sure this is platform:All, OS:All

Comment 7

[Miloslaw Smyk]

Yup, the code suggests the same. Changing Platform/OS to ALL.

Comment 8

[Martyr]

Still there on Moz 0.92 Win NT.

Build: Mozilla/5.0 (Windows; U; WinNT4.0; en-US; rv:0.9.2) Gecko/20010628
Seen with:
1) Large (14 links) table with relative links to named anchors in the document,
with each one of those anchors actually defined in the document.

Not seen with:
1) Large (14 links) table w/ relative links to named anchors in the document,
with only one named anchor actually defined in the document. 
2) Small table (2 links) of the same. 

Comment 9

[Jesse Ruderman]

This bug is a duplicate of bug 59774.

Comment 10

[gabriel]

Yes it is.

*** This bug has been marked as a duplicate of 59774 ***

Comment 11

[Aaron Kaluszka]

v