138313 - major breakage in anchored pages loaded from session history

Status: RESOLVED DUPLICATE of bug 138134

(Core :: DOM: Navigation, defect)

Description

[Miloslaw Smyk]

Anchored pages loaded from session history do not jump to anchor at all.

From the report by Pete Flugstad:
-----------------
Go to:

http://www.jerrypournelle.com/mail/currentmail.html.

Scroll down a bit and click on the "Wednesday" link - this skips you 
forward in the current page to beginning of the Wednesday mail. 
The URL is:

http://www.jerrypournelle.com/mail/currentmail.html#Wednesday

Without scrolling, click Back and you go back to the top of 
the page (where you scrolled to in order to see the Wednesday
link).

Click Forward again, and the URL changes back to the Wednesday anchor,
but the web page itself stays put.
----------------------

Now, this problem is caused by the fix for bug #135679, which landed
as nsDocShell.cpp version 1.443.

I'll attach a patch in a moment.

Comment 1

[Miloslaw Smyk]

Attachment: A quick fix.

This should fix this bug and still allow the solution from bug #135679 to work,
but I'd like the people from there to verify this claim.

Comment 2

[Radha on family leave (not reading bugmail)]

This is essentially what I have described in bug 138134

Comment 3

[John Keiser (jkeiser)]

BTW, this is a regression of bug 59744, which was recently fixed.

Comment 4

[Radha on family leave (not reading bugmail)]

I don't think the patch here will work well. In some cases, it would say, there
was an anchor, even though there was no anchor. I think what should be done,
based on what bug 135679 is trying to achieve is, set *aWasAnchor to false, if
both the current uri and the new uri are totally different.  But I haven't read
thro' bug 135679 yet. May be Ere Maijala can comment on this. 

As a last resort,  I would like to back out bug 135679, since it was not a bug
targeted for mozilla1.0 or nominated for the current release. That should
restore anchor behavior back to what it was for the current release  which in my
opinion is a must.

Comment 5

[Radha on family leave (not reading bugmail)]

jkeiser, this  bug is not a regression of 59774. This bug is a regression of
135679. (Please see my previous comments).  Bug 138134 is a regression of 59774.

Comment 6

[John Keiser (jkeiser)]

I agree with backing out the scrollifanchor stuff in that bug.  I am no longer
convinced it is even possible to fix that bug until we pass different load flags
or something for form posts.

Don't bother backing out the stuff in nsFormSubmission.cpp--it was fine, and
orthogonal to the patch that is breaking this.

Comment 7

[Miloslaw Smyk]

John is right, the changes to ScrollIfAnchor() should be backed out.

Radha, my patch helps, but not in all cases. But, returning wasAnchor set to
TRUE even if new page has no anchor is correct in some situations. We discussed
this with John on IRC and the consensus was to change the variable's name to
avoid such misunderstandings in the future.

Comment 8

[Radha on family leave (not reading bugmail)]

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