Closed Bug 27006 Opened 25 years ago Closed 24 years ago

<meta> content-type with charset doesn't repost form data on reload

Categories

(Core :: DOM: Core & HTML, defect, P2)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Future

People

(Reporter: webmaster, Assigned: harishd)

References

(
URL
)

Details

(Keywords: dataloss, Whiteboard: [nsbeta2-][nsbeta3-][rtm++][r=rpotts,pollmann SR=vidur])

Attachments

(5 files)

test case
(deleted), text/html
Details
Patch that might help for new Session History
(deleted), patch
Details | Diff | Splinter Review
patch to docshell which makes it grab session history info (post data) during a reload.
(deleted), patch
Details | Diff | Splinter Review
fixes http to stop processing data if it's stopped.
(deleted), patch
Details | Diff | Splinter Review
Proposed patch [ partial fix - Doesn't fix international sites :-(]
(deleted), patch
Details | Diff | Splinter Review
User-Agent: Mozilla/5.0 [en-US] (Windows_98; I) BuildID: 2000012520 I am the author of the URL. A javascript onchange event is triggered when a different item in the drop-down list is selected. The asp script is run utilizing the selection as a filter. However, when run from this version of Mozilla, the ASP does not receive the necessary information and defaults to show everything. Reproducible: Always Steps to Reproduce: 1.Open the URL 2.Select a different item from the drop-down list 3.The page will reload with default data Actual Results: Page reloads with default data (drop-down list selection is not transferred to server) Expected Results: A shorter list should have appeared. Here is the HTML in the page: <select name="HL_Category_ID" onChange="if(options[selectedIndex].value) this.form.submit()"> <option value="0">(all categories)</option> <option value=1>Books</option><option value=2>Events</option><option value=3>Friends</option><option value=4>Futurists</option><option value=5>Interviews</option><option value=6>Resources</option><option value=7>Viewpoints</option> </select>
Reassigning to the Document Object Model component, as that is where onChange is implemented. (APIs exposed to JavaScript that are specific to the browser aren't part of the JavaScript engine itself.) FYI this is *much* more likely to get fixed if you can reproduce it with the simplest possible testcase. Could you author another page that has just enough to demonstrate that onChange is not being called, possibly by just popping up an alert or changing another field? Thanks - Mike
Assignee: mccabe → vidur
Component: Javascript Engine → DOM Level 0
QA Contact: rginda → desale
Attached file test case (deleted) —
Note that the problem with the reduced test case is that the search component of the original URL isn't removed before adding the new search component from the form submission. I don't think it's the same problem seen in the original report. Handing over to pollmann - the forms owner.
Assignee: vidur → pollmann
This failure to truncate existing variables is already reported as bug 25330.
Status: NEW → ASSIGNED
Keywords: beta1
As Vidur suggested, the problem in the original bug has nothing to do with truncating variables (bug 25330). The problem in the original bug is unclear, but is not a general "Drop-down list onchange event doesn't submit correct data" bug. I did a trace on the network traffic in the transaction and noticed that we are posting the reqest correctly. We then receive the correct server response. Then, for some unexplained reason, we immediately go back and request the original page, which hasn't had a category selected, and then get the original page back form the server. I don't know why this is yet, but am looking into it. I have only seen this bug on this particular website.
Minimal test case to get the form submitted twice: <HTML> <BODY> <FORM ACTION="/foo.html"> <INPUT TYPE=SUBMIT> </FORM> </BODY> </HTML> The only requirement here is that the ACTION begins with a / This means potentially many pages could be affected. All pages with a form beginning with / are being sent to the server twice. This particular test case is unusual in that the post *data* is only sent the first time, but not the second. I'm still tracing down: 1) Why the heck is the page being posted twice? nsFormFrame::OnSubmit is called only once Down in netlib, it seems like the relevent functions are called only once 2) Why is the post data lost after the first post?
Severity: normal → major
Component: DOM Level 0 → Networking
OS: Windows 98 → All
Hardware: PC → All
Summary: Drop-down list onchange event doesn't submit correct data → <FORM ACTION="/..."> sent to server twice
Ack! Please ignore my previous two posts - there is no double posting - this was due to a bug in my network tracing script. *blush*. Removing Netlib people I added to CC. Thanks!
Severity: major → normal
Component: Networking → Form Submission
Summary: <FORM ACTION="/..."> sent to server twice → Drop-down list onchange event doesn't submit correct data
Putting on PDT+ radar. But must have 2 reviewers and risk needs to be low. See jar.
Whiteboard: [PDT+] w/b minus on 03/20
The problem is caused by this: <meta http-equiv="Content-Type" content="text/html; charset=windows-1252"> This causes the incoming document (which has the correct results from the correct post data) to be reloaded. The problem is that when we reload the document, we do not post the form data.
Summary: Drop-down list onchange event doesn't submit correct data → <meta> content-type with charset doesn't repost form data on reload
hmmm... well it sounds like you understand the bug... but is there anything that we can do in short order, with REALLY low risk? For beta 1, we really need to be done by this weekend. Comments? Proposals? Reviews? Side effects? Thanks, Jim
Working on a fix - this seems really closely related to bug 17685. I'll try to come up with something tonight.
I can't verify that bug 17685 was fixed. This bug depends on the same functionality as bug 17685. The postDataStream needs to be stored in session history then reset and resent when the page is reloaded. I have tried several changes to the docshell, but can't get access to the postDataStream. Travis or Radha, can either of you point out where this might be going wrong? (I've tried both beta1 branch and tip and can't see this working on either).
I don't think this is fixable for beta1 because we are still using the old session history, which doesn't store the post data stream. I'm attaching a diff that might help once we switch over to new session history (completely untested). This code tries to get the postDataStream during reload and re-use it. Doesn't look like this is implemented yet, even for the new world. Removing PDT+ because I don't think this can make beta1 Passing over to Travis because the change is in docShell.
Assignee: pollmann → travis
Status: ASSIGNED → NEW
Whiteboard: [PDT+] w/b minus on 03/20 → w/b minus on 03/20
There is a test case for this bug here: http://blueviper/forms/0027006_double_post This will post a very simple form, and return a page with: 1) a meta charset tag to force a reload 2) a global counter indicating how many times the page has been loaded 3) a list of all the data posted to the form You should notice that the counter goes up by 2 each time the form is posted, indicating the page was loaded twice. When this bug is fixed, the post results will be shown on the page (HL_Category_ID = 1) If it's not fixed you'll just see the counter. There is a test case for general form post reload functionality (bug 17685) here: http://blueviper/forms/0027006_double_post/index2.html When posted, this will load a page with: 1) a global counter indicating how many times the page has been loaded 2) a list of all the data posted to the form You'll see that the form data is posted on this page (HL_Category_ID = 1) After viewing this page, click reload. When bug 17685 is fixed, you'll see the form data is still posted after reload. It's currently not.
Depends on: 17685
Putting on PDT- radar for beta1.
Whiteboard: w/b minus on 03/20 → [PDT-]w/b minus on 03/20
FYI: I just downloaded Netscape 6 preview release 1, and noticed that the bug is still present. Another test case URL exists on our site which exhibits the same behavior; it can be found at http://www.firstmatter.com/newsletter/archives.asp. Michael
I just posted bug 35208 that is a crasher related to the original URL http://www.firstmatter.com/hotlinks.asp. Seen on my Win95 and another person's W2K box.
Adding nsbeta2 to keywords.
Keywords: nsbeta2
Whiteboard: [PDT-]w/b minus on 03/20 → w/b minus on 03/20
valeski pretty sure this a dup. Suggest ekrock include to how to do this in his educational docs for content development. Putting on [nsbeta2-] for beta2.
Assignee: travis → valeski
Whiteboard: w/b minus on 03/20 → [nsbeta2-] w/b minus on 03/20
Whiteboard: [nsbeta2-] w/b minus on 03/20 → [nsbeta2-]
See bug 34743. Another example is available here: http://www.orageeks.com/xgeek.html
*** Bug 40820 has been marked as a duplicate of this bug. ***
I also encountered this bug and can confirm it is fixed on my page. It was occuring when I pressed "back" to go to a previous form page. (I didn't have the meta tag though) I now recieve a message asking if I would like to repost when I press "back". If I click "ok" it works, however I find having to click "ok" everytime very annoying. Can this confirmation be disabled somehow? (so that it defaults to "ok")
Yes, I'm aware of the situation that this dialog can be annoying at times. I noticed that Nav 4.x doesn't put this dialog on all pages that have post data in it. I'm trying to figure what criteria Nav 4.x uses to put up this dialog. There is a way to turn off this dialog. Please look for the file "appstrings.properties" in your download. In the source tree this file is in docshell/base. In that file, either delete or comment off the line that says "repost=The page you are about to load....." The dialog won't come up, but the default is to repost data. Please be aware that reposting by default, may make you do things you dont really inteded to do.
*** Bug 34987 has been marked as a duplicate of this bug. ***
Regarding the pop up message about reposting form data: I think the reason you don't see this message when you press the "back" button to go to a previous page is that Nav 4.x doesn't repost the data. It appears to simply load the previous cgi page output from the cache and display that. In order to get it to repost, you actually have to press "reload" at which point you do get the message. Example: Page1.html has a form with fields. When submitted, it's action is Page2.cgi Page2.cgi prints some info based on Page1.html form values, also has a submit button to get to Page3.cgi. Page3.cgi print some stuff. You start at Page1.html, submit to get to Page2.cgi. You then submit Page2.cgi to get to Page3.cgi. At Page3.cgi, you press "back", which returns you to a cached copy of Page2.cgi. If you then press "reload", a pop-up appears and pressing "ok" resubmits the form values from Page1.html. From some testing, the above example appears to be the way Nav 4.x works. Could this be duplicated in Mozilla?
*** Bug 38940 has been marked as a duplicate of this bug. ***
*** Bug 37250 has been marked as a duplicate of this bug. ***
*** Bug 41648 has been marked as a duplicate of this bug. ***
*** Bug 28019 has been marked as a duplicate of this bug. ***
This bug still exists, see bug 28019 for details.
bug 18643 seems to be related. not sure.
Bug 18643 is actually a separate issue - an alternate method of sending a form's charset from the client to the server. It's non-standard (IE only, not in any spec) and not yet implemented in mozilla, so it's not part of this error.
*** Bug 47122 has been marked as a duplicate of this bug. ***
Nominating for nsbeta3 - this breaks a number of sites, including many, if not all, post results generated by Microsoft FrontPage 4.0.
Keywords: correctness, nsbeta3
I'm lost. what's the bug here. is the summary correct?
Target Milestone: --- → Future
Close to correct - the problem is with pages that are returned as the result of a POST to the server that contain something like this: <meta http-equiv="Content-Type" content="text/html; charset=windows-1252"> This meta tag will cause the page to be requested a second time (using the new charset this second time). The problem is that the second request is still a POST but does not repost the form data as it should. The server then sees this second request a simply another request with no post data and will return something wrong, or depending on the CGI on the server side, nothing at all. There are a whole class of sites that fail because of this and something I've noticed is that a lot of them are generated by FrontPage: <meta name="GENERATOR" content="Microsoft FrontPage 4.0">
See my comment on 2000-03-20 09:14 for a testcase that demonstrates the problem.
*** Bug 50272 has been marked as a duplicate of this bug. ***
This seems like a bad bug - any hope it will make beta3 or rtm?
Keywords: mostfreq
Eric, It looks like in pages with meta charset, the reload() operation should get the postdata from SH reset to the beginning of the stream and repost it just like nsDOcShell::LoadHistoryEntry() does. Is that right? Please look at nsDocShell::Reload() There is some code inside #if 0. I think we attempted to fix this but then it caused some other crash.
Marking dataloss and 4xp, nominating rtm, and raising priority to P2 for these reasons: 1) Forms are one of the most widely used things on the web. 2) By failing to submit/repost correctly, we're likely to lose/corrupt user data. 3) Front Page is (unfortunately! ;-> ) widely used for web page creation. Fouling up all/many of the forms created by Front Page is a very bad idea. 4) We're getting multiple DUPs indicating that this is likely to affect many pages/sites/users. Jud, I know you're swamped. Any chance we could reassign to someone else and get this fixed? Somehow? ;->
Keywords: 4xp, dataloss, rtm
Priority: P3 → P2
Whiteboard: [nsbeta2-] → [nsbeta2-][nsbeta3+]
pollmann - all the test cases in here are dead except for the one on blueviper which I think is only partially working. Can you change the one on blueviper to use a charset that will *force* a reload. There's a check in the i18n metacharset observer that will *not* reload the current document to adjust the charset if the existing charset is the same as the one requested in the meta tag. On my win98 box, the charsets are the same, thus I can't get the reload to happen.
I've attatched 2 patches which fix this bug. 1. docshell - this turns on session history usage durning reload. apparently this used to cause a crash when loading the japanese home.netscape.com page, but I'm not seeing that. 2. http - This fixes a crash in http that change #1 aggrivates. I need reviews. Also, I'm not sure exactly how things are supposed to work here, but currently (w/ my patches) if you load the blueviper URL, then click the button, you get a "do you want to repost form data" dialog which seems wrong because as far as the user is concerned, they're only hitting submit once, the first time. Not sure how to handle that case here. Description of what's happening. You click submit which causes a POST. The response coming back from the server contains a META charset header in the html content. This causes nsMetaCharsetObsever::Notify() to be called which will eventually call nsDocShell::Reload() if the default charset and the charset suggested by the META tag don't match. Without my changes we use the nsDocShell::InternalLoad() method to do the reload and that method doesn't take a session history entry (and subsequently doesn't repost post data). My changes make nsDocShell::Reload() use the nsDocShell::LoadHistoryEntry() method (and subsequently reposts the post data). LoadHistoryEntry() is the method that throws the dialog.
Keywords: patch
Hey Jud, I just changed the testcase on blueviper to return content-type x-sjis, which should force a reload unless you have the Japanese version of Windows. :) Feel free to change the CGI yourself if you want, it's on blueviper at /home/httpd/cgi-bin/count2.cgi and that dir and file are world-writable (well, inside the firewall anyhow)
Also, before checking in the fix, it would be good to try verifying that it doesn't reopen the all-elusive crasher bug 45297. *sigh* Here's hoping that this is fixed by now! :) This fix is also good because it will cause post data to be restored for form submits inside frameset pages, not just this case.
the crash in 45297 was caused in nsPresShell::GetHistoryState. This could be because in nsPresShell.cpp nsILayoutHIstoryState is not a COMPtr Weak Ref. It is a simple pointer object nsILayoutHistoryState *. I think while trying to reload some objects, the history object was freed by sessionHistory, but presShell didn't know about it. If at all that crash re-appeared, one good way to fix that would be to change the pointer object in nsPresShell to a weak ref.
Jud for your HTTP changes r=gagan.
I have a review from Gagan on the http piece. Just need the docshell part reviewed.
The docShell changes look good to me. You can mark me as the reviewer, but I'm not the expert. Radha, can you review them too?
pdt agrees p2
Whiteboard: [nsbeta2-][nsbeta3+] → [nsbeta2-][nsbeta3+][pdtp2]
I have provided my review too in the email to judson r=radha
Not holding PR3 for this, so marking nsbeta3-. Already have the rtm nomination to get this on the radar after PR3.
Whiteboard: [nsbeta2-][nsbeta3+][pdtp2] → [nsbeta2-][nsbeta3-][pdtp2]
re-assigning to rickg. note: my fix just causes more problems down the road and should *not* be used. gessner has a solid understanding if the "proper" fix (and also note that even the "proper" fix has it's drawbacks :-/). I've said it a million times, meta http-equiv is a bastardization and an abomonation!
Assignee: valeski → rickg
*** Bug 22131 has been marked as a duplicate of this bug. ***
Removing rtm and beta3
Status: NEW → ASSIGNED
Keywords: rtm
Whiteboard: [nsbeta2-][nsbeta3-][pdtp2]
Restoring keywords and marking [nsbeta2-][nsbeta3-][rtm-]. Rick -- FYI, to preserve a historical record for future prioritization of which bugs were nominated but rejected, we leave the keywords in and add the minuses in the Status. This will help us in ns601 prioritization work. Eliminating nominations is bad because it loses that data. Thanks!
Keywords: rtm
Whiteboard: [nsbeta2-][nsbeta3-][rtm-]
Renominating for RTM. See bug 57838. I'm almost certain that is a duplicate of this bug because the page contains a <META> with charset specified and uses form post.
Whiteboard: [nsbeta2-][nsbeta3-][rtm-] → [nsbeta2-][nsbeta3-]
Blocks: 57838
No longer blocks: 57838
Harish and I are splitting up the task, and hope to offer a solution in a reasonable timeframe. I'm afraid that a quickie hack wont cut it, so we have to make some real effort to get you a workable solution. It's likely to be low-risk, but more time consuming that the HACK I'd originally hoped to do. (The cause of the extra work is due to the way that the meta observer works today. It doesn't return an error to the parser at all.)
marking rtm need info since you appear to be working on this bug.
Whiteboard: [nsbeta2-][nsbeta3-] → [nsbeta2-][nsbeta3-][rtm need info]
Moving bug to my list.
Assignee: rickg → harishd
Status: ASSIGNED → NEW
This patch looks good. (r=rpotts) There's much more that needs to be done to fix this situation - but this is a good first step for RTM :-)
Whiteboard: [nsbeta2-][nsbeta3-][rtm need info] → [nsbeta2-][nsbeta3-][rtm need info] r=rpotts, NEED SR=
Redundant r=pollmann :) This patch really should go in for RTM. Without it, this bug can cause data loss and even potentially monetary loss! (Transaction succeeds but user is told it didn't suceed - user may try again repeatedly, or give up and use another browser - causing multiple transactions, and *very* unhappy user.)
The NS_IF_RELEASE of mBundle in the OnStopRequest makes me feel better, so sr=vidur. I will say, though, that the whole notion of the "parser bundle" as a mechanism for holding and transferring opaque object references is horrible and should be fixed post rtm. The biggest problem I have with it is the risk it introduces for circular references and leaks. The fact that the bundle keeps string keyed references gives the illusion of weakness, though the references are actually strong. Adding an NS_IF_RELEASE of the bundle to the OnStopRequest of nsParser reduces the chances of leaking (a chance that already existed even before this checkin because the docshell is stored in the parser bundle). Even so, this mechanism should be changed post RTM.
Vidur: parsebundle was intended to reflect state -- not transfer ownership of object references. It was a way to share Read-only string info with observers. I'd prefer it went away altogether. I think that we should reconsider how we do meta charset handling. Making the parser be responsible for this (as a process) weakens the parser for general use. Here's what I recommend that we do: We could keep the notification mechanism (improved as vidur suggests), but not let the observer cause a document reload if the charset changes. Instead, let the i18n library walk the content model (as a post process of document loading) and convert the content with the new charset. Incremental reflow could be used to update the view(s).
For RTM, this change is OK with the I18N team. For the future, rickg> Instead, let the i18n library walk the content model (as a post process rickg> of document loading) and convert the content with the new charset. But won't the data in the content model be misconverted data? What we need is the data off the wire in order to do the correct conversion.
Marking rtm+. Will checkin into trunk first per jar's request.
Status: NEW → ASSIGNED
Whiteboard: [nsbeta2-][nsbeta3-][rtm need info] r=rpotts, NEED SR= → [nsbeta2-][nsbeta3-][rtm+] r=rpotts, NEED SR=vidur
Harish, Are you going to open a new bug for the "real" fix?
Bob - Yes I will.
Fix is in the TRUNK. Also, added Erik to the cc list.
Whiteboard: [nsbeta2-][nsbeta3-][rtm+] r=rpotts, NEED SR=vidur → [nsbeta2-][nsbeta3-][rtm+] r=rpotts,SR=vidur
Whiteboard: [nsbeta2-][nsbeta3-][rtm+] r=rpotts,SR=vidur → [nsbeta2-][nsbeta3-][rtm+][r=rpotts,pollmann SR=vidur]
rtm++, please checkin ASAP.
Whiteboard: [nsbeta2-][nsbeta3-][rtm+][r=rpotts,pollmann SR=vidur] → [nsbeta2-][nsbeta3-][rtm++][r=rpotts,pollmann SR=vidur]
Fix landed on the branch. Marking FIXED.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Verified using 2000-11-04-09 MN6 using both of blueviper tests with WinNT
Verifying on Windows 98 build 2000-11-09-09-MN6 And Linux RedHat6.2 build 2000-11-09-09-MN6
Status: RESOLVED → VERIFIED
Can the bug which is opened for the "real" fix for this problem me marked "mostfreq", please (seeing as this one is)? Thanks, Gerv
Sorry for the spam, but I believe I submitted the same bug report a while ago and I only see now it might be a dup from this bug. See bug #68433 still opened.
Component: HTML: Form Submission → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: