227672 - [FIXr]Repost on back/forward confuses session history

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Boris Zbarsky [:bzbarsky]]

STEPS TO REPRODUCE:

1) Load attached testcase
2) Click the "submit" button
3) Click the "back" button in the navigation toolbar
4) Click the "forward" button in the navigation toolbar. OK the repost dialog.
5) Click the "back" button in the navigation toolbar

Expected results: go back to the testcase page

Actual results: get a postdata alert and are not able to go back

Comment 1

[Boris Zbarsky [:bzbarsky]]

This seems to be a regression from the patch in bug 144301...

I tried undoing this change in that bug:

@@ -5412,9 +5408,13 @@
     // Determine if this type of load should update history.    
     if (aLoadType == LOAD_BYPASS_HISTORY ||
          aLoadType & LOAD_CMD_HISTORY ||
-         aLoadType & LOAD_CMD_RELOAD)         
+         aLoadType == LOAD_RELOAD_NORMAL)         
         updateHistory = PR_FALSE;

and that fixed this problem, without regressing that bug as far as I can tell...
I'm not sure why that line was added -- a reload should never create a new
history entry, should it?

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Testcase

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed patch

Note that in the testcase, after the second time I go "forward" there are
_three_ entries in session history instead of just two (without this patch).

Comment 4

[Boris Zbarsky [:bzbarsky]]

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

Adam, could you possibly review?  I wish radha were still reading bugmail, but
maybe you know why she made this weird change?

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch that actually compiles

I accidentally diffed while experimenting with something else...

Comment 6

[Adam Lock]

Comment on attachment 136990 [details] [diff] [review]
Patch that actually compiles

The original patch went in for bug 160869. I'm supposing that this test is to
stop certain kinds of reloads and history actions from (including involuntary
meta charset reloads) from adding history items. 

nsDocShell.h defines quite a few combinations of LOAD_CMD_RELOAD so we'd have
to be careful not to break any of those combos. Assuming nothing breaks, I
don't have any problems so r=adamlock

http://lxr.mozilla.org/seamonkey/source/docshell/base/nsDocShell.h

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 136990 [details] [diff] [review]
Patch that actually compiles

Nothing breaks that I've found yet... And the code was not added for bug 160869
but for bug 144301 like comment 1 says.

darin, could you sr?

Comment 8

[Darin Fisher]

Comment on attachment 136990 [details] [diff] [review]
Patch that actually compiles

sr=darin

sorry it took me so long to get to this one!

Comment 9

[Boris Zbarsky [:bzbarsky]]

Taking.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Checked in for 1.7a.

Comment 11

[Boris Zbarsky [:bzbarsky]]

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

Comment 12

[Max Alekseyev]

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