289301 - [FIXr]After BACK across POSTDATA View Source shows old page, not current

Status: VERIFIED FIXED

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

Description

[Hugh.Stewart]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.7.6) Gecko/20050226 Firefox/1.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.7.6) Gecko/20050226 Firefox/1.0.1

I have a searchable database and enter a query, which displays a result
page.  If I go to some other page (HOME or follow a link) and then BACK I get a
pop-up asking if I want to resubmit my POSTDATA, I OK that and get my query
results page again.

If I then display View Source it shows me the pre-backing page, not the
currently displayed page.

Reproducible: Always

Steps to Reproduce:
1. go to (eg) http://www.cambridgefolk.org.uk/dance_index/dance_index.html
2. enter (eg) Fred into the first line of the form 
3. hit enter or click on find by title
4. admire results.
5. pick HOME or follow some link on the results page
6. pick BACK
7. Get the page you are trying to view contains POSTDATA that has expired from
cache etc pop-up
8. pick OK
9. Admire the results page again
10. view->page source
11. observe that it shows the source for the page you have just backed out of

Actual Results:  
wrong page source displayed

Expected Results:  
view page source should showe the source of the currently-displayed page

Comment 1

[Gabriel Chadwick]

I confirm this using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2)
Gecko/20050406 Firefox/1.0+

Comment 2

[Jason Barnabe (np)]

The site is set up somewhat unusually. The query page POSTs to dance_index.php
to get the query results, and clicking on a result GETs dance_index.php for the
detail.

My WAG is that the code doesn't realize that these are in fact two different
pages and keeps the descriptor from the GET instead of overwriting it with the
descriptor for the POST.

Boris, feel like shedding some light on this?

Comment 3

[Boris Zbarsky [:bzbarsky]]

No, something else wacky is going on.  Session history is just confused -- note
what the labels of the pages in the back/forward dropdowns are as you go through
the steps to reproduce (especially after you hit "back").

Comment 4

[Boris Zbarsky [:bzbarsky]]

So the real problem here is that nsWebShell::EndPageLoad calls
nsDocShell::EndPageLoad up front, even if the load failed.  This, amongst other
things, clears mLSHE.  So now we're sitting there with a null mLSHE and the
mOSHE for the page we're coming from!  Then when we try to do a force-reload, we
don't update session history, so we still have the wrong mOSHE.

Quick hack would be to grab a ref to the SHE before calling nsDocShell::EndLoad
and use it, but is it really right for EndLoad to just blithely proceed even
though the status we got is an error?

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed patch

The comments should describe what's going on.  The webshell hunk is to make us
not load the source for the page we left, while the docshell hunk is to make us
load the right source from the cache (without that cache key game viewing
source after going back to our POST result prompts for a repost).

Comment 6

[Boris Zbarsky [:bzbarsky]]

Note that while I'd dearly love to simplify this code in general, that seems
like more of a 1.8 or 1.10 task...

Comment 7

[Darin Fisher]

Comment on attachment 180558 [details] [diff] [review]
Proposed patch

This seems ok to me.  sr=darin

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 180558 [details] [diff] [review]
Proposed patch

as discussed on irc:

webshell:
+	     NS_WARN_IF_FALSE(loadingSHE,
+			      "Should have loading session history entry! "
+			      "How did we manage not to?");

What if session history is disabled?

docshell:
+	 else if (!updateHistory && mOSHE) // Do we need to null-check mOSHE?

updateHistory seems to be always false here

as for the comment, again, what if shistory is disabled?

seems ok otherwise, I think

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch updated to comments

Requesting 1.8b approval.  This patch shouldn't really affect any cases that
aren't already broken, and helps keep docshell from being confused about which
page it has loaded right now....

Comment 10

[Asa Dotzler [:asa]]

Comment on attachment 181104 [details] [diff] [review]
Patch updated to comments

a=asa

Comment 11

[Boris Zbarsky [:bzbarsky]]

Fixed.

Comment 12

[Jason Barnabe (np)]

Verified. Hugh, the fix will be included in Firefox 1.1.