215405 - scroll position not remembered on pages with cache-control: no-store http header

Status: VERIFIED FIXED

(Core :: DOM: Navigation, defect)

Description

[Julian Gall]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4) Gecko/20030624
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4) Gecko/20030624

On the site given, the Back Button does not set the page to the same scroll
position it was in when the link was clicked. Other sites work as expected in
Mozilla. This site works as expected in IE and Opera.

Reproducible: Always

Steps to Reproduce:
1. Scroll to the bottom of the indicated page.
2. Click a link to go to another page in the site.
3. Click the Back Button.
Actual Results:  
The original page is positioned scrolled to the top.

Expected Results:  
The original page should be positioned scrolled to the bottom, which is where it
was when the link was clicked.

This is a problem throughout this web site and makes it very clumsy to read news
stories at the bottom of the page. Click link, go back, scroll back down, repeat
etc. Other web sites don't seem to have the problem. Also, Opera and IE have the
expected behaviour (on this site) and remember where you were on the page when
you left it.

Comment 1

[Mitch]

This was a bug - can't remember the number, but i'll find it. It is definetly
been fixed. Can you try a newer build please ?

Comment 2

[Mitch]

Here you go. Bug 204364. Fixed in 1.4b.

Comment 3

[timeless]

mitch, did you look at the reporter's build id? it appears to be June 24 which
is 1.4 RC3 (which is 1.4 Final) see mozilla.org/releases/

Comment 4

[Julian Gall]

Yes, my version is 1.4. I downloaded it just before checking the bug within 
the last couple of hours

Comment 5

[Mitch]

Yes, my mistake. Sorry. 

Tough i should add, i can't seem to reproduce on 1.5b (todays cvs), so
may have regressed and been fixed again ?

Comment 6

[Brian Cole]

Still a problem in Firebird nightlys as of 9/23/03.

Comment 7

[Dai Toyama]

The platform should probably changed to "All." I can reproduce the problem using
the referenced Web site using my Linux build.

Comment 8

[MORA]

Well, the http://news.independent.co.uk/uk/ site actually works for me, using
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6a) Gecko/20031102
Firebird/0.7+. 

However, I do encounter this problem at other websites, such as
http://www.tweakers.net/, using the same set-up. I don't have Mozilla 1.4+
available to verify this. 

Comment 9

[kost8]

I can reproduce this bug in Mozilla 1.5 final on a Win98 PII platform.
Hit this link http://www.art-poetrybykiesha.com/gallery.html  then go
to Gallery 2, then back to Gallery again. The site is static not dynamic.
Can not re-produce on Firebird 0.7 milestone by Aebrahim

Comment 10

[John Lee]

I reproduced this bug at the forums on http://tontoclan1.com and I am on Windows
XP Pro. My user-agent string is:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031109
Firebird/0.7+ (aebrahim)

Comment 11

[Gunnar Schroeder]

I can confirm this for recent versions of Firebird and Mozilla:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031223
Firebird/0.7+
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031208

It happens on quite a few sites, not on all. And sometimes it even works, but a
few seconds later it again works not on some sites. Strange. Examples:
http://www.abxzone.com/forums
http://geizhals.at/eu/

The site in the original bug report always fails.

Comment 12

[Ratko Tomic]

This problem (Mozilla 1.5, Win32, win2k/sp2) occurs on the entirely static pages
and without any network activity (no packets going out), i.e. even when the
forward and the back pages are entirely in Mozilla's cache. For example the page:

 http://www.if.ufrj.br/famous/physlist.html

is a list of names of physicists and each link points to a jpeg picture on the
same site. If you click on any of the names, say "Galileo Galilei", then go
back, the browser sometimes forgets the referer position and goes to the top of
the list and sometimes to the original position. Even selecting the same picture
and going back and forth few times, it seemingly randomly decides to go to the
top or to restore the proper position from the referer page, all without any
network activity.

Comment 13

[Anthony]

Using Firebird 0.7 in Win2k, I have noticed two flavours of this bug.

1. As referenced above, when using the back button on some pages (dynamic?) you
are _always_ returned to the top of the page.

2. This also seems to occur less predictably with other (static?) pages.  I
really wish I could be more specific, but it seems like one in fifty times I
click back, I am returned to the top of the page rather than the link location.
There is a way to differentiate this second occurance from the first: upon
clicking back and being returned to the top of the page, resize the browser
window by dragging one of the edges.  This will immediately move the page to the
link location (where it should have been).

Comment 14

[Ratko Tomic]

Re #12 symptom -- Just installed 1.6 and the problem seems fixed.

Comment 15

[Gunnar Schroeder]

After reading comment #14 I did a fresh install of Mozilla 1.6 and created a new
profile. I still experience the bug.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6) Gecko/20040113

Comment 16

[Ratko Tomic]

Re #15: Yes it seems that the bug is still there in 1.6 release, although for
the site I used originally it is fixed. Here is a site which shows it every time:

 <a href="http://sprott.physics.wisc.edu/fractals.htm">Fractal of the day</a>

If you click on any of the picture links and go back the scroll position is
forgotten (IE5 returns correctly). It occurs without any network activity.

Comment 17

[Ratko Tomic]

Re #13: I checked your resize method on the link from #16 and it seems to fix
the provblem and from there on it was returning to the correct position. It
looks as if some variable wasn't initialized until resize occured. Once it was
set correctly, the position was remembered with or without further resize.

Comment 18

[Jesse Ruderman]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7a) Gecko/20040205
Firebird/0.8.0+ (scragz)

I can repro at
* http://news.independent.co.uk/uk/ 
* http://www.tigerroar.com/tigertown/
* http://www.tweakers.net/
All of these sites use cache-control: no-store (I used http://web-sniffer.net/
to check).

Layout history should try to restore scroll position (but not form contents)
even when we respect a site's request to not cache for Back.

Comment 19

[Jesse Ruderman]

Some comments on this bug are off-topic (not about sites that use cache-control
or pragma: no-cache) and should be turned into separate bugs.

Comment 20

[Jeff Buck]

Ebay seems to be a really easy site to reproduce this with.. I don't see the
cache control no cache listed for e-bay, so maybe this note belongs somewhere
else. I've seen this bug on ebay in every gecko browser that I've ever used, and
it's still here for firefox 0.8... 

