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

VERIFIED FIXED in mozilla1.9.2a1

Status

()

Core
Document Navigation
--
major
VERIFIED FIXED
14 years ago
7 years ago

People

(Reporter: Julian Gall, Assigned: graememcc)

Tracking

(Blocks: 1 bug, {verified1.9.1})

Trunk
mozilla1.9.2a1
verified1.9.1
Points:
---
Bug Flags:
blocking-aviary1.0 -
blocking1.9.1 -
wanted1.9.1 -
blocking1.9 -
wanted1.9 +
blocking1.8.1 -
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fixed1.9.1b4])

Attachments

(2 attachments, 11 obsolete attachments)

22.29 KB, patch
beltzner
: approval1.9.1+
Details | Diff | Splinter Review
16.36 KB, patch
Details | Diff | Splinter Review
(Reporter)

Description

14 years ago
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

14 years ago
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

14 years ago
Here you go. Bug 204364. Fixed in 1.4b.

Comment 3

14 years ago
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/
(Reporter)

Comment 4

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

Comment 5

14 years ago
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 ?

Updated

14 years ago
Summary: Back button does not position page to where it was when sending link was clicked → scroll position not remembered (because of pragma: no-cache?)

Comment 6

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

Comment 7

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

Comment 8

14 years ago
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

14 years ago
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

14 years ago
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

14 years ago
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

14 years ago
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

14 years ago
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

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

Comment 15

14 years ago
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

14 years ago
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

14 years ago
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

14 years ago
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.
Summary: scroll position not remembered (because of pragma: no-cache?) → scroll position not remembered on pages with cache-control: no-store

Comment 19

14 years ago
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

14 years ago
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

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

Comment 22

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

Comment 23

14 years ago
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

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

Comment 25

13 years ago
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

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

Comment 27

13 years ago
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)
*** Bug 244884 has been marked as a duplicate of this bug. ***
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?
Keywords: helpwanted
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)....
OS: Windows XP → All
Hardware: PC → All
*** Bug 237859 has been marked as a duplicate of this bug. ***

Updated

13 years ago
Depends on: 251784

Comment 32

13 years ago
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.
Blocks: 251784
No longer depends on: 251784

Comment 33

