This is a 0.9.4 build. If you hit enter in any text field in bugzilla the form is entered. This is a bad regression. If you have a form with multiple buttons hitting return in a text field means that the form is submitted with a vague button press. It used to be that the only way that hitting return in a text field would submit a form was if the text field was the only visible UI form element. I suspect that this is going to break a lot of web sites. We should really fix it.

Isn't this a feature?

No, it isn't. Which button did you depress to submit that form? That information probably isn't passed to the CGI on the other end which means it could end up breaking a truckload of sites.

> which means it could end up breaking a truckload of sites Unlikely, since our behavior now matches IE's. See bug 22526 for discussion, reviews, and the like.

This is definitely a bug, and IE does NOT behave this way. As much as IE is in violation of standards, mimicking IE's behavior is not desirable anyway. It would be naive to think that making such a drastic change in behavior of a long-established browser would by any means be acceptable. If Mozilla changes this drastically from release to release, people will simply turn to IE as the browser of choice. What a terrible thing to do!!!

You're forgetting a HUGE piece of the puzzle: What if there is NO submit button? What if I want to control form submission by JavaScript using "document.entryform.submit()"? Pressing [Enter] on an input element should NEVER cause an automatic submit(), unless that element is of type "SUBMIT". If a web page wants to submit the form based on any action, JavaScript can easily be used to do it. But having the Enter key submit the form automatically, basically prevents Mozilla from being able to be used to repetitively enter groups of numbers. The numeric keypad has an [Enter] key. A common method for moving between fields on the form is to trap the [Enter] key and treat it the same as [Tab]. Now you've broken this. And worse yet, if the user presses Enter on a form, there's no way to trap and cancel the submit operation because the onSubmit="... return false" is now broken also.

