AHAH triggered by text input enter key press, breaks e.g. autocomplete

Reproduced.

Same here, very odd.

This problem first presented on this issue: http://drupal.org/node/214292.

To sum up, I belive that when javascript/jQuery modifies any attribute of a submit button, this button automagically gets promoted to 'defaultButton' of the form (an attribute I don't know how to control), so this shoud be part of a wider patch on D7:

a) Modify form api theme_button to allow the creation of html button type, or
b) Build a wider js function that converts all but real submit type inputs to button type.

I've sent an email to John Resig about this issue to bring some light on it.

If I were to take a guess, I bet that this is the fault of the jquery.form.js file. Post to subscribe. Thanks for looking into it theborg.

Further investigations

I've been using a test form built upon 'search form' to try get the reason the default button is changing. Some things I've found until now:

a) The default button of any form is the first submit button of the DOM inside that form. For that reason when webchick was pushing enter key the 'attach' button was firing.

Solution proposed: To build every form with only two 'official submit buttons' (including reset) either by adding a class/attribute to the defaults and letting theme_button decide or directly modifing all the forms.

b) When using jQuery or even with pure Javascript one submit button is removed, the form losses the default button and IE beeps. Adding another button converts the new one in the default. (weird!) BUT when the button removed is the default, IE assings the next button as default correctly.

Solution proposed: Take a look at collapse.js to see another way of adding the slide effect.

I'm attaching the form I've used to do the tests (is linking jQuery code to the official site).

A related issue is at http://drupal.org/node/218997. Clicking on *any* textfield in a pollform will create a new add more button. Copying from that issue:

On any form that has an AHAH add more button (like a poll), clicking the enter key on any text field in the form will trigger the AHAH behavior and add a new element.

Steps to reproduce:

1) Enable the poll module and create a new poll.
2) Move to the title field and hit the enter button.
3) A new element will be added to the AHAH add more section.

Hitting the enter key in any other textfield in the form will create the same effect.

We also saw the same behavior in the D6 version of CCK when putting autocomplete forms inside an AHAH add more section and then selecting an option by hitting the enter key. Each time you hit the enter key a new add more element is added.

Nedjo is here by me looking at this and thinks the problem is nothing to do with autocomplete but instead that pressing the enter key is triggering a form submission and our javascript is reacting to that.

subscribing

Nedjo came up with a simple one line solution that works for me by changing the AHAH event from 'click' to 'mousedown' in form.inc. Patch attached.

KarenS: does that fire, if you use your keyboard to tab to the button and push the button? Or when you just press enter in a one-field form, like the search block form? (Ie. when a mouse is not involved)?

Well, it is better to test instead of asking :) I tried to reproduce the problem and it indeed appeared without the patch. With the patch however, the "Add another choice" button is flashing but it does not submit. Tested in Firefox 2.0.11 and Safari 3.0.4. The flashing happened in both browser, so it is probably not a browser specific issue, but it is so minor to hold this up.

BUT, with the patch applied, keyboard submission of forms such as the poll form results in the AHAH event firing 5 times (I get 5 more new poll options instead of 1). Without the patch, I only get 1 more option on keyboard submit. So there is indeed a keyboard side effect of this suggestion. (Reproduced in both browsers).

Doh, while taking a shower, I realized that this is probably not the AHAH event firing 5 times, and indeed, it is basically not firing, but the usual form submission happens, which adds 5 new options.