13 years ago
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 ;-(

Updated

13 years ago
Flags: blocking-aviary1.0?

Comment 34

13 years ago
(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
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

13 years ago
> 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

13 years ago
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.
*** Bug 256440 has been marked as a duplicate of this bug. ***
Blocks: 216376

Comment 39

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

Comment 40

13 years ago
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

13 years ago
Could the maintainer please change
Keywords: helpwanted
to
Keywords: conversion

as this fits much better?

Comment 42

13 years ago
*** Bug 261221 has been marked as a duplicate of this bug. ***
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.
Severity: normal → major

Comment 44

13 years ago
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

13 years ago
(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

13 years ago
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.
Assignee: radha → danm.moz
Flags: blocking-aviary1.0? → blocking-aviary1.0+

Comment 47

13 years ago
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. 
Flags: blocking-aviary1.0+ → blocking-aviary1.0-
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 ;-)
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?
The previous comment was most likely intended to be on bug 255372.
(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

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

Comment 53

13 years ago
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.
Blocks: 258133
*** Bug 211242 has been marked as a duplicate of this bug. ***
Ben, either of the solutions you propose in comment 48 sounds fine; the former
may actually be easier to implement.

Updated

12 years ago
Assignee: danm.moz → nobody
*** Bug 238203 has been marked as a duplicate of this bug. ***

Comment 57

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

Updated

12 years ago
Flags: blocking1.9a1?
*** Bug 306200 has been marked as a duplicate of this bug. ***

Comment 60

12 years ago
*** Bug 299789 has been marked as a duplicate of this bug. ***
*** Bug 297397 has been marked as a duplicate of this bug. ***

Comment 62

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

Comment 63

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

Comment 64

12 years ago
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.
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

12 years ago
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

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

Updated

12 years ago
Flags: blocking-aviary2?
*** Bug 313528 has been marked as a duplicate of this bug. ***

Comment 69

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

Comment 70

12 years ago
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.

Updated

12 years ago
Flags: blocking-aviary2? → blocking1.8.1?

Comment 71

12 years ago
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

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

Comment 73

11 years ago
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

11 years ago
(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

11 years ago
(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

11 years ago
Not going to block 1.8.1 for this.
Flags: blocking1.8.1? → blocking1.8.1-
*** Bug 344680 has been marked as a duplicate of this bug. ***

Updated

11 years ago
Flags: blocking1.9a1? → blocking1.9-
Whiteboard: [wanted-1.9]
*** Bug 348193 has been marked as a duplicate of this bug. ***

Comment 79

11 years ago
(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

11 years ago
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

11 years ago
(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).
Summary: scroll position not remembered on pages with cache-control: no-store → scroll position not remembered on pages with cache-control: no-store http header

Comment 82

11 years ago
comment#81

I thought we already established that...

Comment 83

11 years ago
(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

11 years ago
(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).

(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

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

Comment 87

11 years ago
(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

11 years ago
(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

11 years ago
(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"
Duplicate of this bug: 367705
Duplicate of this bug: 374669
(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

10 years ago
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

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

Comment 95

10 years ago
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

10 years ago
Created attachment 260056 [details] [diff] [review]
patch

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

Comment 97

10 years ago
Created attachment 260091 [details] [diff] [review]
patch
Attachment #260056 - Attachment is obsolete: true

Comment 98

10 years ago
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.
Attachment #260091 - Flags: review?(dbaron)

Comment 99

10 years ago
any relation?

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

Comment 100

10 years ago
no
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.
Attachment #260091 - Flags: superreview?(bzbarsky)
Attachment #260091 - Flags: review?(dbaron)
Attachment #260091 - Flags: review?(bzbarsky)
So before I start digging into the mOSHE/mLSHE mess, can you explain what you think the patch is doing?
Comment on attachment 260091 [details] [diff] [review]
patch

r- pending reply to comment 102.
Attachment #260091 - Flags: superreview?(bzbarsky)
Attachment #260091 - Flags: superreview-
Attachment #260091 - Flags: review?(bzbarsky)
Attachment #260091 - Flags: review-
Duplicate of this bug: 394524

Comment 105

10 years ago
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

10 years ago
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....
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

10 years ago
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.
Yes, we know this is a problem.  Please see https://bugzilla.mozilla.org/etiquette.html ?

Comment 110

10 years ago
Thanks n808 for letting us know about the extension.  I just installed it & hope it will help.
Flags: wanted1.9+
Whiteboard: [wanted-1.9]

Comment 111

9 years ago
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/

Updated

9 years ago
Duplicate of this bug: 427078

Comment 113

9 years ago
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.
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

9 years ago
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

9 years ago
"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.

Updated

9 years ago
No longer blocks: 258133

Comment 117

9 years ago
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.
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

9 years ago
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.
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

9 years ago
Created attachment 327307 [details] [diff] [review]
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.
Attachment #260091 - Attachment is obsolete: true

Comment 122

9 years ago
Created attachment 328142 [details] [diff] [review]
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/
Attachment #327307 - Attachment is obsolete: true

Updated

9 years ago
Flags: wanted1.9.1?
Flags: blocking1.9.1?

Comment 123

9 years ago
Created attachment 328639 [details] [diff] [review]
patch

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

Comment 124

9 years ago
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.
Attachment #328639 - Flags: review?(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)...
Attachment #328639 - Flags: review?(bzbarsky) → review-

Updated

9 years ago
Component: History: Session → Document Navigation
QA Contact: chrispetersen → docshell

Comment 126

9 years ago
Created attachment 331914 [details] [diff] [review]
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.
Attachment #331914 - Flags: review?(bzbarsky)

Updated

9 years ago
Attachment #328639 - Attachment is obsolete: true
Duplicate of this bug: 216376

Comment 128

9 years ago
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 ...
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

9 years ago
+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 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.
Attachment #331914 - Flags: review?(bzbarsky) → review-
Whiteboard: [good first bug]

Updated

9 years ago
No longer blocks: 216376
(Assignee)

Comment 132

9 years ago
Created attachment 344699 [details] [diff] [review]
Patch - my attempt

So, despite knowing next to nothing about docshell and session history and layout, I had a pop at this.
Attachment #344699 - Flags: superreview?(bzbarsky)
Attachment #344699 - Flags: review?(bzbarsky)
Flags: wanted1.9.1?
Flags: wanted1.9.1-
Flags: blocking1.9.1?
Flags: blocking1.9.1-
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...
(Assignee)

Comment 134

9 years ago
Created attachment 345713 [details] [diff] [review]
Patch v2

Like so?
Attachment #344699 - Attachment is obsolete: true
Attachment #345713 - Flags: superreview?(bzbarsky)
Attachment #345713 - Flags: review?(bzbarsky)
Attachment #344699 - Flags: superreview?(bzbarsky)
Attachment #344699 - Flags: review?(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.
Attachment #345713 - Flags: superreview?(bzbarsky)
Attachment #345713 - Flags: superreview-
Attachment #345713 - Flags: review?(bzbarsky)
Attachment #345713 - Flags: review-
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...
(Assignee)

Comment 137

9 years ago
Created attachment 348012 [details] [diff] [review]
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.
Attachment #348012 - Flags: superreview?(bzbarsky)
Attachment #348012 - Flags: review?(bzbarsky)
(Assignee)

Comment 138

9 years ago
Created attachment 348117 [details] [diff] [review]
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.
Attachment #345713 - Attachment is obsolete: true
Attachment #348117 - Flags: superreview?(bzbarsky)
Attachment #348117 - Flags: review?(bzbarsky)

Updated

9 years ago
Flags: wanted1.9.2?
Keywords: helpwanted
Whiteboard: [good first bug] → [good first bug][has patch][needs review]
Attachment #348012 - Flags: superreview?(bzbarsky)
Attachment #348012 - Flags: superreview+
Attachment #348012 - Flags: review?(bzbarsky)
Attachment #348012 - Flags: review+
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 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... :(
Attachment #348117 - Flags: superreview?(bzbarsky)
Attachment #348117 - Flags: superreview-
Attachment #348117 - Flags: review?(bzbarsky)
Attachment #348117 - Flags: review-
(Assignee)

Comment 141

9 years ago
Created attachment 351197 [details] [diff] [review]
Tests for existing behaviour v2

Nit addressed.

Carrying forward r+sr=bz.
Attachment #348012 - Attachment is obsolete: true
Attachment #351197 - Flags: superreview+
Attachment #351197 - Flags: review+
(Assignee)

Comment 142

9 years ago
Created attachment 351199 [details] [diff] [review]
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.
Attachment #348117 - Attachment is obsolete: true
Attachment #351199 - Flags: superreview?(bzbarsky)
Attachment #351199 - Flags: review?(bzbarsky)
The updated test file is missing the makefile change, no?
Comment on attachment 351199 [details] [diff] [review]
Patch v4

r+sr=bzbarsky.  This looks great.
Attachment #351199 - Flags: superreview?(bzbarsky)
Attachment #351199 - Flags: superreview+
Attachment #351199 - Flags: review?(bzbarsky)
Attachment #351199 - Flags: review+
Assignee: nobody → graememcc_firefox
Created attachment 351206 [details] [diff] [review]
Existing behavior tests with makefile change, as mq diff
Attachment #351197 - Attachment is obsolete: true
Attachment #331914 - Attachment is obsolete: true
(Assignee)

Comment 146

9 years ago
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?
(Assignee)

Comment 147

9 years ago
s/purgeHistory/PurgeHistory
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!
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Whiteboard: [good first bug][has patch][needs review] → [good first bug]
(Assignee)

Updated

9 years ago
Flags: in-testsuite+
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.
Status: RESOLVED → VERIFIED
Target Milestone: --- → mozilla1.9.2a1
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.
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.
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.  :(

Updated

9 years ago
Duplicate of this bug: 448875
(Assignee)

Comment 154

8 years ago
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...
(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 on attachment 351199 [details] [diff] [review]
Patch v4

This baked quite long now.
Attachment #351199 - Flags: approval1.9.1?
Attachment #351199 - Flags: approval1.9.1? → approval1.9.1+
Comment on attachment 351199 [details] [diff] [review]
Patch v4

a191=beltzner
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
Keywords: checkin-needed
Whiteboard: [good first bug] → See comment 158 for what to check in on 1.9.1
(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.
Flags: wanted1.9.2?
Keywords: checkin-needed → fixed1.9.1
Whiteboard: See comment 158 for what to check in on 1.9.1 → [fixed1.9.1b4]
> 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...
> 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!
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
Keywords: fixed1.9.1 → verified1.9.1

Comment 163

8 years ago
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
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.

Updated

7 years ago
You need to log in before you can comment on or make changes to this bug.