I'm kinda surprised that there's no mention of ebay issues here. My girlfriends
been talking trash about linux over this one issue for 2 years, and ebay's
almost the only site she ever goes to :).. 

Comment 21

[loomis]

Definately still here with Firefox 0.8 and eBay. Still very annoying. Please fix
this bug.

Comment 22

[Jesse Ruderman]

eBay is bug 217120, not this bug.  eBay does not use cache-control.

Comment 23

[Ratko Tomic]

It seems that this bug is related to the strange way Mozilla handles Browser
Back operation -- it needlessly repaints the page from the top, then, as if on
some Done-message (maybe from the memory bitmap drawing engine) it jumps to the
final position. This jumpy behavior is disruptive even when it works and it
ought to be taken out -- on the Back-key simply keep the current page as is
until the previous page is ready to be shown, then just show the previous page
at the right place with no jumping.

The scroll problem seems to be the case of the Done-message not being sent or
being missed/ignored due to some race condition (e.g. if Done-message arrives
before the initial from-the-top repaint finishes, the Done-message gets
ignored/forgotten). The internal Back-position is not lost. Namely, you can
toggle Maximize/Restore window button and the correct position will get restored
in the alternate window size. Toggling window size again goes to the correct
Back position in the original window size.

On a general note, the Back operation should restore the exact previous view and
place and not let the web-site bully the user to the top. User can always hit
refresh if he wants to see the latest version of the page. Browser should be the
servant of the user not of the site marketoids or quirky designers.

Comment 24

[Asa Dotzler [:asa]]

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

Comment 25

[Manish Jethani]

Simple test case with Bugzilla:

1. Go to http://bugzilla.mozilla.org/
2. Search for some text, like "cache".

3. Scroll down a page or two, click on one of the bugs.
4. Go "Back". The query results page should scroll down to the position it left
in (the bug that was clicked on), but it remains at the top instead.

5. Go "Forward" now. Scroll to the bottom of the page. Click "Show List".
6. Go "Back". For this page, the scroll to the bottom of the page happens correctly!

Comment 26

[Jesse Ruderman]

Bugzilla is bug 47350, not this bug.  Bugzilla does not use cache-control.

Comment 27

[Florian Munz]