So basically, as I suspected, the use of 'mousedown' instead of 'click' robs off the AHAH forms from keyboard submission. It ties AHAH submission to the mouse. Whether this is acceptable is up for debate. A better, keyboard friendly solution would be ideal IMHO. If nothing like that comes around, all we can do is to commit this for now and then reopen this bug for later fixing (I suspect that's a non-critical bug), since AHAH forms should be usable with the keyboard as well.

As I said in #5, after some tests, the problem appears when the user press the 'enter key'.

I think its about the default button of the form. The enter key triggers submit on the first submit button it finds on the form, and modifing DOM like in collapse alters this behaviour.

The patch in #3 still is valid to these issue, please do some tests.

#3 It works for me on Karen's http://drupal.org/node/218997.

I no longer have Nedjo at my side to comment on this, unfortunately, but but I tried the patch in #3.

The patch fixes the problem in the poll module and also the original problem with the CCK add more button, and also preserves the ability to use the enter key on the button.

I also tried to reproduce the problem in #1 and was able to reproduce the problem without the patch but the patch fixed it.

So I would say this is the right fix and is RTBC unless any of the javascript experts see any reason it might break something else. I am definitely *not* a javascript expert so I don't know that :)

(Very, very, minor, but note that the patch in #3 includes a code comment with the word "behaviour". We had a round earlier before the string freeze that moved these to the US spelling of "behavior". It'll be a couple of hours before I can roll a new patch so I wanted to mention it here.)

fake editor re-roll for the code comment eurospelling.

Rerolled the patch with the spelling fix in #16.

Oops, I tested in Firefox and all was well. Trying now in IE7 it may not be working. I'll post back.

Here's what I found:

In latest Firefox for Windows, everything is broken without the patch and works correctly with the patch.

In IE7 for Windows, there is no problem with the 'add more' button, with or without the patch. Hitting the enter key does not trigger a form submission, so the bad behavior doesn't happen. However, IE 7 does have the problem noted in #1 and the patch has no effect on that.

So the patch will fix some problems for some browsers, and (maybe) has no effect, good or bad, in others. We probably should test in a couple other browsers, but those are the only two I have available.

Note that it is not just the author autocomplete box that triggers the attach button to fire, it will happen if you hit the enter key on any textfield in the form. If you happen to have a file selected in that box, it will get attached.

Nedjo noticed this yesterday when we were trying to figure out what was happening in CCK -- look at where the focus is as you move through the form. It's hard to see in some browsers but is pretty clear in IE7. Create a new story and open up the file attachments section so you can see what is going on. As soon as you click the mouse in the author textfield, the focus changes to the attach button in the file attachments section.

Confirmed, IE7 doesn't accept changing input type. Working on a new patch.

I wasn't able to get the Attach button visibly do anything in Firefox 2 or 3. But I suspect it is what is responsible for a node not being either submitted or previewed on pressing Enter.

The patch in #18 does not fix this. What are we supposed to test?

@Arancaytar. Until we have a better patch you can test this one. It's not working on IE7, but it should do ok with FF.

Sorry, nothing, even after clearing the cache. What should actually happen - should I see a Preview page after pressing Enter in a node textfield, or should the node be submitted?

Nothing should happen after pressing enter in a node textfield, apart from maybe a line break, or selecting from autocomplete options. Am I missing something?

#1 and #6 describe two situations where the javascript is firing on the wrong action. In both of these situations, hitting the enter key in any textfield on the form triggers the javascript button action.

We're trying to find a patch that will fix this. You need to recreate the problems in #1 and #6, apply the patch, and see if it fixes them, once we have a patch that might work.

I still think the focus question is important. You can't see where the focus is in Firefox, but you can see it in IE7 and hopefully there is some Mac browser where you can see when the button has focus, or you can tweak your css somehow so it shows you which element is active. Clicking in the author textfield puts focus on the 'attach' button (which it obviously should not). Once the focus is there, hitting the enter key is obviously going to fire the button.

#1 and #6 describe two situations where the javascript is firing on the wrong action. In both of these situations, hitting the enter key in any textfield on the form triggers the javascript button action.

How can I see this javascript action being triggered? Because nothing visible happens on the page for me. Sorry if I'm missing the point here...

Reproduced and patch fixes behaviour in FF, Safari 3 and Opera on XP.

On IE7 all my fieldsets are collapsed and won't expand. Get this error: Line 30, Character 9, could not get the type property, command is not supported

edit: Arancaytar - you need to expand both the authoring information and upload fieldsets. Then use the authoring autocomplete, then enter to select a username - then you'll see it. Patch stops this from happening on everything except IE.

Finally I was able to reproduce this! It actually happens in IE 6.0. However, I do not see this behavior in Firefox - neither in Firefox 2.0.0.11 nor in Firefox 3.0b2. The site is unpatched. Weird? I'll try again on another computer.

Arancaytar: many of us reproduced it in Firefox and other browsers as well, so it looks quite weird.

(Not to get off track, but in Arancaytar's defense, I can't reproduce the file attach thing either using Firefox 2.0.0.11 on Vista. I can reproduce the poll thing in Firefox.)

I can reproduce both and problem #1 also occurs in Drupal 5. Both problems could be fixed if we had a way to create a context around each button then limit key presses to that scope. ie. this button belongs to these fields.. Not quite sure how it would work though.

As theborg already mentioned. This will *always* happen when there are multiple submit buttons and will default to the first all the time. We need smarter button handling.

Only in IE type attribute is read only, so I've built a button from scratch with the attributes of the submit button to convert.
Cloning attributes in IE is also a nightmare!, the only way I found is with a propietary IE function.

After cloning the submit button and coverting it to a button type, another problem presents:
The 'default button' enter keypress behavior is lost as in #5.
So, I am cloning the 'edit-submit' button of the form to activate it.

After lots of trial and error with the IE doomed DOM, lets mark this to review.

The general issue of triggering unwanted submits by hitting enter in a text input is commonly hit. Various approaches have been tried, including

* attach a keypress event handler to the form or to text inputs and prevent submission if the enter key was pressed
* attach a submit event handler to the form returning false (preventing submission) and a click handler to submit buttons programmatically submitting the form

Both of these seem involved and likely to trigger conflicts with other behaviours.

I'm not sure we're going to find a single solution that covers all cases (JS or not, etc.). For Javascript purposes. theborg's approach feels appropriate. We convert our submit button into a "push button", http://www.w3.org/TR/html4/interact/forms.html#push-button--that's what is is, a button to trigger a Javascript action.

Let's try this (I don't have a test site handy to do so):

      if ($(element_settings.element).attr('type') == 'submit') {
        $(element_settings.element).attr('type', 'button');
      }

Not working on IE:
MSDN docs:
"As of Microsoft Internet Explorer 5, the type property is read/write-once, but only when an input element is created with the createElement method and before it is added to the document."

jQuery clone() it doesn't work.

Here's a roughed in, untested draft of what we might need to do.

Changes:

1. The logic needs to be within the iteration loop, since we need to apply it to each element.

2. If we have a select element, we clone it, change the type attribute, append the clone, then remove the original.

Whether cloning convinces IE we should be able to change the type I don't know.

I've heard anecdotally the clone method sometimes fails in IE7.

More descriptive title.

@nedjo. #38 is not working on IE6/7 it gives and exception on *changing type*.

I did the same in #24, for non-IE browsers changing type of inputs is the only thing needed.

#34 still applies.

Sorry, I didn't want to change the title.

Fix a couple of problems: Need to set the new element as the one to attach AHAH to. Still untested.

Even if it turns out to work, this cloning and appending feels clunky.

Of course, if we were only concerned with JS, we could render a button in the first place. (It's always seemed counter-intuitive to me that we have a Forms API element of type 'button' that renders, not as a button, but as a submit input, like, um, the 'submit' element.) But for non-JS usage we need the submit button.

We could consider testing for JS support on the server and rendering a button accordingly, but that would introduce a whole new set of issues (what if a user comes straight to the AHAH page and doesn't yet have a JS cookie flag? etc.).

Tested on IE6 and this code .attr('type', 'button') throughs an exception.

@theborg, thanks for reporting back.

#34 is enough to establish that hacking to try to get IE to work is not our approach.

I wonder if we can work with the event target--determine if the submit button was the target of the event, don't trigger ajaxSubmit if not. I'll try.

I tried the event target approach, but ran into another IE wall. Here's the patch I tried on ahah.js:

-  $(element_settings.element).bind(element_settings.event, function() {
-    $(element_settings.element).parents('form').ajaxSubmit(options);
-    return false;
+  $(element_settings.element).bind(element_settings.event, function(event) {
+    // Ensure this is the target element of the event.
+    // This test prevents e.g. submits triggered by text input enter
+    // key presses.
+    if (event.target == element_settings.element) {
+      $(element_settings.element).parents('form').ajaxSubmit(options);
+      return false;
+    }

Firefox nicely recognized the text input as the event target, so skipped ajax submission. But good old IE insisted the target was the submit input, so no dice.

So here's a different approach. We stick with the 'mousedown' event, which is working okay so far as it goes. Then we address the keyboard entry use case. I've done this by adding a new setting in the array we're passing, keypress. If this flag is set, ahah.js adds an additional event handler to watch for keypress and, if it was the enter key pressed, trigger the ajaxSubmit (by triggering the mousedown event).

Tested on IE and Firefox, seems to be working.

I'm not totally happy with the result. I'd really prefer not to be messing around with mousedown and keypress events. But maybe this is the cleanest we can come up with. At least it's a single cross-browser solution that doesn't resort to IE hacking.

Patch #45 applies to includes/form.inc with 2 lines offset, but no other problems.

Testing in FF 2.0.0.11, FF 3.0b4pre, Opera 9.24, IE6. All on Win XP SP2.

First bug:

1.) Enter /node/add/page
2.) Expand File attachment and Authoring info
3.) Enter Author field, allow autocomplete to trigger, press Cursor-Down, Enter to select a name, Enter again to submit.

Bug A behavior: File attachment AHAH visibly triggers on both Enter presses.
Bug B behavior: File attachment does not trigger on first Enter to select, but does on second Enter to submit. The form still cannot be submitted by pressing Enter.

Correct behavior: The author info field gets filled with no other visible results; the form gets submitted on the second Enter press.

Test results:

             Pre-patch     Post-patch
FF 2.0.0.11        Correct       Correct
FF 3.0b4pre        Correct       Correct
Opera 9.24         Bug B         Correct
IE 6               Bug A         Correct*

*The "Attach" button visibly gets focused, but nothing else happens so we can let this slide.

Second bug:

1.) Enter /node/add/poll
2.) Focus the node title field
3.) Press enter

Bug A behavior: The AHAH triggers to add more elements to the poll.
Bug B behavior: The browser refuses the Enter keypress as an invalid command, accompanied by an alert sound.
Correct behavior: The node form is submitted. Note that 5 more choices are statically added by the page when reloading the form (in order to gracefully degrade).

             Pre-patch     Post-patch
FF 2.0.0.11        Bug A         Correct
FF 3.0b4pre        Bug A         Correct
Opera 9.24         Bug A         Correct
IE 6               Bug B         Bug B

A pox on that crappy browser. >_<

Note a buggy behavior observed for both tests in all browsers, pre and post patch: Submitting the form by pressing Enter sends a query to the server, but does not actually do anything beyond validating the form. None of the submit hooks get triggered. It should at least trigger the Preview action as a default. I think this is a bug, though perhaps not a critical one, and certainly an unrelated one.

@Arancaytar: Thanks for this detailed tests!

I've also seen the bad behaviour you noted on comments only validating but no submitting the form when pressing enter key. This is another bug.

With #46 IE bug B is not solved, probably because IE looks for the default submit (the attach button) to trigger the form.

@Arancaytar: yes, thanks for the detailed analysis!

On the second (poll) bug, I don't get bug B. For me in IE 6 the pre-patch bug is the same in IE an in FF - the AHAH triggers to add more elements to the poll - and is resolved by the patch at #45. Could this issue be dependent on the version of IE 6?

It's not clear how or even if bug B, when it does occur, is related to the AHAH behaviour.

Let it be noted that my version of IE is at least partially broken (well, more broken...), and my memory was bloated at the time of the test. I'll try again to make sure, and even if I do get it, I wouldn't let this stand unless someone else can reproduce it. We've established that my computer is acting weird...

... unfortunately I've reproduced it both on my machine and another. Please try to add a poll at my test site (no registration needed, guests can create polls) to compare results.

Patch works for me in FF2, IE7 and IE6 - also tested in opera. If I keep hitting enter multiple times in the empty 'authored on' text field, I can get it to start adding poll options again, but that'd be a pretty stupid thing to do.

So since this fixes the worst aspects of both bugs, and anything else is minor (and possibly separate issues), I think it's ready.

I tried the patch in #45 in both FF and IE7. Everything works as expected in FF -- pressing enter on a poll choice field generates a full form submission with five new fields, pressing enter on autocompletes in either the author or CCK add more forms selects the value without triggering the AHAH.

In IE7, everything works as above except that pressing enter in a poll choice triggers a warning sound instead of a form submission (seems to be the same as IE6 behaves as reported above). I checked that the form does not lose its ability to be submitted by hitting enter on the submit button, it just won't submit if you hit enter in a textfield.

I tested both FF and IE 7 to be sure that submission with the keyboard instead of the mouse still works right, both for form submission and the AHAH button submission, and it seems to work correctly.

Seems like a reasonable solution that fixes most problems, with the last one being very minor (that IE won't submit the form by hitting the enter key in a textfield). You could even make an argument that the form should not submit when you hit the enter key on the poll choice, so I don't know that that is even a problem.

Should we add some more explanation to the code to explain why this has been done so someone doesn't decide later to clean this 'unnecessary' code out? I'd suggest some, but I doubt I know how to explain it right.

I've added some code comments and also fixed a small error (I'd called the property keydown instead of keypress).

From the looks of the latest change, it seems that the keyboard events were not tested (the fixed code fixed that).

The bugs described were fixed by the time I tested. What is the keypress event supposed to change in the apparently working behavior? If I know what to look for, I can repeat my test tonight and include Kestrel in the list of tested browsers.

Tested in FF2, IE7, Opera 9.25 and 9.5beta - works as advertised in all of them, for both bugs.

Thanks for retesting catch. Committed #54 to Drupal 6.x. Still RTBC for 7.x.

Committed to CVS HEAD. Thanks.

The fix here introduced its own problems, and still leaves some questions. Please help out on #634616: Various problems due to AJAX binding to mousedown instead of click if you're interested.