401113 - Scrollbar position not always restored after recovering from crash

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[Michael Kraft [:morac]]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.8) Gecko/20071008 Firefox/2.0.0.8
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.8) Gecko/20071008 Firefox/2.0.0.8

Someone reported a Session Manager bug that Session Manager wasn't restoring the scroll position when recovery a crash, but that Firefox did. See https://www.mozdev.org/bugs/show_bug.cgi?id=17987

Upon researching this I determined that even without any extensions installed Firefox 2.0.0.8 (and Minefield 2007102504) would not always restore the scroll position when recovering from a crash.

It appears to depend on how long it takes the page to start loading.  If it loads quickly the scroll bar position is restored, if not the scroll bar position is not restored.

As far as I can tell this only affects crashes.  I could not reproduce the problem when setting the "Show my windows and tabs from last time" feature in Firefox.

Based on the bug report I received, this appears to be a regression since he stated the problem started in 2.0.0.8.

I'll attach a sessionstore.js file that seems to trigger it pretty reliably (though that could change if the mozdev site speeds up).

Reproducible: Sometimes

Steps to Reproduce:
1. Open a few https and http pages and scroll part way down the page. Try and pick at least one page that takes a while to start loading.
2. Wait 10 seconds and kill the Firefox.exe process.
3. Start Firefox and choose recover.
Actual Results:  
Sometimes the scroll position on pages is not restored.

Expected Results:  
The scroll position should be restored in all cases.

This is tricky to reproduce since it doesn't seem to depend on how fast the page actually loads, but how fast it starts to load.  In my tests I used https://www.mozdev.org/bugs/show_bug.cgi?id=17987 which sat at "loading..." for about 5 seconds before the page started to load.  Any less than that and the scroll position will be restore.

As I mentioned this only seems to be an issue when restoring a crash, not when restoring windows and tabs from last time.

Comment 1

[Michael Kraft [:morac]]

Attachment: Test sessionstore.js which sometimes exhibits the problem

I've been able to reproduce the problem with this sessionstore.js file about 80% of the time.  Just close Firefox, drop it in your profile directory and start Firefox back up again.  You may have to repeat this a few times to recreate the problem since it appears to be a timing issue.

Comment 2

[Jesus Cea]

I confirm the regression.

Comment 3

[Jesus Cea]

I more trivial testcase:

1. Open firefox with a clear session.
2. Open a window with http://www.20minutos.es/noticia/297096/0/obras/ave/barcelona/
3. Scrolldown the page.
4. Open a new tab. Go to a site requiring FF basic authentication. Login.
5. Kill the browser abruptly. That is, the "kill" command, no the "exit" menu entry.
6. Restart the session.
7. DELAY the credential fillout 30 seconds.
8. Put your credentials there.
9. Go to the other tab: scroll position was lost.

Comment 4

[Daniel Veditz [:dveditz]]

I can reproduce this as well using steps in comment 3. FF2.0.0.7 restores the scrolling position, FF2.0.0.8 does not.

During Firefox 2.0.0.8 we fixed

bug 367605
When quitting Firefox before all pages have loaded, session saving forgets pages that weren't loaded

bug 384872
HttpOnly cookie attribute lost after session restore

bug 389274
[FIX]Midas demo. (designMode) fails to work properly after restoring with session restore

This is not serious enough to stop-ship FF2.0.0.9, but definitely something for FF2.0.0.10 in a few weeks

Comment 5

[Boris Zbarsky [:bzbarsky]]

I would place bets on bug 367605.

Is it possible that we're firing the session-saving timer before we've restored the data on the still-loading tab and thus clobbering that data or something?

Comment 6

[Jesus Cea]

I undid the patch in bug 367605 in today build, and the problem vanished. I ask for rollback patch in bug 367605 for 2.0.0.9, while work is done doing a regression free patch for 2.0.0.10.

I mark again the "blocking 2.0.0.9" bit in this bug, since this is a regression and the culprit is identified and trivially solved. If I'm being pushing too far, please, let me know.

Comment 7

[Boris Zbarsky [:bzbarsky]]

The real question is whether this bug or bug 367605 is worse.

Comment 8

[Michael Kraft [:morac]]

I'd focus on at that "browser.stop()" line added in the fix for bug 367605 because the behavior is similar to having the page stopped while loading.

Comment 9

[Jesus Cea]

Patch in bug 367605 creates bug 401097 too.

Comment 10

[Daniel Veditz [:dveditz]]

Not going to hold 2.0.0.9 for this one, it's going to take more time to craft a solution that fixes this while keeping bug 367605 fixed also. For most users this will be a minor annoyance after a crash -- at that point they're grateful for the recovery and can live with this for another release.

Not so great for those of us who live in "restore tabs from last time" mode but we'll just have to wait.

I'm assigning to Simon carrying over from bug 367605, but he's marked himself "busy" so we may need to find someone else.

Comment 11

[Simon Bünzli]

Attachment: don't update scroll data for incompletely initialized tabs

Comment 12

[Simon Bünzli]

Attachment: branch patch

Comment 13

[Reed Loden [:reed]]

Checking in browser/components/sessionstore/src/nsSessionStore.js;
/cvsroot/mozilla/browser/components/sessionstore/src/nsSessionStore.js,v  <--  nsSessionStore.js
new revision: 1.84; previous revision: 1.83
done

Comment 14

[Daniel Veditz [:dveditz]]

Comment on attachment 286705 [details] [diff] [review]
branch patch

approved for 1.8.1.12, a=dveditz for release-drivers

Comment 15

[Reed Loden [:reed]]

Checking in browser/components/sessionstore/src/nsSessionStore.js;
/cvsroot/mozilla/browser/components/sessionstore/src/nsSessionStore.js,v  <--  nsSessionStore.js
new revision: 1.5.2.50; previous revision: 1.5.2.49
done

Comment 16

[Kay Abendroth]

verified1.8.1.12
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.12) Gecko/20080128 Firefox/2.0.0.12

Comment 17

[u279076]

in-litmus+
https://litmus.mozilla.org/show_test.cgi?id=7798

Comment 18

[Teodosia Pop]

I am able to reproduce this issue using  Mozilla/5.0 (Windows NT 6.1; rv:2.0b9pre) Gecko/20110109 Firefox/4.0b9pre.

Comment 19

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(In reply to comment #18)
> I am able to reproduce this issue using  Mozilla/5.0 (Windows NT 6.1;
> rv:2.0b9pre) Gecko/20110109 Firefox/4.0b9pre.

Please file a new bug with as much information as possible. This bug was fixed long ago.