Closed Bug 46338 Opened 24 years ago Closed 24 years ago

Form submission urls getting added to session history.

Categories

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

Product:
Core
Component:
DOM: Navigation
Type:
defect

Tracking

()

Status:
VERIFIED WORKSFORME
Milestone:
Future

People

(Reporter: mscott, Assigned: mscott)

References

Details

(Whiteboard: [nsbeta2-][pdtp1]) 

I'm not sure if this belongs to session history or to form submission. I'll
start with session history. Here's what I'm doing:
1) Make a change to a bugzilla bug. Hit the commit button. This runs a form
posting url.

2) I get redirected to a new screen that says the change succeeded.

If I hit the back or the reload button, I expect to be taken back to the
bugzilla bug I just modified.

Instead I get a dialog saying something about the url contains post data, do I
wish to repost. If I hit OKAY or if I hit CANCEL, we re-run the form submission
url and the data gets reposted to the server.

This is really bad as it causes you to post data back to th server when you
don't want to. What if I was buying a book on amazon and I hit the reload or
back button. I'd hate to end up with two books!
nominating for beta2 because of the reposting data problem. I think that meets
the criteria for pulling the beta off the wire.
Keywords: nsbeta2, nsbeta3
The data is not re-posted when you hit cancel button in that dialog. 
Hmm I'm seeing the data get reposted when I repost. I hit cancel and it
immediately trys to post the data again. In the case of bugzilla, I can actually
get a response back from the server where it says:

Your changes collided with someone else's changes. And it shows me the changes I
just posted as the stuff I'm colliding with. 
Not sure what exactly happens with bugzilla. The dialog work was done for bug # 
17685. some of the pages this was tested on are in that bug. I suspect if cache 
is playing a role in it. Myself/Neeti are working on hooking cache with session 
history. Try with cache set to "everytime". 
Nav triage team: [nsbeta3+]
Whiteboard: [nsbeta3+]
Putting on [nsbeta2-] radar.
Whiteboard: [nsbeta3+] → [nsbeta2-][nsbeta3+]
I'm going to ask PDT to re-examine this. The fact that hitting okay or cancel on
the dialog when you hit the back or reload button causes the form url to get
reposted is a really big problem when it comes to buying things from sites.

This almost happened to me just today when I was buying some tickets online. I
hit that dialog after submitting the purchase and I actually powered down my
machine because I knew if I tried to dismiss the dialog, I'd end up with buying
two sets of tickets!!

Maybe I'm the only one that sees the post on cancel though. I think Radha
mentioned that she doesn't see it. If it's just me then a minus would be
appropriate but if this really is going on I would pull it off the wire for this
fix.
Status: NEW → ASSIGNED
Whiteboard: [nsbeta2-][nsbeta3+] → [nsbeta3+]
Putting on [NEED INFO] radar. PDT needs to know impact to user and risk of fix 
to make a call on this bug. QA - Can we get this reproduced and find out if 
purchases go through twice?  Also, how much of a risk would this be to fix?  
Please verify any problems this might cause according to mscott's comments.
Whiteboard: [nsbeta3+] → [nsbeta3+][NEED INFO]
nsbeta2-.  Commercial sites should all have guards against accepting the exact
same transaction twice in a row and interpreting it as a distinct order.
Bugzilla gives you a midair collision, for instance.  Don't want to risk making
changes to history for this in nsbeta2.
Whiteboard: [nsbeta3+][NEED INFO] → [nsbeta3+][NEED INFO][nsbeta2-]
*** Bug 46825 has been marked as a duplicate of this bug. ***
heh I'm sure the user is going to appreciate that answer when they just bought
twice as many shares on e*trade tha they intended to =). Oh well...
>>QA - Can we get this reproduced and find out if 
purchases go through twice?

Ummm, sure - as soon as PDT gets me a credit card number or I can find some relevant testcase that doesn't involve money.
Whiteboard: [nsbeta3+][NEED INFO][nsbeta2-] → [nsbeta3+][NEED INFO] Need Info again
Priority: P3 → P1
PR2 is out...adding [nsbeta2-] Already has [nsbeta3+]
Whiteboard: [nsbeta3+][NEED INFO] Need Info again → [nsbeta3+][nsbeta2-][NEED INFO] Need Info again
Target Milestone: --- → M18
I wrote a testcase: http://clarence.de/post-test/form.html

I can reproduce a similar sitation to a repost after hitting "Cancel" with it
using M17 and 2000082313 on win NT.
This is what I think that happens:

1. POST data to a page which responds with a redirect via HTML META.
2. Go back; Mozilla asks you, if you want to repost the data.
3. Say "Cancel".

Actual result:
The page with the redirect will be retrieved with HTTP _GET_.
Don't know if the data will be posted again. It doesn't arrive in my CGI, but
maybe Apache doesn't pass it via CGI on a HTTP GET.

