David Mårtensson Reporter
	Description • 21 years ago

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

When trying to view source on a page set to no cache, mozilla tries to reload page.

Normaly this should not be to big a problem but, IIS sets no cache on ASP pages
with an error.

This makes it much harder to debug since sometimes the next fetch does not
reproduce the error.

Reproducible: Always

Steps to Reproduce:
1.Try the url
2.Reload the url
3.View source
4.Works fine
5.Enter something in the textfield and submit
6.View source

Actual Results:  
It tries to repost the form

Expected Results:  
It should Not repost the form since that could change the state of the server.

I do not know if this problem existed before but I have not seen it before 1.4
final and since I do all my development on work in mozilla I think I should have
seen it before, but I am not 100% certain.

This will probably only affect developes and I think I can do a workaround for
my pages but still, it annoying to have to rewrite a page to debug it.

I do not whant to switch to IE because I still have to reverify everything in
mozilla.

	Frank Wein [:mcsmurf]
	Comment 1 • 21 years ago

*** This bug has been marked as a duplicate of 166786 ***

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 2 • 21 years ago

Reopening; this is a separate problem.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 3 • 21 years ago

Over to darin; the problem here is that the page in question is a 500 error
page, and we don't cache error pages.  Per discussion, we should probably cache
these and expire them immediately, so that back/forward/viewsource work but
reload refetches the right thing.

David, could you please keep the test page up for a bit so we can test possible
fixes?

	Darin Fisher
	Comment 4 • 21 years ago

this shouldn't be very difficult to fix.  taking for 1.5 beta.

	David Mårtensson Reporter
	Comment 5 • 21 years ago

Sure, i'll leave the testcase on the server.

If you need any changes to it just let me know.

	Darin Fisher
	Comment 6 • 21 years ago

i'll try for 1.6 beta, but no promises.

	Hixie (not reading bugmail)
	Comment 7 • 21 years ago

bz says that fixing this should fix a similar problem with HTTP auth pages.

STEPS TO REPRODUCE
   1. Load http://www.sbscpressinfo.org/
   2. Cancel the prompt.
   3. View source

ACTUAL RESULTS
   You are prompted again.

EXPECTED RESULTS
   The source with no prompt.

	Darin Fisher
	Comment 8 • 20 years ago

no time to work on this now... the solution, i think, is to cache error pages
with zero freshness lifetime.  as a result, they will be accessible from the
cache when loading the page via back/forward, viewsource, or saveas.  however,
normal browsing will always hit the server.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 10 • 20 years ago

Comment on attachment 153068 [details] [diff] [review]
Like so?

darin, what do you think?

	Darin Fisher
	Comment 11 • 20 years ago

Comment on attachment 153068 [details] [diff] [review]
Like so?

actually, i think we want to use an expiration time of 0, which implies that
the document is already expired (0 means the Epoch, 1970).

see nsHttpChannel::UpdateExpirationTime() where we set 0 as the expirationTime
in cases where 'Cache-control: no-cache' is returned from the server, for
example.

i'm also a bit concerned about logic in ProcessNormal that calls
InitCacheEntry.  it calls UpdateExpirationTime.  perhaps the solution here is
to coerce nsHttpResponseHead into returning PR_TRUE for MustValidate().  that
may be better.	in fact, you could probably just use the response code for
that.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 13 • 20 years ago

Er, make that !=2 && !=3 in the patch, I think.  We do want to allow caching of
redirects.

	Darin Fisher
	Comment 14 • 20 years ago

yeah, something like that.  we may want to synchronize this filter with the
switch-case in nsHttpChannel::ProcessResponse() to be 100% sure that it doesn't
have unexpected side-effects.  like what about the 2xx responses not covered in
the switch-case.  i'll need to think about this some more, and do some thorough
testing.  what sort of testing have you done?

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 15 • 20 years ago

Synchronizing makes sense.  I can do that.

I've tested that it fixes this bug, but I don't have any error pages on hand
that would allow themselves to be cached in a detectable way...

	Darin Fisher
	Comment 16 • 20 years ago

Comment on attachment 153080 [details] [diff] [review]
So more like this?

In fact, this is incorrect since some redirects may be cached.	Even a 302 can
be cached if the server sends a Cache-control header that specifies a max-age
or some equivalent header.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 18 • 20 years ago

Comment on attachment 155398 [details] [diff] [review]
Address comments

Darin, how's this look?

	David Mårtensson Reporter
	Comment 19 • 20 years ago

My testpage from when I reported the bug still exists, does it require any
changes to test the patch, please just tell me what and I will try to update the
testpage or create a new one.

	Darin Fisher
	Comment 20 • 20 years ago

Comment on attachment 155398 [details] [diff] [review]
Address comments

does the compiler optimize away all of the |return PR_TRUE| cases?

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 21 • 20 years ago

David, your testpage is one of the things I tested the patch on.  It doesn't
need any changes that I can see.

Darin, the compiler does optimize all that away.

Fix checked in.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 22 • 20 years ago

Er.. I did check this in on August 9.  So this is long fixed.