I can reproduce this on any WordPress based Weblog (e.g. http://theflow.de)
which has the somewhat aggresive default cache-settings:

@header("Expires: Mon, 26 Jul 1997 05:00:00 GMT");
@header("Last-Modified: " . gmdate("D, d M Y H:i:s") . " GMT");
@header("Cache-Control: no-store, no-cache, must-revalidate");
@header("Cache-Control: post-check=0, pre-check=0", false);
@header("Pragma: no-cache");

if I comment out the two Cache-Control headers the scroll position is remembered

(Firefox 0.8 and Mozilla 1.7b on Windows XP, it's working on the OSX Firefox 0.8
for me)

Comment 28

[Boris Zbarsky [:bzbarsky]]

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

Comment 29

[Boris Zbarsky [:bzbarsky]]

So the problem here is the code in nsDocShell::EndPageLoad that does the following:

    /* Check if the httpChannel has any cache-control related response headers,
     * like no-store, no-cache. If so, update SHEntry so that 
     * when a user goes back/forward to this page, we appropriately do 
     * form value restoration or load from server.
     */

followed by:

        PRBool discardLayoutState = ShouldDiscardLayoutState(httpChannel);       
        if (mLSHE && discardLayoutState && (mLoadType & LOAD_CMD_NORMAL) &&
(mLoadType != LOAD_BYPASS_HISTORY))
            mLSHE->SetSaveLayoutStateFlag(PR_FALSE);            

The problem is that layout state includes not just form values but also scroll
positions.

There's similar code in nsDocShell::AddToSessionHistory also....

Do we want to change SetSaveLayoutStateFlag to not include scroll positions?

Comment 30

[Boris Zbarsky [:bzbarsky]]

That would need to be done in the presshell code (see
PresShell::CaptureHistoryState, etc).  But the problem is that this just calls
SaveState on the frames in the frame tree.  It has no concept of different
_types_ of state.

Here we're trying to differentiate between state that should always be persisted
(scroll positions) and state that should only sometimes be persisted (form
control state)....

Comment 31

[Boris Zbarsky [:bzbarsky]]

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

Comment 32

[Jason Bassford]

The dependency status of bug 251784 is incorrect.  This bug blocks the tracking
bug, not the other way around.  (Only once all of the bugs tracked are resolved
will the tracking bug be resolved.)

Fixing the dependency relationship for this bug - somebody responsible for the
tracking bug should fix the others.

Comment 33

[Bugzilla]

http://www.sagem.com/support/site/page_accueil.php?site=1

shows the effect with Mozilla 1.72 unde W2K and XP too.
FF no tested.

I have a lot of IE users to "convert".
After a very short test they all find this bug and 
have a very strong reason to stay with IE ;-(

Comment 34

[Mark Pulver]

(In reply to comment #33)

> I have a lot of IE users to "convert".
> After a very short test they all find this bug and 
> have a very strong reason to stay with IE ;-(

"ditto". :(


And for the sake of version tracking, this bug also exists in:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.2) Gecko/20040811
Firefox/0.9.1+ StumbleUpon/1.995

Comment 35

[Boris Zbarsky [:bzbarsky]]

We're well aware the bug exists.  See comments on exactly which code is broken.
 If you have all this free time on your hands, feel free to submit patches...

Comment 36

[Ratko Tomic]

> See comments on exactly which code is broken.

Does that also explain the part of the behavior where, after failing to restore
the position on the Back key, if you resize the Browser window (by toggling
between Maximize or Restore), the lost position gets somehow restored for the
new resized window. That happens as long as you don't do any intermediate
scrolling within the document after it came back incorrectly. This behavior
doesn't exist with the bugzilla list variant of the scroll bug.

Comment 37

[Elan Shudnow]

This also happens to me with sites such as ebay.com and bestbuy.com.  When
scrolling down, clicking a link, and then hitting the back button, my browser
resets the vertical scrollbar to the top of the page, not the last place I left off.

Comment 38

[:Gavin Sharp [email: [e-mail]]]

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

Comment 39

[Bill Mason]

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

Comment 40

[Bugzilla]

The error persists still in 1.8a3

Why can't i be fixed?

I know a lot of users which takes that 
(very anyoing) bug as a (very good) reason to stay with IE...
As they would run into it at least 3 times every day.
Using tabs is no accepted work arround.

ebay seems to have changed their code, but
that could not be the way to go to wait until
all webpages will be made compatible to a mozilla bug ;-)

Comment 41

[Bugzilla]

Could the maintainer please change
Keywords: helpwanted
to
Keywords: conversion

as this fits much better?

Comment 42

[Bill Mason]

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

Comment 43

[Mike Pinkerton (not reading bugmail)]

Just want to point out that this is a high visibility issue. I get a bunch of
reports about this not working for Camino, and it works in Safari which makes us
look sloppy.

Comment 44

[Norbert]

the bug also appears in the bugzilla search results list
(https://bugzilla.mozilla.org/buglist.cgi?query_format= ...):

if i click a bug and go back, the scroll position is not restored.

Comment 45

[Bill Mason]

(In reply to comment #44)
> the bug also appears in the bugzilla search results list
> (https://bugzilla.mozilla.org/buglist.cgi?query_format= ...):
> 
> if i click a bug and go back, the scroll position is not restored.

See comment 26.

Comment 46

[chris hofmann]

dan, this would be another one to look at and make some progress if we can. 
might  be too hairy for taking on the aviary branch, but lets wait and see.

Comment 47

[Asa Dotzler [:asa]]

This would be nice to fix but any fix is likely to be too risky at this point.
If a patch materializes that doesn't scare us, then we'll consider it for
approval, but not going to block the release on this. 

Comment 48

[Ben Goodger (use ben at mozilla dot org for email)]

A couple of naive suggestions, but based on some investigation since this
adversely affects a number of sites I visit: 

- Maybe have some sort of state parameter passed through to SaveState that
specifies what sort of state should be saved? (all, scroll position)... 
- In 'cache-control: no-cache' mode only persist state for scrollboxframes -
this seems less specific though but may involve changing less code. 

I'm happy to play around with this code but would require guidance so I didn't
blunder down the wrong path ;-)

Comment 49

[Johnny Stenback (:jst)]

dbaron, any thoughts on if/why your trunk fix for bug 236889 fixed this crash on
the trunk? Worth migrating that fix to the aviary branch?

Comment 50

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The previous comment was most likely intended to be on bug 255372.

Comment 51

[Johnny Stenback (:jst)]

(In reply to comment #50)
> The previous comment was most likely intended to be on bug 255372.

Oh, ahem, yeah, it was. Duh.

Comment 52

[Loic Le Tersec]

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

Comment 53

[Ratko Tomic]

As an additional property of the scroll bug (which may be useful in trying to
get to the bottom of it), I have noticed that having multiple maximized browser
windows (e.g. opened via Ctrl-N), with some content in each of the windows,
makes the scroll position problem come up more frequently.

Comment 54

[Boris Zbarsky [:bzbarsky]]

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

Comment 55

[Boris Zbarsky [:bzbarsky]]

Ben, either of the solutions you propose in comment 48 sounds fine; the former
may actually be easier to implement.

Comment 56

[Carsten Book [:Tomcat]]

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

Comment 57

[Tristor]

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

Comment 58

[:Gavin Sharp [email: [e-mail]]]

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

Comment 59

[:Gavin Sharp [email: [e-mail]]]

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

Comment 60

[Marc Bejarano]

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

Comment 61

[Boris Zbarsky [:bzbarsky]]

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

Comment 62

[Elmar Ludwig]

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

Comment 63

[Elmar Ludwig]

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

Comment 64

[Matt]

So what you're looking at is playing around with history save states?  Making sure that the scrollbar position for frames should save no matter what should be the basis behind a patch, eh.  Also, any changes to that should probably make sure to save the literal last position of the scroll bar; if an extension or script removes or adds elements, the scroll should reflect that and go to the exact position rather than any certain elements it was focused on (i.e. if it saves the scroll state based on elements in the page rather than the literal distance from the top or side of the page scrolled, that should be changed).

Seems like a good idea on how to fix this?  If so, I'll see what I can do to try and figure out a possible patch.

Comment 65

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

So if I understand you right you want to restore the scrollpositions of scrollbars that are removed and added due to dynamic changes to the page? Sounds resonable to me but I'm not sure that fixing that is related to this bug.

But feel free to hack up a patch and if it fixes this bug attach it here, otherwise attach it to a new bug.

Comment 66

[mmortal03]

Cannot reproduce in main listed URL above. http://news.independent.co.uk/uk/ 
Can reproduce at http://www.tigerroar.com/tigertown/

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051027 Firefox/1.6a1

Comment 67

[Mark Mentovai]

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

Comment 68

[:Gavin Sharp [email: [e-mail]]]

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

Comment 69

[Jesse Ruderman]

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

Comment 70

[James Spooner]

Bug 322168 (https://bugzilla.mozilla.org/show_bug.cgi?id=322168) is similar to this bug, but only depends on the Expires header and is still a problem in Firefox/1.5.

Comment 71

[Richard Silverstein]

I have experience with this bug too.  It always happens at http://nytimes.com .  I'm using FF 1.5.  Very annoying & tedious as the original reporter noted.

Comment 72

[Jerry Baker]

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

Comment 73

[bindu]

What I understand is that this bug related to session history processing. Your guys may need to add or change the code in either nsDocshell.cpp and nsPresShell.cpp in order to deat with the history scroll position properly. For instance, in the function ScrollViewToShowRect(), you can double check if it is LOAD_HISTORY or LOAD_RELOAD_NORMAL. If not procedding, otherwise skip. Or do the similiar in nsDocShell.

By the way, this bug is always reproducable on www.msn.com

Comment 74

[Kenyon Ralph]

(In reply to comment #73)
> By the way, this bug is always reproducable on www.msn.com

No, scroll position is remembered for me on that page if I click to go somewhere else then go back.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.1) Gecko/20060226 Firefox/1.5.0.1

Comment 75

[bindu]

(In reply to comment #74)
> No, scroll position is remembered for me on that page if I click to go
> somewhere else then go back.
Ok, it is reproducable on the following version when the scroll position is not at the bottom.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1

Comment 76

[Darin Fisher]

Not going to block 1.8.1 for this.

Comment 77

[:Gavin Sharp [email: [e-mail]]]

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

Comment 78

[Phil Ringnalda (:philor)]

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

Comment 79

[sean_holland1]

(In reply to comment #75)
> (In reply to comment #74)
> > No, scroll position is remembered for me on that page if I click to go
> > somewhere else then go back.
> Ok, it is reproducable on the following version when the scroll position is 
> not at the bottom.
> 
> Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.1) Gecko/20060111
> Firefox/1.5.0.1
> 
It's still reproducable on http://tweakers.net/ and on http://www.tigerroar.com/tigertown/ but not at http://news.independent.co.uk/uk/ , http://nytimes.com/ , http://www.msn.com . In all of these cases the scroll bar was never at the bottom (because that seemed to be the problem).

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.7) Gecko/20060909 Firefox/1.5.0.7

Comment 80

[AnimalFriend]

Can confirm that this bug still exists on FF2.0b2 for the following sites
- http://tweakers.net/ (pragma: no-cache)
- http://www.heise.de/newsticker/ (expires: 900)

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b2) Gecko/20060821 Firefox/2.0b2

Comment 81

[Thomas Bertels]

(In reply to comment #80)
This bug is caused by the http header "cache-control: no-store", which can be seen with the extension LiveHTTPHeaders (http://livehttpheaders.mozdev.org/).
The current cause is the fix for bug 252023 (the no-store entity did not have any effect on non ssl pages before).

Comment 82

[Matt]

comment#81

I thought we already established that...

Comment 83

[Thomas Bertels]

(In reply to comment #82)
> comment#81
> 
> I thought we already established that...
> 
Indeed, but it seems it wasn't clear for some users.

Comment 84

[AnimalFriend]

(In reply to comment #81)
> (In reply to comment #80)
> This bug is caused by the http header "cache-control: no-store", which can be
> seen with the extension LiveHTTPHeaders (http://livehttpheaders.mozdev.org/).

LiveHTTPHeaders is not compatible with current FF 2.0. You know an alternative tool (there should be one in case the community shall be able to fix this bug).

Comment 85

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to comment #84)
> LiveHTTPHeaders is not compatible with current FF 2.0. You know an alternative
> tool (there should be one in case the community shall be able to fix this bug).

You can use the web-sniffer (http://web-sniffer.net/) to have a look at the HTTP headers.

Comment 86

[Flaky Jake]

This extension claims to address the issue > Restore Scroll Position 0.3 http://www.gozer.org/mozilla/extensions/

Comment 87

[Richard Silverstein]

(In reply to comment #86)
> This extension claims to address the issue > Restore Scroll Position 0.3
> http://www.gozer.org/mozilla/extensions/
> 
Thanks, Flaky Jake.  I've just installed it.  I'm hoping it'll work.  It would be a godsend if it did.

Comment 88

[Wiley Miller]

(In reply to comment #86)
> This extension claims to address the issue > Restore Scroll Position 0.3
> http://www.gozer.org/mozilla/extensions/
> 

This extension/patch appears to work with FF2 final release.  The behavior is a little different at certain sites though.  At msnbc.com, a return to a previous page always first goes to the top of the page, then jumps back down to the last scroll position.  It's a good patch, but a real fix to the code needs to be addressed.  This has been an issue since 2003 !!!

Comment 89

[Jerry Baker]

(In reply to comment #88)
> At msnbc.com, a return to a previous
> page always first goes to the top of the page, then jumps back down to the last
> scroll position.  

FYI: That's how FF works most of the time even when it is working "correctly"

Comment 92

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #22)
> eBay is bug 217120, not this bug.  eBay does not use cache-control.

eBay seems to send "Pragma: no-cache" and "Cache-Control:·private" headers for search results, no. That triggers this bug, right?

Comment 93

[Jesse Ruderman]

I don't think so.  I think no-store is the only way to trigger Firefox's anti-caching behavior.  So I don't think bug 374669 is a dup.

Comment 94

[Thomas Bertels]

bug 374669 isn't related to this bug, it's JavaScript related.

Comment 95

[Daniel Schroeter]

In the past I have to switch to opera/konqueror more and more often for sites using cache-control. Those group are increasing in the past.
69 votes and a browser with a market share about 20% and no one can fix this bug since 2003!? Or is interested of a fix?

Comment 96

[Hideo Saito]

Attachment: patch

I think this patch to make it possible to return to the scroll position without using history session.

Comment 97

[Hideo Saito]

Attachment: patch

Comment 98

[Hideo Saito]

Comment on attachment 260091 [details] [diff] [review]
patch

David, could you review this patch?

The behavior that the scroll position returns to is same as comment 88.

> a return to a previous page always first goes to the top of the page,
> then jumps back down to the last scroll position.

However, this is unavoidable because this patch doesn't use the session history except mScrollPositionX and mScrollPositionY.

Comment 99

[Worcester12345]

any relation?

Bug 378606 – [FIX]Does not return to the scroll position in the page after reload 

Comment 100

[Marc Bejarano]

no

Comment 101

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 260091 [details] [diff] [review]
patch

I should have redirected this review request when I got it.  Sorry I didn't catch it.  I'm really not an appropriate reviewer for this code.  So redirecting the review request to Boris.

I'm a little puzzled by the first chunk of the patch -- whether it should be operating on mOSHE or mLSHE, and whether it's the right time (see also bug 103279).  But Boris should know what should be going on here.

Comment 102

[Boris Zbarsky [:bzbarsky]]

So before I start digging into the mOSHE/mLSHE mess, can you explain what you think the patch is doing?

Comment 103

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 260091 [details] [diff] [review]
patch

r- pending reply to comment 102.

Comment 105

[Tim Keenan]

This also affects people using accessibility products, though no worse than anyone else.
I personally have been wondering about this behavior for a while now.

Comment 106

[Daniel Schroeter]

We are having more then 70 votes and a patch for this annoying bug.
Can someone give the patch a chance/review, please!!!? I have to use an other browser for one often visit homepage and would be happy to use the next FF3 for all browsing activities....

Comment 107

[Boris Zbarsky [:bzbarsky]]

I did give it a chance.  It looks wrong to me, offhand, but I would be glad to have the patch submitter explain why he thinks it's right.  He hasn't bothered to.

Comment 108

[n808]

Adding another vote. A frequently visited site of mine, http://runryder.com (R/C helicopter forum) is a true pain to browse due to this bug, and I will be switching browsers on that site, now that I know how it should work.  

The extension "Remember Scroll Position" on http://www.gozer.org/mozilla/extensions/ only partially works -- tool bar buttons only, not mouse buttons, which is what I use for back and forward.

Comment 109

[Boris Zbarsky [:bzbarsky]]

Yes, we know this is a problem.  Please see https://bugzilla.mozilla.org/etiquette.html ?

Comment 110

[Richard Silverstein]

Thanks n808 for letting us know about the extension.  I just installed it & hope it will help.

Comment 111

[GBB1]

In Firefox 3 beta 5 this bug is still there.
Please check on this site to confirm the problem (already confirmed by other forum members):
http://core.tweakers.net/

Comment 113

[Daniel Schroeter]

Some more sites I have found the last week:
http://www.zoll-auktion.de/auktion/browse.php?id=191
http://www.barometer-group.de/frankfurt/Inserenten-2008/Gastronomie.html
To reproduce: Scroll down the page and click on one item. Use the back-button in Firefox and you will land on top of the page.
On those sites it is very grueling to use Firefox (3.0b5). Maybe the German webmasters prefer cache-control: no-store very often :-((
I fear that I will not use the final Firefox 3.0 on those site. Every other Browser (Konqueror, IE, Opera) don't have any problems with this sites.

Comment 114

[Frank Wein [:mcsmurf]]

Daniel: We already know enough URLs to reproduce it. Please do not add further comments to this bug here unless it really brings the bug forward. We are waiting for the patch author and/or a new patch (possibly from someone else).

Comment 115

[David]

I came to this bug report because we have some members on our site saying that they have the problem.

We use the no-cache headers on only a few pages of our site, however the problem appears to be more widespread than just the no-cache headers. We have pages that don't use these headers at all and still do not remember where your vertical position on the page was.

Any ideas on the progress of this bug being resolved? Appears the problem does not occure in earlier versions of Firefox and all other browsers seem to respond correctly.

Comment 116

[Michael Lefevre]

"We have pages that don't use these headers at all and still do not remember where your vertical position on the page was. ... Appears the problem does
not occure in earlier versions of Firefox and all other browsers seem to
respond correctly."

Well, this bug report only refers to pages with the header, and the issue was present in Firefox 1.x and 2.0 as well. Other problems won't be addressed in this bug, so if you can find/file another bug report with details of the different problem, that would help.

"Any ideas on the progress of this bug being resolved?"

There is nobody working on it currently. Asking for progress updates and/or demanding progress just makes this bug report messier and harder to follow as and when someone tries to fix it.

Comment 117

[Timothy Miller]

I'm not sure if this is exactly the same bug, but I'm noticing something like problem with the latest release of Firefox 3.  

Go to "www.osnews.org"
Scroll down
Click an article title
Click back button
Page positions at top, not where it was when I clicked the link.

Comment 118

[Frank Wein [:mcsmurf]]

Timothy: It's the same problem, see http://web-sniffer.net/?url=http%3A%2F%2Fwww.osnews.org%2F&submit=Submit&http=1.1&gzip=yes&type=GET&uak=0. That page uses the no-store header.

Comment 119

[Timothy Miller]

Well, it appears that other browsers are either ignoring the "no-store" tag, or they're tossing all of the page _except_ for the scroll position.  When I look at this page in Safari, and I click Back, it appears to completely reload the page, but then it repositions to where I was last looking.  That would be an acceptable behavior for Firefox, IMHO.  Perhaps the smarter thing to do would be to compare the old and new documents, and if they're sufficiently similar (90% similar in the DOM tree?), then go to the remembered scroll position.

Don't blame this on the site.  Other browsers behave better in the face of no-store.

I think you should raise the priority of this bug, because it causes major usability problems.  If I'm trying to look through a list of items, where I click on a link then Back a lot, having the page lose its position is incredibly disruptive to my concentration.

Comment 120

[Boris Zbarsky [:bzbarsky]]

Look, everyone agrees that the scroll position should be restored in this situation.  The hard part is rewriting the scroll-position-restoration code to make this work...

Comment 121

[Hideo Saito]

Attachment: test patch

Boris, I am sorry for my too late response, I was not able to answer more than you saw the patch. I think that it is better to use current the mechanism of the session history.

I intend that this patch makes to use the session history available, when the page has cache-control, and does not use the session history except the scroll positions.

Comment 122

[Hideo Saito]

Attachment: test patch

This page has cache-control: no-store. If it does not set offline but downs web, Back/Forw causes to miss loading the page.

http://www.tigerroar.com/tigertown/

Comment 123

[Hideo Saito]

Attachment: patch

When the page coming of "Forward" or "Backward" has "cache-control:no-store", the cache is not used.

Comment 124

[Hideo Saito]

Comment on attachment 328639 [details] [diff] [review]
patch

Boris, could you review this patch?

The page is reloaded without using the cache, because |NS_ERROR_NOT_AVAILABLE| is returned at |nsHttpChannel::OpenCacheEntry|. The scroll positions are returned using the session history.

Comment 125

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 328639 [details] [diff] [review]
patch

This looks like it will bypass the HTTP cache (incorrectly) but restore HTML form control state (also incorrectly)...

Comment 126

[Hideo Saito]

Attachment: patch

Boris, could you review this patch again, though it looks like the patch denied before?

This patch intends to scroll to current scrolled position prior to restored scroll position in session history. Also, I think this influences Bug 103279. Without saved state and scrolling the page, it returns to current position.

Comment 128

[Tomas Janirek]

Hi, this bug persists even in Firefox 3.0.3. I'm using F3 since beta versions, but this week I was really considered to return back to Maxthon(IE), because scroll position bug is very very annoying! I'm wondering why it wasn't fixed yet. Anyway, workaround with Restore Scroll Position plugin fortunately works! But this one of the most important and 'must have' plugins is not present in mozilla plugin area. I thing it should be at the top of the first page or F3 should contain this plugin by default or the bug should be just fixed:)
I think that F3 is amazing browser, but these little annoying things (like poor RSS support ....) makes people second thoughts about switching from IE ...

Comment 129

[Frank Wein [:mcsmurf]]

Tomas: Of course this bug still exists, this is why it says "NEW" at the top and not "RESOLVED". And a workaround is not a real fix, the workaround also has its problems.

Comment 130

[Zdenek Vlach]

+1 please solve this issue. This is really very annoying, because manytimes I have to search where I stoped reading and click on the link. Browsing is then very, very slow.

Comment 131

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 331914 [details] [diff] [review]
patch

So...  I'm really sorry it took me this long to get to this pach, but this is not quite correct (certainly the scrollframe changes are bogus, since they save random scrollframe state on the docshell, not just root scrollframe).

And it's not clear to me that the various EndLoads play nice here, or how this plays with anchor scrolling (which already uses the SHEntry scroll position).

Is there a reason not to just add a flags argument to state-saving to indicate what sorts of state to save, and reuse the already-existing-and-tested code that usually restores scroll position, as sugested in comment 30?  I think that would be a lot less fragile and more likely to get fast review and checkin.

Comment 132

[Graeme McCutcheon [:graememcc]]

Attachment: Patch - my attempt

So, despite knowing next to nothing about docshell and session history and layout, I had a pop at this.

Comment 133

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 344699 [details] [diff] [review]
Patch - my attempt

This only saves scroll position for no-store pages.  We should instead always be saving it (both for no-store, and for secure no-cache).

Would you mind changing things around so that we do that?  Should make the patch simpler too...

Comment 134

[Graeme McCutcheon [:graememcc]]

Attachment: Patch v2

Like so?

Comment 135

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 345713 [details] [diff] [review]
Patch v2

>+++ b/docshell/base/nsDocShell.cpp
>@@ -5368,23 +5368,16 @@

For what it's worth, if you can add -p to your diff options that would be very helpful.

>-    // If the session history entry has the saveLayoutState flag set to false,
>-    // then we should not cache the presentation.
>-    PRBool canSaveState;
>-    mOSHE->GetSaveLayoutStateFlag(&canSaveState);
>-    if (canSaveState == PR_FALSE)
>-        return PR_FALSE;

Please leave this block.  Otherwise we'll store no-store pages in bfcache, and on returning to them show all the form data, etc, etc.

>@@ -5671,16 +5664,23 @@
>+        // The session history entry may still have scroll position
>+        // saved in its layoutHistoryEntry that can be restored
>+        nsCOMPtr<nsILayoutHistoryState> layoutHistory;
>+        aSHEntry->GetLayoutHistoryState(getter_AddRefs(layoutHistory));
>+        if (layoutHistory)
>+          SetLayoutHistoryState(layoutHistory);

Why is this needed?  I wouldn't think we want to smack the state from the history entry (which we're about to load and is currently stored in mLSHE) onto mOSHE, which is what that call would do.  I suspect this block can be removed without affecting this fix, but if not I'd really like to know why.

>+++ b/docshell/shistory/src/nsSHEntry.cpp
nsSHEntry::SetLayoutHistoryState(nsILayoutHistoryState* aState)
> {
>-  NS_ENSURE_STATE(mSaveLayoutState || !aState);
>+  NS_ENSURE_STATE(!aState);

That makes it so that mLayoutHistoryState is always null.  That doesn't sound right to me.  Shouldn't the NS_ENSURE_STATE just be removed?

> NS_IMETHODIMP nsSHEntry::SetSaveLayoutStateFlag(PRBool  aFlag)
> {
>   mSaveLayoutState = aFlag;

Do we ever hit this when mLayoutHistoryState is non-null?  If so, we need to do something smart somewhere.  We don't want to drop the state, but perhaps we should fix restoration to only restore the root scrollframe in this case.  This might need changes to the history state itself (the ability to flag it so that only the root scrollframe state can be extracted from it) or something like that.

>+PresShell::CaptureHistoryState(nsILayoutHistoryState** aState,
>-  FrameManager()->CaptureFrameState(rootFrame, historyState);  
>+  if (!aScrollStateOnly)
>+    FrameManager()->CaptureFrameState(rootFrame, historyState);

This will work for frame states, but not for form control state, which is now stored in content nodes.  Those content nodes do their own saving of state as they're removed from the DOM.  Which is why we really need to flag the fact that that information shouldn't be saved/restored in the (somewhat misnamed) layout history state itself.

The simplest approach to fixing form controls might actually be to have a flag in the layout history state that indicates that it's saving root scrollframe state only and to make nsIDocument::GetLayoutHistoryState() return null if that flag is set.

All that said, do we really want to not restore the state of non-root scrollframes?  It seems to me like it would be a good idea to save it and restore it...

>+++ b/layout/forms/nsComboboxControlFrame.cpp
> nsComboboxControlFrame::SaveState(SpecialStateID aStateID,
>                                   nsPresState** aState)
> {
>+  if (aStateID == nsIStatefulFrame::eDocumentScrollState)
>+    return NS_OK;

Why is this change needed?

> nsIsIndexFrame::SaveState(SpecialStateID aStateID, nsPresState** aState)
> {
>+  if (aStateID == nsIStatefulFrame::eDocumentScrollState)
>+    return NS_OK;

Same here.

Comment 136

[Boris Zbarsky [:bzbarsky]]

One other thought is that it would be good to have some tests here; please let me know if you need pointers as far as writing them.  Mochitests should be able to test everything we need, I think.  That would help catch issues like bfcache being enabled for no-store pages, as in that last patch...

Comment 137

[Graeme McCutcheon [:graememcc]]

Attachment: Tests for existing behaviour

Meh. New patch in the works, and should appear soon.

I hadn't correctly worked out when scroll restoration occurs, which led to some incorrect assumptions, which led to lots of wrongess in that patch.

For the moment, here are tests for the existing behaviour, which would be good to get into the tree in any case, and will help with my next iteration of this patch.

Comment 138

[Graeme McCutcheon [:graememcc]]

Attachment: Patch v3

So, this turns out to be far far simpler than I've been making it.

This patch removes all callers of GetShouldSaveLayoutState, but leaves the code in situ - not sure where we would be with removing functions from the nsIDocShell API in a point release.

Comment 139

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 348012 [details] [diff] [review]
Tests for existing behaviour

This looks great.  One nit: please take out the MISSING_EVENTS_TIMEOUT stuff.  Mochitest has its own test timeout that it uses, and we don't want individual tests overriding that one.  When they do, they often start randomly failing on the slower test machines....

Comment 140

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 348117 [details] [diff] [review]
Patch v3

This is looking pretty good!  Just a few nits:

>+++ b/docshell/test/chrome/215405_nocache.html
>+This should be a long page so we can measure scroll restoration.

Can we get here by using 

  <html style="height: 100%">
    <body style="height: 100%">
      <div style="height: 50%">Some text</div>
      <div style="height: 50%">Some text</div>
      <div style="height: 50%">Some text</div>
      <div style="height: 50%">Some text</div>
    </body>
  </html>

?  That should give you content 2x as tall as the viewport.  Adjust as needed?

>+This should be a long page so we can measure scroll restoration. Ideally, we want one of the lines to be quite long to ensure that the document can be scrolled horizontally

Stick |style="width: 300%"| on one of those <div>s above?

Same for the other HTML file.

>+++ b/docshell/test/chrome/bug215405_window.xul
>+      //Save the scroll position for future comparison
>+      scrollX = gBrowser.contentWindow.scrollX;
>+      scrollY = gBrowser.contentWindow.scrollY;

Maybe do some isnot() compares here to make sure the scrollBy call actually changed the scroll position?

>+++ b/layout/base/nsILayoutHistoryState.h
>+   NS_IMETHOD SetScrollPositionOnly(const PRBool aFlag) = 0;

Put this at the end of the interface, to be safe?

>+++ b/layout/base/nsLayoutHistoryState.cpp
> nsLayoutHistoryState::AddState(const nsCString& aStateKey, nsPresState* aState)
> {
>+  if (!mScrollPositionOnly || aState->HasScrollState())
>+    return mStates.Put(aStateKey, aState) ? NS_OK : NS_ERROR_OUT_OF_MEMORY;
>+
>+  // Silently fail if saving non-scroll states disallowed.
>+  // Callers would assert on failure.
>+  return NS_OK;

That doesn't seem right.  The caller assumes a success return to mean that the nsLayoutHistoryState took ownership of the nsPresState.  So this will leak in the silent failure case.

Further, some callers AddState first and then fill the state, (the HTML form controls, in particular).  They don't put any scroll state in here, but let's not depend on that.

> nsLayoutHistoryState::GetState(const nsCString& aKey, nsPresState** aState)
>+  PRBool entryExists = mStates.Get(aKey, aState);
>+  if (entryExists && mScrollPositionOnly && !(*aState)->HasScrollState()) {
>+    // A non-scroll state exists, but it is incorrect to restore it
>+    aState = nsnull;

So maybe what would be better is to have a ClearNonScrollState() method on nsPresState, and to do:

  if (entryExists && mScrollPositionOnly) {
    (*aState)->ClearNonScrollState();
  }

and then just return it.  This does mean keeping around state in memory that we'll never restore, but I think that's OK.  This will certainly handle cases (hypothetical so far, granted) when someone stores both scroll and non-scroll state in the same nsPresState better...

Alternately, we could do something else to the callers to not leak on AddState, but that involves more surgery on other code, I think.

>+nsPresState::HasScrollState()

Not needed if we do the above, but if we keep it, it should just look like:

  return mScrollState != nsnull;

Thank you again for working on this, and sorry my reviews are being so slow.  :(  I should have read the code changes first and then sorted out the tests... :(

Comment 141

[Graeme McCutcheon [:graememcc]]

Attachment: Tests for existing behaviour v2

Nit addressed.

Carrying forward r+sr=bz.

Comment 142

[Graeme McCutcheon [:graememcc]]

Attachment: Patch v4

Apologies for delay. Was moving house...

> So maybe what would be better is to have a ClearNonScrollState() method on
> nsPresState, and to do:
>
>  if (entryExists && mScrollPositionOnly) {
>    (*aState)->ClearNonScrollState();
>  }
>
> and then just return it.

I went with this. Thanks!

However, this approach exposed the fact that some callers of nsPresState::GetStateProperty weren't checking the return value to determine if the property they had tried to read existed. 

Further, it also highlighted that nsPresState::GetStatePropertyAsSupports always returned NS_OK, and never gave its callers any indication whether the property existed or not, which could potentially lead to dereferencing dangling pointers.

Comment 143

[Boris Zbarsky [:bzbarsky]]

The updated test file is missing the makefile change, no?

Comment 144

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 351199 [details] [diff] [review]
Patch v4

r+sr=bzbarsky.  This looks great.

Comment 145

[Boris Zbarsky [:bzbarsky]]

Attachment: Existing behavior tests with makefile change, as mq diff

Comment 146

[Graeme McCutcheon [:graememcc]]

Boris pushed the existing behaviour tests as http://hg.mozilla.org/mozilla-central/rev/f290559c01d1 and the patch for this bug as http://hg.mozilla.org/mozilla-central/rev/4047bed537d6.

However, the tests interacted badly on linux with those from bug 396519, so the tests were disabled for now, though the fix remains in the tree.

Boris, am I right in thinking then that your suspicion is that the combination of my new tests hitting bfcache and the amount of RAM on the linux tinderbox machines is causing entries to be expired from the session history at a different rate from what the 396519 tests expect?

To make that test less brittle, before hitting the iterator could we do

var history = gBrowser.webNavigation.sessionHistory;
var res = history.purgeHistory(history.count);
isnot(res, Components.results.NS_SUCCESS_LOSS_OF_INSIGNIFICANT_DATA, 
      "failed to purge session history");

to ensure the session history is in a known state?

Comment 147

[Graeme McCutcheon [:graememcc]]

s/purgeHistory/PurgeHistory

Comment 148

[Boris Zbarsky [:bzbarsky]]

OK, so I did a bit more digging today.  What's going on is that the tinderbox has 512 megs of RAM.  We evict things from bfcache based on a per-tab limit (3 by default) and a global limit (ram-dependent; 5 at 512 megs).  When doing the latter, we evict the entries furthest from the current entry over all lie histories.

The problem is that closing a window doesn't destroy the history immediately.  It just drops the pointer to it from the docshell, but the object doesn't go away until GC happens.  The test that started failing expects entries 3 from the current one to still be in bfcache, but if the global limit is 5 and we have at least 3 other entries already in histories that just haven't been GCed yet, then this won't happen.

Purging history at the beginning of that test won't help, since it's the other histories that need to be purged.

I just pushed http://hg.mozilla.org/mozilla-central/rev/f90c51ed3cd2 to reenable these tests, with history purging in the new tests to work around the problem, and filed bug 467960 on the remaining issue.

This bug is fixed.  Graeme, thanks again for picking this up and especially for writing all those tests!

Comment 149

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Verified with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2a1pre) Gecko/20081206 Minefield/3.2a1pre and the identical build on OS X by navigating on following websites where the issue is still visible in Firefox 3.1:

http://tweakers.net/
http://geizhals.at/eu/
http://www.tigerroar.com/tigertown/
http://nytimes.com/
http://www.msn.com/

Boris, is it something we could consider for Firefox 3.1? It will give a better user experience on many different websites.

Comment 150

[Boris Zbarsky [:bzbarsky]]

Given the number of blockers for 3.1, I don't think we should worry about forcing this in there too, honestly.  It's been open for long enough that another 6 months won't kill anyone.

My opinion, of course.

Comment 151

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I agree, we are way past feature freeze and this change simply seems to big. We have to draw the line somewhere and unfortunately I think this one falls on the wrong side.

Comment 152

[Boris Zbarsky [:bzbarsky]]

More precisely, it's a small change in very hairy and sensitive, and worse yet security-related, code.  Graeme's tests are a big plus in terms of peace of mind, but not quite enough for me to want to land on branch without a lot of baking.  :(

Comment 154

[Graeme McCutcheon [:graememcc]]

The tests for this bug and the test I added in here for bug 112564 have been occasionally failing on the windows unit test box since 4th February.

As far as I can see, the first failure is http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1233765108.1233770911.11281.gz, corresponding to changeset 5a924dc19720, although the changeset seems entirely unrelated.

I suspect the failures are being treated as random noise, but the tests had been passing for 2 months prior to that...

Comment 155

[Dave Townsend [:mossop]]

(In reply to comment #154)
> The tests for this bug and the test I added in here for bug 112564 have been
> occasionally failing on the windows unit test box since 4th February.
> 
> As far as I can see, the first failure is
> http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1233765108.1233770911.11281.gz,
> corresponding to changeset 5a924dc19720, although the changeset seems entirely
> unrelated.
> 
> I suspect the failures are being treated as random noise, but the tests had
> been passing for 2 months prior to that...

Filed bug 478241 on this.

Comment 156

[Dão Gottwald [:dao]]

Comment on attachment 351199 [details] [diff] [review]
Patch v4

This baked quite long now.

Comment 157

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 351199 [details] [diff] [review]
Patch v4

a191=beltzner

Comment 158

[Boris Zbarsky [:bzbarsky]]

To make it clear, the revs that need to land are:

http://hg.mozilla.org/mozilla-central/rev/f290559c01d1
http://hg.mozilla.org/mozilla-central/rev/4047bed537d6
http://hg.mozilla.org/mozilla-central/rev/f90c51ed3cd2

Comment 159

[Serge Gautherie (:sgautherie)]

(In reply to comment #158)
> To make it clear, the revs that need to land are:
> 
> http://hg.mozilla.org/mozilla-central/rev/f290559c01d1

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/7c0da7cfd01e

After fixing context for
{
patching file docshell/test/chrome/Makefile.in
Hunk #1 FAILED at 37
1 out of 1 hunks FAILED
}


> http://hg.mozilla.org/mozilla-central/rev/4047bed537d6

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/19b20050f6c6

After fixing context for
{
patching file docshell/test/chrome/Makefile.in
Hunk #1 FAILED at 42
1 out of 1 hunks FAILED
patching file layout/base/nsDocumentViewer.cpp
Hunk #1 FAILED at 1961
1 out of 1 hunks FAILED
}


> http://hg.mozilla.org/mozilla-central/rev/f90c51ed3cd2

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/fbb035e530bf

After ignoring
{
patching file docshell/test/chrome/Makefile.in
Hunk #1 FAILED at 49
1 out of 1 hunks FAILED
}


In the future, please prepare (branch) patches that apply cleanly.

Comment 160

[Boris Zbarsky [:bzbarsky]]

> After ignoring

Er.. but that negates the whole purpose of that patch, which is to enable the tests.  I'll fix, and I would have landed the patches myself later tonight if you didn't want to land them, fwiw...

Comment 161

[Boris Zbarsky [:bzbarsky]]

> Er.. but that negates the whole purpose of that patch

Ah, nevermind; the changeset that disabled the tests didn't land on branch.  So all is good here; thanks for landing stuff, Serge!

Comment 162

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Verified fixed on 1.9.1 with builds on OS X and Windows:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre) Gecko/20090322 Shiretoko/3.5b4pre ID:20090322030800

Comment 163

[Kjetil Trøan]

Well, not to spoil your joy here, but my firefox still doesn't remember the scroll position on most pages, including google.

Mac OS 10.5.8, Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.3) Gecko/20090824 Firefox/3.5.3

Comment 164

[Boris Zbarsky [:bzbarsky]]

Sounds like a different issue from this bug.  Please file a separate bug, once you've made sure that the problem still appears in safe mode (and ideally in a clean profile).  If it doesn't, you might want to ask around at http://support.mozilla.com/en-US/kb/ to see what settings or extensions might be causing it.

Comment 171

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Restricting comments for a year long fixed bug that recently only got spam comments.