Expected result:
The page gets not retrieved at all.
Instead there are 3 possible behaviours:
- Print a error message (that's what 4.x does).
- Go back another step in session history to get to the page with the form
  which was posted (that's what mscott expects)
- Ignore the "go back" (best solution, IMHO).

Note 1:
You'll have to clear the caches explicitly or restart Mozilla to observe the
described behaviour a second time.
Otherwise the page is served from cache, even if you disable all caches in
debug prefs, use the pref "compare it to page on the network every time" and
include the HTTP "Pragma: no-cache" header.
This might be the right behaviour when navigating through session history.
If it's not, somebody should file a different bug for this.

Note 2:
When you go back to a page with radio buttons, previous checked buttons _and_
newly checked buttons will be checked.
This is a different bug (bug 50143), but makes my testcase difficult to use.
Workaround: Place cursor in location field and type return.

Bug 47924 is a dupe or at least very related.
Pressing Reload to a direct result of a POST has the same effect as going back
from a redirected result.

Note also bug 43123 which is somewhat related.

Yes, those are identical.  In fact, the other two are both clearer than this 
one and they are identical to each other as far as I can tell.
Suggestion how to merge bugs 46338, 47924 and 43123:

Bug 43123 is in the main point a RFE for a better UI (3rd button).
This needs not to be done in nsbeta3 (although it would be fine).

Bug 46338 (this bug) deals with a strange misbehaviour, which should definitly
be fixed in nsbeta3.

Bug 47924 is very similar to 46338 and has aspects of 43123.

Thus: Fix 46338 as soon as possible, mark 47924 as a dupe and keep 43123 as a
separate bug.

But if all should be done at once, 47924 seems to be the right bug to track.

In each case, keep a look at comments in duped bugs, they are worth it.

(This comment added to 46338 since it has already nsbeta3+ and P1).

PDT agrees P1, but pretty close to P2
I have a fix in hand for this that came with some other bug fixes I have.
Assuming radha and I give my changes a thumbs up, I'll check the fix in for this.
Assignee: radha → mscott
Status: ASSIGNED → NEW
Whiteboard: [nsbeta3+][nsbeta2-][NEED INFO] Need Info again → [nsbeta3+][nsbeta2-] fix in hand
I just checked in a fix for this.

An easy test, make a change to a bugzilla bug. After it takes you to the next
bug in your list (after the form submission), hit the back button. You should
see yourself taken back to the bug page showing your comments. You should not
see a dialog asking you about reposting the data anymore.

Someone should also fix that dialog so cancel actually works if you do get the
dialog and try to cancel the repost.

In any case, you won't see that dialog because of hitting the back or forward
buttons anymore because the form submission url is no longer in session history.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
modifying this bug in an effort to verify it.
VERIFIED Fixed with 2000090608 builds
Status: RESOLVED → VERIFIED
Blocks: 51614
Appears I may have been too hasty... REOPENING. I believe this to be the same problem,
maybe it isn't, I'm not sure what I'm seeing, so here's what happened.

I created bug 51614 simply to test this without spamming everyone, feel free to use it as
well. To repro:

1. Load the bug in two separate windows (I actually used two machines).
2. Add a comment in window #1 and submit.
3. Now add a comment in window #2 and submit.(make it different so you can tell them apart)
	As expected you'll get a mid-air collision.
4. Click 'Submit your changes anyway'. Now click the 'Back' button.

When you click 'Back', the 'repost form data?' Dialog comes up.
If you click 'OK' you get another mid-air collision with the exact change you just made
(which I belive is correct). If you click 'Cancel' you get some wierd error page(???).

Sooo, looking at 4.x unless we changed something we shouldn't be getting the repost  form
data? Dialog box in this situation.
No longer blocks: 51614
Status: VERIFIED → REOPENED
OS: Windows NT → All
Hardware: PC → All
Resolution: FIXED → ---
Adding [pdtp1] in status summary.
Whiteboard: [nsbeta3+][nsbeta2-] fix in hand → [nsbeta3+][nsbeta2-][pdtp1] fix in hand
Clearing the fix in hand status.
Whiteboard: [nsbeta3+][nsbeta2-][pdtp1] fix in hand → [nsbeta3+][nsbeta2-][pdtp1]
I don't see this in the normal form submission case. So I think the fix handled
90% of the cases. The only condition in which I still see it is as claudius
mentioned, after a bugzilla collision. I think that makes it slightly less
severe than the original..
I agree, clearing nsbeta3+ for reconsideration. cc'ing ckrizter(form submission QA) in case
he has any ideas about how to test this further since I've exhausted mine.
Whiteboard: [nsbeta3+][nsbeta2-][pdtp1] → [nsbeta2-][pdtp1]
I keep being sent to a page saying "Bug processed. Product was not defined;
...Please help us track this down by adding coments to bug 20092."
after clicking "Cancel" on a "Repost form data?" dialog.

Since John Keiser mentioned there that the "repost form data" problem is being
tracked in bug 46338 and bug 43123, I'm commenting in these bugs.

Steps to reproduce:
1. Log out of bugzilla (if you're logged in).
2. Submit an additional comment to bug 51614.
3. Login as Bugzilla asks you to.
4. You see a "Bug processed." page. Click back.
5. You are asked if you want to "Repost form data". Click cancel.

Actual results: You see the process_bug.cgi error page that asks you to add a
comment to bug 20092.

Expected results: Everything else but this. 4.x for example takes me back to the
Login page.

You can vary step 4 by inserting some actions before clicking back, e.g.
1) Follow the "Back to bug 51614" link.
2) Observe that your additional comments are not displayed because the bug page
has been taken from cache. Click reload.
3) Observe that your additional comments are now displayed. Now click back.

