Closed Bug 227672 Opened 21 years ago Closed 21 years ago

[FIXr]Repost on back/forward confuses session history

Categories

(Core :: DOM: Navigation, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(3 files)

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
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?
Attached file Testcase
Attached patch Proposed patchSplinter Review
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 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?
Attachment #136951 - Flags: review?(adamlock)
I accidentally diffed while experimenting with something else...
Attachment #136951 - Attachment is obsolete: true
Attachment #136951 - Attachment is obsolete: false
Attachment #136951 - Flags: review?(adamlock) → review-
Attachment #136990 - Flags: review?(adamlock)
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
Attachment #136990 - Flags: review?(adamlock) → review+
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?
Attachment #136990 - Flags: superreview?(darin)
Comment on attachment 136990 [details] [diff] [review]
Patch that actually compiles

sr=darin

sorry it took me so long to get to this one!
Attachment #136990 - Flags: superreview?(darin) → superreview+
Taking.
Assignee: core.history.global → bz-vacation
Summary: Repost on back/forward confuses session history → [FIXr]Repost on back/forward confuses session history
Checked in for 1.7a.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
*** Bug 167458 has been marked as a duplicate of this bug. ***
*** Bug 185441 has been marked as a duplicate of this bug. ***
Component: History: Session → Document Navigation
QA Contact: docshell
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: