137632 - Back/forward does not scroll to exact same location with XML files, and does not select target if pref set

Status: VERIFIED FIXED

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

Description

[Heikki Toivonen (remove -bugzilla when emailing directly)]

1. Go to URL, notice that "Self emplyment tax" is the first thing visible.
2. Go to some other url, hit Back button

Expected results: to scroll to the exact same location (and select target if
"layout.selectanchor" pref set)

Actual results: the scroll does not happen to the same location. On my screen we
scroll so that above the expected result the first thing we show is "in
Publication 535.". Additional issue is that if you have set the pref
"layout.selectanchor" you should get the anchor selected when you come back.
That does not happen. See PresShell::GoToAnchor() what it does when you follow
the anchor normally.

These things are regressions from bug bug 59774, which broke back/forward
completely for XML. The fix for bug 136788 gets us to the current situation.

Comment 1

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Attachment: Proposed fix: do as we always did

This seems to give me the old behavior. Is this safe? I would think so, we
might just try to scroll from history and sink, but that is only a slight perf
hit that I am willing to take for correct XML functionality. I would rather
take this than wait for months for a better fix. Radha?

Comment 2

[Radha on family leave (not reading bugmail)]

Heikki,  I see what you are saying. I would suggest that you go ahead with this
proposed patch, if you think this needs to be taken care for mach V.I need to
look into PresShell and understand this pref too, before I can propose a right
fix and it is not my priority right now. Thanks.

Comment 3

[Heikki Toivonen (remove -bugzilla when emailing directly)]

So r=radha, please ;) ?

Comment 4

[Radha on family leave (not reading bugmail)]

r=radha. (presuming you have tested the patch with/without the appropriate prefs
and other scenarios.)  The patch simply backs out the erroneous code that was
causing the regression. 

Comment 5

[Johnny Stenback (:jst)]

Comment on attachment 79352 [details] [diff] [review]
Proposed fix: do as we always did

sr=jst

Comment 6

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Fixed on the trunk, and taking. I'll file a new bug on scroll position not
working properly with XML, and refer to this bug as well.

Mailing drivers.

This bug and bug 136788 should be considered together by ADT & drivers. We are
effectively backing out bug 59774 for XML cases.

Comment 7

[Jaime Rodriguez, Jr.]

Claduis - Can you verify that this has been fixed on the trunk and caused no
new, known regressions? thanks!

Comment 8

[Jaime Rodriguez, Jr.]

Resolving as fixed, because the fix has landed on the trunk. Pls change to
verified, when the fix has been verified, with no regressions.

Comment 9

[Claudius Gayle]

VERIFIED Fixed on trunk builds 2002041803, marking as such

Comment 10

[Jaime Rodriguez, Jr.]

adt1.0.0+ (on ADT's behalf) approval for checking into the 1.0 branch. Pls check
this in today, then add the fixed1.0.0 keyword.

Comment 11

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 79352 [details] [diff] [review]
Proposed fix: do as we always did

a=[e-mail] for branch checkin

Comment 12

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Checked in on branch as well.

Comment 13

[Claudius Gayle]

VERIFIED Fixed on the branch with 20020424 builds. wrote bug 140455 to address
smaller issue discovered while testing.

Comment 14

[Radha on family leave (not reading bugmail)]

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