Reloading a page whose length changes causes scroll position to change

RESOLVED FIXED in mozilla1.9alpha1

Status

()

Core
History: Global
RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com], Assigned: Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com])

Tracking

(Blocks: 1 bug)

Trunk
mozilla1.9alpha1
x86
Windows XP
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments)

Testcase and directions to reproduce coming in a minute.
Created attachment 171551 [details]
Testcase (see comment #1)

1. Save the page somewhere, and load it in mozilla.
2. Scroll down so the first set of As is at the very top of the page.
3. Add a new block of As and Bs (I just copied the last four blocks) to the
file, save it.
4. click reload in the browser.

Actual results:
You're now farther down the page than you were.

Expected results:
Stay at the correct scroll location.
Further down the page, or further up the page?  I'd expect us to be at the same
pixel position from the top of the page.
(In reply to comment #2)
> Further down the page, or further up the page?  I'd expect us to be at the same
> pixel position from the top of the page.

Down.  I did some cursory debugging and nsGfxScrollFrameInner::RestoreState does
see the correct y-offset, so whatever line 2267 talks about might be causing the
problem.  I started with the short testcase, scrolled down, then lenghthened the
page and hit reload.  2010 was saved in the state and properly restored, but the
page ended up down a couple of lines from where it was before.  Hitting reload
again saved and restored a y-offset of 2670, so between the initial state
restoration and the result getting on-screen, something changed.
Created attachment 196463 [details] [diff] [review]
patch

This works.  I have no idea why the code does what it does right now - the
unpatched behavior keeps the top of the scroll thumb at the same position and I
guess puts you at the same percent down the page.  With the patch, you end up
at the same *position* down the page.
Attachment #196463 - Flags: superreview?(roc)
Attachment #196463 - Flags: review?(bzbarsky)
Target Milestone: --- → mozilla1.8beta5
Comment on attachment 196463 [details] [diff] [review]
patch

Looks good --- for trunk. I recommend not changing this for 1.8. It's always
been this way until now and there's no reason to take any risks fixing it.
Attachment #196463 - Flags: superreview?(roc)
Attachment #196463 - Flags: superreview+
Attachment #196463 - Flags: review?(bzbarsky)
Attachment #196463 - Flags: review+
Status: UNCONFIRMED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Target Milestone: mozilla1.8beta5 → mozilla1.9alpha
Status: RESOLVED → UNCONFIRMED
Resolution: FIXED → ---
Assignee: nobody → cst
Status: UNCONFIRMED → RESOLVED
Last Resolved: 12 years ago12 years ago
Resolution: --- → FIXED
Blocks: 251784
(In reply to comment #5)
> (From update of attachment 196463 [details] [diff] [review] [edit])
> Looks good --- for trunk. I recommend not changing this for 1.8. It's always
> been this way until now and there's no reason to take any risks fixing it.
> 

Given how far off trunk is from reaching users and the low risk and high impact for web-forum users, could we take this for 1.8.1?
I wouldn't consider any patch to this code low risk, fwiw.
(In reply to comment #7)
> I wouldn't consider any patch to this code low risk, fwiw.
> 

Ok, but it has been on trunk for months without regressions...
You need to log in before you can comment on or make changes to this bug.