You can use IE and hit enter to submit a form. However, IE will use the default button and pass it on as it that button was clicked. ( How the "default" is determined is a mystery but that's a question for another date and time. ) So, when you use the url above the output will look like this in IE: o test1 o test o test2 o test o submit1 o test This is what it looks like in Mozilla: o text1 O test o text2 o test No submit button as part of the form data! I know for a fact that when I used to hack on web sites that when there was more than one button I would depend on the fact that the button press would _always_ be part of the submit data. Here's a real live example of how this breaks a web site as well: go to: http://www.ajb.dni.us/ enter a zip code into the zip code field and hit return. You will get an error: * You need to choose one occupational group to search from. If you press any of the buttons on the form you get somewhere reasonable. This breaks this web site.

ccing pollmann since he fixed bug 22526

Adding nsbranch+ for PDT nomination, and myself to cc: list.

It looks like we purposely doing this, mjudge touched the code last, but I don't think he was the original author. Why is it doing this on purpose? Kin do you know? It appears that 4.x will automatically submit a form as follows: <FORM METHOD=GET ACTION="search"> <A HREF="search">Text <BR>Search:</A> <INPUT TYPE=TEXT NAME="string" VALUE="" SIZE=8> <INPUT TYPE=SUBMIT VALUE="Find"> </FORM> but if I add another text field it stops submitting. That is really strange.

I have forms that have NO submit button at all (only text input fields), and it still submits whenever I press [Enter].

Rod, see bug 22526 for all the hows and so on. I believe that this should be marked a duplicate of 22526, which should be reopened... Ron: you're excluding people who don't have javascript in their browser (eg visually impaired users using Lynx and text-to-speech software) from using your site...

blizzard is of course correct, both winie5.5 and winie6 send the first button. not only do they send it, while focus is in an edit field, the button they intend to send has default attribute explicitly informing the user what will happen if enter is pressed.

*** This bug has been marked as a duplicate of 22526 ***

I understand the problem now - reopening

In IE the submit button that gets included in the submission is the one that has focus. It appears it is always the first submit button, it skips reset buttons. So I guess I can just assume we send the data for the submit btn.

The *real* fix seems like it might be pretty involved. Will it? If so, should we take the proposed patch for the current release? (Since we probably shipped 6.1 with this bug, could we just take this off the list and fix it for the next release?)

As far as I know you guys didn't ship this bug in 6.1. Anyway, it's going to break lots of sites. rods, make sure that if you do hit return in a text field where it's the only field that the submit button data is _not_ included. That's the once case where it isn't.

adding PDT for review

We'd like to be able to take this one. What's the ETA for getting this reviewed and ready for checkin?

The only difference between the way we are doing it with this patch and IE, is when you click in a text field in IE it sets a "default" focus on the first submit it finds in the form. At this time we don't have a notion of "default" focus for a set of submit buttons. We only have a single focus model. But when the user press <return> in a text field we are now doing the right thing by using the first submit button in the form as the submitter. This site http://www.ajb.dni.us/ now work properly, as does the deadbeef url above.

In IE, you can trap the key event and use it to do other things, then cancel the event so that Enter can be used for moving between fields, which is MUST in financial web applications. In Mozilla 0.9.4, you can't cancel the submit, no matter what you do.

Ron, please file a separate bug about the issue you mentioned.

Comment on attachment 51054 [details] [diff] [review] proposed patch r/sr=kin@netscape.com With the following changes: - Add support for BUTTON_SUBMIT - In nsFormFrame::GetFirstSubmitButtonAndTxtCnt() change the |if|-|if| logic to |if|-|else if|.

I made the changes.

Comment on attachment 51054 [details] [diff] [review] proposed patch r=attinasi

The patch doesn't seem to address the point about highlighting the submit button that will be used if the enter key is hit. Are we already doing that, or are we ignoring that?

check it in - PDT+ kin - pls open another bug for the highlighting.

I filed the default submit button hiliting feature that selmer mentioned as bug 102057. So I guess rod is cleared for landing on the 0.9.4 branch.

checked in on branch and tip

This bug has gone away in build 2001092803. Thanks a lot for all the hard work! It is much appreciated!

No, this fix was incorrect, as shown by the testcase provided...the second to last case, which is undoubtedly the most common (user/password combo), fails. A real world example is aolmail.aol.com, which won't submit on an enter in each textbox.

Also broken is enter in, say, the CC field of a bug page, which submits the form in IE.

Umm...actually, every case except the fourth and latest are failing for me in that testcase.

What exactly doesn't work? Because it works the same for me on Wind2k and Linux and passes all the tests and acts just like IE.

In the aolmail.aol.com case, it looks like IE treats an <input type=img> the same as a submit button, so I think all we have to do to get it working again is modify nsFormFrame::GetFirstSubmitButtonAndTxtCnt to look for the first occurrence of a submit button or an <input type=img>.

r=rods

FYI, I can't reproduce the failure to submit from within the Cc field that blake is seeing in my 09/30/01 Mozilla Win32 debug build ... in fact this comment was submitted by hitting return in the Cc field of this bug.

On SuSE Linux, I also see the problems that Blake described, except that the fifth testcase also works. Most of them fail. It doesn't seem like the new patch would make the testcases work, since the testcases use <input type="submit"> rather than <input type="img">.

On today's build on linux, the first, second, third, and second-to-last test cases do not submit (but say they should). The rest seem to work as they should.

Comment on attachment 51523 [details] [diff] [review] Patch (Adds <input type=img> to list of submit types) sr=attinasi r=rods (from comment)

pls check this in - PD+

fixed

The linux specific issues blake, jeff, and akk are seeing, are puzzling since this code is all XP. I can help look into the issue tomorrow when I have a linux build.

NS_FORM_BUTTON_IMAGE is not defined in any h file. I'm unable to build from cvs currently.

Jon, fix for that is in.

FYI, the comprehensive test case works just fine in my 10/02/01 RedHat 6.2 Debug Mozilla build. I even submitted this bug via a return in the Cc field.

This is still broken for me in 2001100208 on SuSE Linux. Reopening. Is there any information I can provide that would help isolate this? Blake, are you still seeing this?

Which test(s) fail?

I'll try to remember to post a complete list tonight. The first and second ones definitely failed.

I just verfified that with today's tip build on Linux they all work.

This *still* doesn't work properly for me with the 2001100308 trunk build. Here's the complete results of the testcases: 1: Fail 2: Fail 3: Fail 4: Pass - testcase has only one <input> (+ two <input type="radio">s) 5: Pass - testcase has only one <input> 6: Pass - note that passing this testcase means that it *doesn't* submit 7: Pass - note that passing this testcase means that it *doesn't* submit8: Fail 9: Pass - testcase has only one <input> So basically I would summarize my results (excluding testcases 6 and 7) as follows: - If the form contains only one generic <input> field, then it submits when I press Enter. - If the form contains two or more generic <input> fields, then the form does not submit when I press Enter. Blake, if you're out there, can you test this again? Is anyone else still seeing this?

Bugzilla munged my comment a bit--note that testcase 8 fails.

moving to 0.9.6

I see this bug using build 2001100403 on Win98. Here are the testcase results I got: 1. Fail 2. Fail 3. Fail 4. Pass 5. Pass 6. Pass 7. Pass 8. Fail 9. Pass

OK. I have a trunk mozilla 2001-10-03-08 build on Linux. I see the same results as djk and Jeff: 1,2,3,8 Fail 4,5,6,7,9 Pass And I _cannot_ submit by hitting enter in the cc field on this form. The cc field used to work, by the way....

OK. The last build in which testcase #1 works for me is 2001-09-27-08. In 2001-09-27-21 it fails.

This is really strange since all the tests work in both my Win32 and RH 6.2 Linux Mozilla Trunk debug builds from 10/04/01. Are any of you guys building mozilla yourselves? It might be helpful to us if you could put a printf in nsGfxTextControlFrame2::SubmitAttempt() that tells us why it's not being executed.

My debug build passes the testcase. My optimized build fails. Oh, joy. I added the following printf: nsIFrame* submitBtn = mFormFrame->GetFirstSubmitButtonAndTxtCnt(inputTxtCnt); fprintf(stderr, "The button: %p\nThe input count: %d\n", submitBtn, inputTxtCnt); In my opt build I get (first testcase): The button: (nil) The input count: 2 In my debug build I get (first testcase): The button: 0x897a6ac The input count: 2 Looks like the button-getting code fails in opt builds...

Yeah, in my optimized builds the test cases where the button should be read doesn't work at all. They don't submit when you hit return.

Ok, I just attatched a fix ... the problem was due to the fact that a QI call was done inside an NS_ASSERTION macro. I'm even submitting my comment with the return key in the Cc field of this bug with my OPT build. Can we get r=, sr=, a= and pdt on this?

looks good r=rods

Removed PDT+ so it shows up on PDT's radar. PDT: Kin's patch is very simple fix, without it form-submisson is broken in optimized builds.

pls check this into the branch - PDT+

sr=waterson

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

Checked in OPT build fix to the TRUNK: mozilla/layout/html/forms/src/nsFormFrame.cpp revision 3.175 and the MOZILLA_0_9_4_BRANCH: mozilla/layout/html/forms/src/nsFormFrame.cpp revision 3.165.2.5 Mailing drivers to see if they want this on the MOZILLA_0_9_5_BRANCH.

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

I still have questions about the correctness of this patch. I don't think that hitting enter while typing in a summary at http://bugzilla.mozilla.org/enter_bug.cgi?product=Browser should submit that report when I haven't put any comments in the report. IE seems to do the same thing but I'm not sure we should be emulating IE on this one. If you read the original description of this bug it was explicitely reporting a problem with hitting enter in Bugzilla forms causing unwanted submits. It seems strange that the fix for this bug is to return to that undesired state. Either way this doesn't need to go into the 0.9.5 branch.

This feels, looks and clicks wrong. Submitting the form by pressing enter on a bugzilla summary entry feels to me like tricking the user. Are we _positive_ this is the correct fix?

I happen to like IE's behavior, and I'm glad that Mozilla has started emulating it. My preference would be to keep the current behavior. The people who filed dups of this bug would agree with me, I think; also, IIRC, the original bug to add this feature was popular (if somewhat controversial). At the very least, though, no matter what, *PLEASE* make sure that Enter submits the form if there's an <input type="password"> field in the form. I've always found this feature most useful for signing into Web sites quickly.

Then I think it should be a pref. UI in preferences would be something like: Form submit button: * Enter o Ctrl-Enter o None * - being the default.

At the moment it works like IE in debug builds and has for more than a week. This patch merely fixes a optimized build issue, I inadvertently put some important code inside a NS_ASSERTION

Checked in OPT build fix to the MOZILLA_0_9_5_BRANCH: mozilla/layout/html/forms/src/nsFormFrame.cpp revision 3.173.2.1 a=blizzard@mozilla.org

verifying on windows 98 2001-10-12-04-0.9.4

verifying on 2001-10-24-05-trunk