Bug 39532 - REGRESSION: Enter does not trigger submit of forms when focus is on select.
Summary: REGRESSION: Enter does not trigger submit of forms when focus is on select.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P1 Normal
Assignee: Dimitri Glazkov (Google)
URL:
Keywords: InRadar
: 41379 43064 (view as bug list)
Depends on: 9756
Blocks: 39021
  Show dependency treegraph
 
Reported: 2010-05-22 09:06 PDT by Martin Häcker
Modified: 2010-11-24 11:09 PST (History)
16 users (show)

See Also:


Attachments
reduction (768 bytes, text/html)
2010-05-22 09:06 PDT, Martin Häcker
no flags Details
Second try with js this time (901 bytes, text/html)
2010-05-23 02:56 PDT, Martin Häcker
no flags Details
Patch (6.74 KB, patch)
2010-06-24 13:40 PDT, Dimitri Glazkov (Google)
no flags Details | Formatted Diff | Diff
Patch (9.08 KB, patch)
2010-07-02 10:26 PDT, Dimitri Glazkov (Google)
ap: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Häcker 2010-05-22 09:06:01 PDT
Created attachment 56780 [details]
reduction

See the included reduction, to trigger it choose an option from the popup and hit enter with the focus still on the form element.

It should now submit (that is reload the page as no submit url is specified in the form) which you should see because the default choice is again chosen instead of what you chose in the popup.

It doesn't however - the form is not submitted.
Comment 1 Alexey Proskuryakov 2010-05-22 10:45:42 PDT
The new behavior matches Firefox. Why it it wrong? Does the change affect any existing Web sites?
Comment 2 Dimitri Glazkov (Google) 2010-05-22 18:04:18 PDT
For what it's worth the new behavior matches IE, too.
Comment 3 Martin Häcker 2010-05-23 02:37:22 PDT
This is strange - I haven't seen this behaviour in Firefox or IE when I found this bug in our application, so probably my reduction reduces the problem too much. I'll try to give a more complete reduction that shows the problem (but that will involve javascript).
Comment 4 Martin Häcker 2010-05-23 02:56:01 PDT
Created attachment 56817 [details]
Second try with js this time

This is the initial breakage that I detected - this time it only happens on webkit nightlies
Comment 5 Martin Häcker 2010-05-23 02:59:18 PDT
I've added a second reduction - this time with more js (sorry about that). To see the problem:

1. click 'Yeehaw'
2. Choose a value from the popup
3. while the focus is still on the popup, press enter and

4. see that the form does not trigger the js submit handler that is attached.

