114982 - HTTP 300 page screws up session history

Status: VERIFIED FIXED

(Core :: DOM: Navigation, defect, P2)

Description

[Jeremy M. Dolan]

http://metro.turnpike.net/G/GoatBoy/bill.html

The above URL gives a HTTP 300 responce. Click one of the choices (which are all
404s). When that page loads, click the down arrow by back. Last page correctly
shows as '300 multile choices'. Now click it, or click back itself. Nothing
happens. Click back again, and again, and again, nothing happens.

Skip two back. Click forward. Nothing happens.

If it doesn't work from the above link, it's the second other site at:
http://metro.turnpike.net/G/GoatBoy/bill.html

Comment 1

[Joe Francis]

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

Comment 2

[Jeremy M. Dolan]

Hrm, Linux 2002020809, this works somewhat now.

I Press back TWICE, and then it goes back to the 300 page. Not perfect, but much
better.

Comment 3

[Jaime Rodriguez, Jr.]

Resolving as a dupe of bug 97470. Pls implement this change in te fix for 97470.

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

Comment 4

[Jeremy M. Dolan]

Cripes, what happened to the whole 'seperate bug' thing. Adding a note for
myself on the whiteboard before this gets lost. Please don't V until the fix is
actually verified with this case.

Comment 5

[Alec Flett]

this is not a dupe of the global history bug! reopening.
Jaime: note the "History: Session" vs. "History: Global"

Comment 6

[Radha on family leave (not reading bugmail)]

cc'ing darin.  This problem does not exist in the 9.4 branch. Here's what's
going on:

1) Visit the url above,
2) The multiple choices page comes up.
3) Click on one of the choices
4) Click back

At step 4,  session history properly initiates the load of previous page(the
multiple choice page). However, uriLoader's onStartRequest() receives a error
status code from the request, therefore the laod is aborted.  When you click
back again, the above process repeats, but this time the the request does not
have a error status code. So the loading continues and the previous page is loaded. 

I think a recent change in nsHTTPChannel could have caused thsi regression.

Darin can comment more on this. 

Comment 7

[Darin Fisher]

i'll investigate this today... sorry for not responding sooner.

Comment 8

[Darin Fisher]

yup, this is definitely a HTTP bug.  we've likely had this bug for a long time.

the problem is that we don't cache the page for a 300 redirect.  i didn't
realize at the time, that a 300 redirect doesn't have a Location header. 
instead it always shows a page.  for redirects we don't cache the page, because
we assume redirects will always have a Location header.

vinay actually has a patch for another bug, which will also fix this.

-> me

Comment 9

[Darin Fisher]

however, vinay's patch (for bug 127348) would fix this by blowing away the cache
entry for the 300 redirect.  instead, we should cache it as a normal page load
(i.e., treat it like a 200 OK).

attaching a patch to do that...

Comment 10

[Darin Fisher]

actually, this should wait until vinay's patch lands, since it'll conflict.

here's the plan:

1- if we receive a redirect and there isn't a Location header, then treat the
response like a 200 (i.e., just call ProcessNormal).  this will ensure that the
message body is cached.

2- if we receive a redirect and there is a Location header, always use the
Location header.  RFC 2616 says that servers SHOULD send a Location header w/ a
300 redirect to indicate the preferred new URL, and it says that user-agents MAY
automatically select this URL and redirect, so we'll do that.

vinay: can you write up this patch?

Comment 11

[Vinay Badami]

Attachment: allow caching to happen if ProcessRedirection fails

Comment 12

[Darin Fisher]

Comment on attachment 75161 [details] [diff] [review]
allow caching to happen if ProcessRedirection fails

sr=darin w/ one nit: the style of this file is to leave off the braces when
there is only one line under a conditional.  please stick to that style.  thx!

Comment 13

[Vinay Badami]

Rick
Can u please review this one ?

Comment 14

[rpotts (gone)]

Comment on attachment 75161 [details] [diff] [review]
allow caching to happen if ProcessRedirection fails

r=[e-mail]

Comment 15

[Asa Dotzler [:asa]]

Comment on attachment 75161 [details] [diff] [review]
allow caching to happen if ProcessRedirection fails

a=asa (on behalf of drivers) for checkin to the 1.0 branch

Comment 16

[Vinay Badami]

Fixed on both the tip and Moz10branch

D:\mozilla\netwerk\protocol\http\src>cvs commit nsHttpChannel.cpp
Checking in nsHttpChannel.cpp;
/cvsroot/mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp,v  <-- 
nsHttpChannel.cpp
new revision: 1.108.2.3; previous revision: 1.108.2.2
done
D:\mozilla\netwerk\protocol\http\src>cvs commit
cvs commit: Examining .
? patchRes
? bak
Checking in nsHttpChannel.cpp;
/cvsroot/mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp,v  <--  nsHttpChann
new revision: 1.110; previous revision: 1.109
done

Comment 17

[Asa Dotzler [:asa]]

adding fixed1.0.0 keyword (branch resolution). This bug has comments saying it
was fixed on the 1.0 branch and a bonsai checkin comment that agrees. To verify
the bug has been fixed on the 1.0 branch please replace the fixed1.0.0 keyword
with verified1.0.0.

Comment 18

[sairuh (rarely reading bugmail)]

vrfy'd fixed with 2003.02.19 comm trunk on linux rh8.0.