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

VERIFIED FIXED in Future

Status

()

Core
HTML: Form Submission
P2
normal
VERIFIED FIXED
19 years ago
17 years ago

People

(Reporter: Mike Strother, Assigned: harishd)

Tracking

({dataloss})

Trunk
Future
dataloss
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(5 attachments)

(Reporter)

Description

19 years ago
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>

Comment 1

19 years ago
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

Comment 2

19 years ago
Created attachment 5108 [details]
test case

Comment 3

19 years ago
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

Comment 4

19 years ago
This failure to truncate existing variables is already reported as bug 25330.
Status: NEW → ASSIGNED

Updated

19 years ago
Keywords: beta1

Comment 5

19 years ago
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.

Comment 6

19 years ago
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

Comment 7

19 years ago
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

Comment 8

19 years ago
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

Comment 9

19 years ago
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

Comment 10

19 years ago
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

Comment 11

19 years ago
Working on a fix - this seems really closely related to bug 17685.  I'll try to
come up with something tonight.

Comment 12

19 years ago
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).

Comment 13

19 years ago
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

Comment 14

19 years ago
Created attachment 6716 [details] [diff] [review]
Patch that might help for new Session History

Comment 15

19 years ago
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.

Updated

19 years ago
Depends on: 17685

Comment 16

19 years ago
Putting on PDT- radar for beta1.
Whiteboard: w/b minus on 03/20 → [PDT-]w/b minus on 03/20
(Reporter)

Comment 17

19 years ago
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

Comment 18

19 years ago
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.

Comment 19

18 years ago
Adding nsbeta2 to keywords.
Keywords: nsbeta2
Whiteboard: [PDT-]w/b minus on 03/20 → w/b minus on 03/20

Comment 20

18 years ago
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

Updated

18 years ago
Whiteboard: [nsbeta2-] w/b minus on 03/20 → [nsbeta2-]

Comment 21

18 years ago
See bug 34743.  Another example is available here:

http://www.orageeks.com/xgeek.html

Comment 22

18 years ago
*** Bug 40820 has been marked as a duplicate of this bug. ***

Comment 23

18 years ago
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.

Comment 25

18 years ago
*** Bug 34987 has been marked as a duplicate of this bug. ***

Comment 26

18 years ago
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?

Comment 27

18 years ago
*** Bug 38940 has been marked as a duplicate of this bug. ***

Comment 28

18 years ago
*** Bug 37250 has been marked as a duplicate of this bug. ***

Comment 29

18 years ago
*** Bug 41648 has been marked as a duplicate of this bug. ***

Comment 30

18 years ago
*** Bug 28019 has been marked as a duplicate of this bug. ***

Comment 31

18 years ago
This bug still exists, see bug 28019 for details.

Comment 32

18 years ago
bug 18643 seems to be related. not sure.

Comment 33

18 years ago
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.

Comment 34

18 years ago
*** Bug 47122 has been marked as a duplicate of this bug. ***

Comment 35

18 years ago
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

Comment 36

18 years ago
I'm lost. what's the bug here. is the summary correct?
Target Milestone: --- → Future

Comment 37

18 years ago
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">

Comment 38

18 years ago
See my comment on 2000-03-20 09:14 for a testcase that demonstrates the problem.

Comment 39

18 years ago
*** Bug 50272 has been marked as a duplicate of this bug. ***

Comment 40

18 years ago
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

Updated

18 years ago
Whiteboard: [nsbeta2-] → [nsbeta2-][nsbeta3+]

Comment 43

18 years ago
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.

Comment 44

18 years ago
Created attachment 14758 [details] [diff] [review]
patch to docshell which makes it grab session history info (post data) during a reload.

Comment 45

18 years ago
Created attachment 14759 [details] [diff] [review]
fixes http to stop processing data if it's stopped.

Comment 46

18 years ago
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

Comment 47

18 years ago
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)

Comment 48

18 years ago
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.

Comment 50

18 years ago
Jud for your HTTP changes r=gagan.

Comment 51

18 years ago
I have a review from Gagan on the http piece. Just need the docshell part
reviewed.

Comment 52

18 years ago
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?

Comment 53

18 years ago
pdt agrees p2
Whiteboard: [nsbeta2-][nsbeta3+] → [nsbeta2-][nsbeta3+][pdtp2]
I have provided my review too in the email to judson r=radha

Comment 55

18 years ago
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]

Comment 56

18 years ago
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

Comment 57

18 years ago
*** Bug 22131 has been marked as a duplicate of this bug. ***

Comment 58

18 years ago
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-]

Comment 60

18 years ago
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-]

Updated

18 years ago
Blocks: 57838

Updated

18 years ago
No longer blocks: 57838

Comment 61

18 years ago
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.)

Comment 62

18 years ago
marking rtm need info since you appear to be working on this bug.
Whiteboard: [nsbeta2-][nsbeta3-] → [nsbeta2-][nsbeta3-][rtm need info]
(Assignee)

Comment 63

18 years ago
Moving bug to my list.
Assignee: rickg → harishd
Status: ASSIGNED → NEW
(Assignee)

Comment 64

18 years ago
Created attachment 18314 [details] [diff] [review]
Proposed patch [ partial fix - Doesn't fix international sites :-(]

Comment 65

18 years ago
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 :-)

Updated

18 years ago
Whiteboard: [nsbeta2-][nsbeta3-][rtm need info] → [nsbeta2-][nsbeta3-][rtm need info] r=rpotts, NEED SR=

Comment 66

18 years ago
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.)

Comment 67

18 years ago
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.

Comment 68

18 years ago
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).

Comment 69

18 years ago
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.
(Assignee)

Comment 70

18 years ago
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

Comment 71

18 years ago
Harish, Are you going to open a new bug for the "real" fix?
(Assignee)

Comment 72

18 years ago
Bob - Yes I will.
(Assignee)

Comment 73

18 years ago
Fix is in the TRUNK.

Also, added Erik to the cc list.
(Assignee)

Updated

18 years ago
Whiteboard: [nsbeta2-][nsbeta3-][rtm+] r=rpotts, NEED SR=vidur → [nsbeta2-][nsbeta3-][rtm+] r=rpotts,SR=vidur
(Assignee)

Updated

18 years ago
Whiteboard: [nsbeta2-][nsbeta3-][rtm+] r=rpotts,SR=vidur → [nsbeta2-][nsbeta3-][rtm+][r=rpotts,pollmann SR=vidur]

Comment 74

18 years ago
rtm++, please checkin ASAP.
Whiteboard: [nsbeta2-][nsbeta3-][rtm+][r=rpotts,pollmann SR=vidur] → [nsbeta2-][nsbeta3-][rtm++][r=rpotts,pollmann SR=vidur]
(Assignee)

Comment 75

18 years ago
Fix landed on the branch. Marking FIXED.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 76

18 years ago
Verified using 2000-11-04-09 MN6 using both of blueviper tests with WinNT 

Comment 77

18 years ago
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

Comment 79

17 years ago
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.
You need to log in before you can comment on or make changes to this bug.