This is the breakage that got introduced into my app - luckily my testsuite captured it early. My workaround for now is to attach a js key-handler to the select field that listens to the enter key. (Not nice, but good enough for our users for now).
Comment 6 Martin Häcker 2010-05-24 04:04:25 PDT
I just wanted to add that in the second reduction you can see how WebKit Nightlies behave different to Firefox and IE which both trigger the submit handler that is attached.
Comment 7 Dimitri Glazkov (Google) 2010-05-24 10:01:59 PDT
(In reply to comment #6)
> I just wanted to add that in the second reduction you can see how WebKit Nightlies behave different to Firefox and IE which both trigger the submit handler that is attached.

Are you sure about IE? I can't make it trigger using your test. Can we reduce it a bit better -- preferably without the libraries?
Comment 8 Alexey Proskuryakov 2010-05-24 12:04:05 PDT
Further reducing the test is of course our job as WebKit developers, as long as we have a test case that truthfully reproduces the problem. But your help with reducing problems is greatly appreciated!
Comment 9 Dimitri Glazkov (Google) 2010-05-24 12:05:43 PDT
(In reply to comment #8)
> Further reducing the test is of course our job as WebKit developers, as long as we have a test case that truthfully reproduces the problem. But your help with reducing problems is greatly appreciated!

Shhh.. Don't tell him that it's our job! :)
Comment 10 Indrek Siitan 2010-06-20 01:49:27 PDT
Actually, this has regressed half-way - using the enter key still works while the focus is on an <input type="text"> element, doesn't on <select>.

I personally strongly vote for reverting to old behaviour - using web-based apps a lot it's quite annoying to lift your hand from keyboard to the mouse to click the submit button.
Comment 11 Indrek Siitan 2010-06-20 01:52:14 PDT
And yes, on Firefox enter doesn't work on <select> either, but on the other hand you cannot "tab" onto a select field using a keyboard on Firefox at all - something that can be done on Safari.
Comment 12 Dimitri Glazkov (Google) 2010-06-20 08:16:21 PDT
(In reply to comment #10)
> Actually, this has regressed half-way - using the enter key still works while the focus is on an <input type="text"> element, doesn't on <select>.
> 
> I personally strongly vote for reverting to old behaviour - using web-based apps a lot it's quite annoying to lift your hand from keyboard to the mouse to click the submit button.

Heh. I made this change intentionally to match the behavior of other browsers -- by request from web developers! See the long and passionate debate on bug 9757. It seems that I can't please everyone :)

Seems like we need to find a compromise -- a new behavior that fixes old compat problems and minimizes the user.
Comment 13 Dimitri Glazkov (Google) 2010-06-20 08:18:34 PDT
(In reply to comment #12)
> (In reply to comment #10)
> > Actually, this has regressed half-way - using the enter key still works while the focus is on an <input type="text"> element, doesn't on <select>.
> > 
> > I personally strongly vote for reverting to old behaviour - using web-based apps a lot it's quite annoying to lift your hand from keyboard to the mouse to click the submit button.
> 
> Heh. I made this change intentionally to match the behavior of other browsers -- by request from web developers! See the long and passionate debate on bug 9757. It seems that I can't please everyone :)
> 
> Seems like we need to find a compromise -- a new behavior that fixes old compat problems and minimizes the user.

I meant bug 9756 :)
Comment 14 Dimitri Glazkov (Google) 2010-06-22 09:31:35 PDT
(In reply to comment #11)
> And yes, on Firefox enter doesn't work on <select> either, but on the other hand you cannot "tab" onto a select field using a keyboard on Firefox at all - something that can be done on Safari.

You can by default on non-OS X platforms. On OS X, see here: http://support.mozilla.com/en-US/kb/Pressing+Tab+key+does+not+select+menus+or+buttons
Comment 15 Dimitri Glazkov (Google) 2010-06-22 09:38:29 PDT
I am torn on this. On one hand, only Opera triggers form submission when hitting enter on select box. All other browsers ignore the keystroke. Being consistent across browsers helps compatibility, especially in the corner cases like this.

On the other hand, we've changed the expectations of the users. The patch to change this is trivial, but I would like some input from WebKit engineers.
Comment 16 Indrek Siitan 2010-06-22 09:43:27 PDT
My $0.02 worth of pros for reverting to old behaviour:

 * Safari users like me are used to the behaviour in 4.0.

 * I think it makes more sense to be consistent with the concept, rather than other browsers - if you can use keyboard to focus on a form element, you should be able to hit enter to submit the form. Should work the same way for both input fields and selects. And, for all other elements (such as radio-/checkboxes) if I have "tab to all controls" set from my System Prefs.
Comment 17 Dimitri Glazkov (Google) 2010-06-23 14:49:04 PDT
I think I'll go ahead and change this back, per IRC discussion.
Comment 18 Martin Häcker 2010-06-24 10:14:54 PDT
To add another data point: if a select field has a submit js handler (applied vie jquery), the web application probably expects the user to submit that form field somehow and that should be supported.

Regards,
Martin
Comment 19 Dimitri Glazkov (Google) 2010-06-24 13:40:10 PDT
Created attachment 59690 [details]
Patch
Comment 20 Alexey Proskuryakov 2010-06-25 11:45:23 PDT
Comment on attachment 59690 [details]
Patch

+    [ "Select with a submit", "!select,submit", "y" ], // match neither FF nor IE, instead follow logic.
+    [ "Select with a disabled submit", "!select,-submit", "n" ],

Can we have tests for both select variants (popup and box)?
Comment 21 Adele Peterson 2010-06-28 22:40:57 PDT
<rdar://problem/8076104>
Comment 22 Dimitri Glazkov (Google) 2010-06-29 08:19:18 PDT
(In reply to comment #20)
> (From update of attachment 59690 [details])
> +    [ "Select with a submit", "!select,submit", "y" ], // match neither FF nor IE, instead follow logic.
> +    [ "Select with a disabled submit", "!select,-submit", "n" ],
> 
> Can we have tests for both select variants (popup and box)?

Sure thing! I'll upload a new patch.
Comment 23 Alexey Proskuryakov 2010-06-29 21:53:16 PDT
*** Bug 41379 has been marked as a duplicate of this bug. ***
Comment 24 patrick 2010-06-30 14:58:33 PDT
(In reply to comment #16)
> My $0.02 worth of pros for reverting to old behaviour:
> 
>  * Safari users like me are used to the behaviour in 4.0.
> 
>  * I think it makes more sense to be consistent with the concept, rather than other browsers - if you can use keyboard to focus on a form element, you should be able to hit enter to submit the form. Should work the same way for both input fields and selects. And, for all other elements (such as radio-/checkboxes) if I have "tab to all controls" set from my System Prefs.

I agree with Indrek's explanation.  The primary reason we use Safari as the primary browser within our work Intranet is this consistent behavior where the enter key always submits a form.  

Most of our staff at work have multi-button mice with one of the buttons programmed as the enter key.  When making a drop down selection and wanting to submit the form, all they had to do was make a selection, and then hit their secondary mouse button to submit the form.  Now they have to either switch to a text box and then hit enter, or use the mouse to find the submit button, which usually requires scrolling down the page.  This new behavior dramatically slows down productivity.
Comment 25 Dimitri Glazkov (Google) 2010-07-02 10:26:54 PDT
Created attachment 60377 [details]
Patch
Comment 26 Dimitri Glazkov (Google) 2010-07-02 10:27:42 PDT
(In reply to comment #20)
> (From update of attachment 59690 [details])
> +    [ "Select with a submit", "!select,submit", "y" ], // match neither FF nor IE, instead follow logic.
> +    [ "Select with a disabled submit", "!select,-submit", "n" ],
> 
> Can we have tests for both select variants (popup and box)?

New patch up. Thanks for pointing that out!
Comment 27 Alexey Proskuryakov 2010-07-02 10:49:07 PDT
Comment on attachment 60377 [details]
Patch

> +		} else if (type == "selectBox") {
> +			control = document.createElement("select");
> +			control.size = 5;
> +			control.options.add(new Option("a"));

Not that I have anything against tabs in patches...

> +    } else if (event->type() == eventNames().keypressEvent) {

So, is it indeed keypress that triggers submission, not keydown?

r=me
Comment 28 Dimitri Glazkov (Google) 2010-07-02 10:58:06 PDT
(In reply to comment #27)
> (From update of attachment 60377 [details])
> > +		} else if (type == "selectBox") {
> > +			control = document.createElement("select");
> > +			control.size = 5;
> > +			control.options.add(new Option("a"));
> 
> Not that I have anything against tabs in patches...

Argh -- I've forgotten to update my TextMate settings -- thanks for catching that.

> > +    } else if (event->type() == eventNames().keypressEvent) {
> 
> So, is it indeed keypress that triggers submission, not keydown?

Yep -- btw, here I am just restoring the code I removed.

> r=me
Comment 29 Dimitri Glazkov (Google) 2010-07-02 11:07:39 PDT
Landed with fixups as http://trac.webkit.org/changeset/62391.
Comment 30 WebKit Review Bot 2010-07-02 11:23:05 PDT
http://trac.webkit.org/changeset/62391 might have broken Qt Linux Release
Comment 31 Martin Robinson 2010-07-02 13:55:35 PDT
This fails on GTK+ for select boxes still. I believe it would work if the logic added to SelectElement::listBoxDefaultEventHandler was added to  SelectElement::menuListDefaultEventHandler, as GTK+ seems to follow that path at the conditional at line 797 in SelectElement.cpp. I'm not sure if this is appropriate, but I'm pretty sure this is why it's failing.
Comment 32 Dimitri Glazkov (Google) 2010-07-02 15:20:22 PDT
(In reply to comment #31)
> This fails on GTK+ for select boxes still. I believe it would work if the logic added to SelectElement::listBoxDefaultEventHandler was added to  SelectElement::menuListDefaultEventHandler, as GTK+ seems to follow that path at the conditional at line 797 in SelectElement.cpp. I'm not sure if this is appropriate, but I'm pretty sure this is why it's failing.

I ended up just updating the expectations here: http://trac.webkit.org/changeset/62409
Turns out this behavior just is different across ports -- the ifdefs are specifically there to delineate the differences,
Comment 33 Alexey Proskuryakov 2010-11-24 11:09:54 PST
*** Bug 43064 has been marked as a duplicate of this bug. ***