You always get the same result: the annoying "Product not defined" error page.
Tested on PC/Linux build 2000091521.
Adding 4xp keyword to this bug. If you consider this an error, please undo.
Keywords: 4xp
mscott, your fix can't be a complete fix for this bug.
If I see it right, your fix causes HTTP 301 and 302 responses not to be added
to session history. That's a good behaviour (4.x does the same), but it doesn't
fix all occurrences of this bug.

Most posts (much more than the mentioned 10%) return a page with HTTP response
200 (OK). These pages _must_ be added to session history (Mozilla and 4.x do
so).

The problem is, that pressing "Cancel" in the dialog triggers an HTTP GET on the
page, but should cause no action at all. As long as nobody can say for sure,
that no entity containing the old post data is sent with the HTTP GET, I
consider this a definitive blocker for nsbeta3. And even if we can proof, that
the old post data isn't sent again: Mozilla does approximately the opposite the
user told it to to and that is very likely to occure.

PS: Summary (and component?) of this bug do not reflect the remaining problem.

BTW: HTTP 303 and 307 shouldn't be added to session history, too.
But Mozilla handles them like HTTP 200 for now (bug 48202), so this is work for
the future.


If someone can clearly describe what we're proposing to fix, we can consider
whether this should be plussed.
Whiteboard: [nsbeta2-][pdtp1] → [nsbeta2-][pdtp1][b3 need info]
no response, minus.  If you want to answer the previous question, please feel 
free to bring this back to our attention.
Whiteboard: [nsbeta2-][pdtp1][b3 need info] → [nsbeta2-][pdtp1][nsbeta3-]
Target Milestone: M18 → Future
The problem is, the Cancel button has unexpected behavior in the "Repost form 
data?" dialog box, which comes up when going back to or reloading a page 
resulting from an HTTP POST request.  When you hit cancel, instead of doing 
nothing and grabbing the page from the cache, as it should, it sends an HTTP 
GET request to the server, which makes no sense whatsoever and almost never 
retrieves the original page.  Steps to reproduce posted by Andreas a few 
comments up from this one.

So there are two major possible ways of fixing this:
1. Short Way
- Turn OK / Cancel into Yes / No, and make No grab from the cache instead of 
performing HTTP GET
2. Longer Way
- Same as above, but add a new Cancel button which aborts the reload or back 
button.  (Thus a total of 3 buttons: Yes / No / Cancel.)

(I have not verified this in the last week but have not seen any comments on 
this or bug 46338 / bug 43123, so I assume it's still broken.)

P.S. If we're tracking the problem in this bug, the summary needs to be changed 
to "Cancel on Reload of POST request performs HTTP GET" or something similar.
clearing the minus so this actually gets looked at now that the ? has been answered.
Whiteboard: [nsbeta2-][pdtp1][nsbeta3-] → [nsbeta2-][pdtp1]
Discussion seems to have moved to bug 55055.

nav triage team: not a beta stopper.
Keywords: nsbeta1-
mscott has fixed the original issue long time ago (2000-09-01).
Problems with the dialog (see claudius' comments from 2000-09-06 16:44 when
reopening this bug) are fixed too.
Other comments in this bug are off-topic (including mine ;) and are covered by
other bugs (especially bug 55055) now.

One problem remains with HTTPS, but I think that belongs in its own bug. See
bug 68797.
Status: REOPENED → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → WORKSFORME
mass-verifying WorksForMe bugs which haven't changed since 2001.12.31.

set your search string in mail to "EmperorLondoMollari" to filter out these
messages.
Status: RESOLVED → VERIFIED
Component: History: Session → Document Navigation
QA Contact: claudius → docshell
You need to log in before you can comment on or make changes to this bug.