bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

<select> element shouldn't consume key events which don't cause any default action

VERIFIED FIXED in Firefox 29

Status

()

Core
Event Handling
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: Liviu PADURARU, Assigned: masayuki)

Tracking

(Depends on: 1 bug, 4 keywords)

25 Branch
mozilla29
x86
All
dev-doc-complete, regression, reproducible, site-compat
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox26 wontfix, firefox27 wontfix, firefox28 wontfix, firefox29 fixed, firefox-esr24 unaffected)

Details

(Whiteboard: [bugday-20131111])

Attachments

(5 attachments, 3 obsolete attachments)

(Reporter)

Description

5 years ago
User Agent: Mozilla/5.0 (Windows NT 5.1; rv:25.0) Gecko/20100101 Firefox/25.0 (Beta/Release)
Build ID: 20131025151332

Steps to reproduce:

Not working in Mozilla Firefox 25.0:

<html>
<head>
<title>Test</title>
</head>
<body>
<form name="forma" method="get" action="test1.php">
Test <select onkeypress="forma.submit()" size="2"><option>Da</option><option>Nu</option></select>
</form>
</body>
</html>

Comment 1

5 years ago
Works for me on 25: the form submits when I press a key while the select is focused.  Please try to reproduce in safe mode with all add-ons disabled: https://support.mozilla.org/en-US/kb/troubleshoot-firefox-issues-using-safe-mode
Flags: needinfo?(lpaduraru)
(Reporter)

Comment 2

5 years ago
1. It does not function either in safe-mode.
2. The problem is that it does not function when I press <Enter>, but it works when I press <Tab>.
3. The whole code functions properly when I press <Enter> in other Firefox versions or IE.
4. The code functions properly in Firefox 25.0 if only I modify size="2" in size="1".

Comment 3

5 years ago
Created attachment 830076 [details]
testcase

Comment 4

5 years ago
Reproduced comment #2 with 25.0 and 2013-11-10-03-02-05-mozilla-central-firefox-28.0a1.en-US.linux-x86_64

wfm: 2013-07-24-03-02-04-mozilla-central-firefox-25.0a1.ru.linux-x86_64    2983ca6d4d1a
bug: 2013-07-25-17-15-58-mozilla-central-firefox-25.0a1.en-US.linux-x86_64 76fb8f815ac7

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2983ca6d4d1a&tochange=76fb8f815ac7
Blocks: 501496
Status: UNCONFIRMED → NEW
Component: Untriaged → Event Handling
Ever confirmed: true
Flags: needinfo?(lpaduraru)
Keywords: reproducible
OS: Windows XP → All
Product: Firefox → Core
Whiteboard: [bugday-20131111]
See bug 903715. We assume that nobody expect "double action" for a key operation. Therefore, if keydown event is consumed by the <select>'s default action, keypress event is never fired anymore.

If web apps need to override the behavior, they should handle keydown event.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → INVALID
Is bug 903715 relevant? The testcase of this bug contains no dropdown (because of "size=2").
(Reporter)

Updated

5 years ago
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Created attachment 830166 [details]
testcase

At least Chrome and IE11 didn't prevent default action by default if the dropdown is closed or no dropdown is present from the start. I support reopening.
Let's sort out what's the problem of this bug.

If you think that keypress event should be fired even if a default action occurs, I don't agree with this. See bug 903925. Such behavior causes "double action". I don't think that it's better for users.

If you think that keypress event should be fired if any default action doesn't occur, it might be better.

I understood this bug is as the former. It this the latter?
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #8)
> Let's sort out what's the problem of this bug.
> 
> If you think that keypress event should be fired even if a default action
> occurs, I don't agree with this. See bug 903925. Such behavior causes
> "double action". I don't think that it's better for users.

I'm saying the "default action" for <select> elements should not submit the form. Chrome for Windows, IE11, and older versions of Firefox didn't prevent the default action, but also didn't submit. (Try the testcase if you don't believe.)
The "double action" will not occur because the "default action" will not submit the form itself.
Created attachment 830827 [details]
testcase

Added an <input> element for comparison.
Attachment #830166 - Attachment is obsolete: true
(In reply to Masatoshi Kimura [:emk] from comment #9)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #8)
> > Let's sort out what's the problem of this bug.
> > 
> > If you think that keypress event should be fired even if a default action
> > occurs, I don't agree with this. See bug 903925. Such behavior causes
> > "double action". I don't think that it's better for users.
> 
> I'm saying the "default action" for <select> elements should not submit the
> form. Chrome for Windows, IE11, and older versions of Firefox didn't prevent
> the default action, but also didn't submit. (Try the testcase if you don't
> believe.)

So, do you agree with only the latter case should be fixed?

I should explain the background of current behavior.

The reason why bug 501496 moves a part of default action handler from keypress to keydown is for more serious compatibility with the other browsers. On the other browsers, even if preventDefault() of a keydown event is called, it doesn't cause preventing default action of <select> element. Therefore, our old code ignored defaultPrevented value intentionally.
http://mxr.mozilla.org/mozilla-central/source/layout/forms/nsListControlFrame.cpp?rev=a03c584f9874#2078

If we still used keypress event handler for handling default action, a call of preventDefault() of keydown would cause only our users cannot operate <select> element on such page because of no keypress event. (E.g., user may not be able to close dropdown on such page!)

By this change, we met bug 903715. The cause of the bug is, not preventing default of keydown event during dropdown open causes dispatching keypress event on new focused <input> element and it causes submitting the form. This bug also has Chrome for Windows and IE but not so Chrome for Mac and Safari. So, the other browsers behave inconsistent between platforms. We chose the better behavior, Mac version.

This is the reason why I don't agree with fixing the former case of my previous comment.

I believe that we should fix only the latter case of it.
Anyway, it's not clear what is a reported problem from comment 0. It looks like all keys should kick the keypress event handler. Then, the request is INVALID as I decided because it's by design mentioned in comment 11.
Hm, I must have been screwed up.
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → INVALID
(Reporter)

Comment 14

5 years ago
File test.html
--------------
<html>
<head>
<title>Test</title>
</head>
<script type="text/javascript">
function enter(myfield,e)
{
var keycode;
if (window.event)
  keycode=window.event.keyCode;
else
  if (e)
    keycode=e.which;
  else
    return true;
if (keycode==13)
  return false;
else
  return true;
}
function submitenter(myfield,e)
{
var keycode;
if (window.event)
  keycode=window.event.keyCode;
else
  if (e)
    keycode=e.which;
  else
    return true;
if (keycode==13)
{
  myfield.form.submit();
  return false;
}
else
  return true;
}
function dezactiveazaenter(evt)
{
var k=evt.keyCode || evt.which;
return k!=13;
}
</script>
<body onload="document.getElementById('s1').focus()">
<form name="forma" method="get" action="test1.php">
Test <select name="s1" id="s1" onkeypress="if (!enter(this,event)) { forma.submit(); return dezactiveazaenter(event); }" size="2"><option>Da</option><option>Nu</option></select>
</form>
</body>
</html>

==================================

File test1.php
--------------
<?php
echo $_GET["s1"];
?>

==================================

Not working in Mozilla Firefox 25.0, but it works in IE, for example.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Thank you, I recalled what was the problem I thought here.

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #8)
> If you think that keypress event should be fired even if a default action
> occurs, I don't agree with this. See bug 903925. Such behavior causes
> "double action". I don't think that it's better for users.

To be clear, I'm not proposing this.

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #11)
> On the other browsers, even if preventDefault() of a keydown event
> is called, it doesn't cause preventing default action of <select> element.

It is untrue at least on Chrome for Windows and IE 11 if the dropdown is not open/present and Enter key is pressed. If preventDefault() of the keydown event is called, both browsers prevented the default action (if my understanding is correct, firing a keypress event is a part of the default action of the keydown event).

> If we still used keypress event handler for handling default action, a call
> of preventDefault() of keydown would cause only our users cannot operate
> <select> element on such page because of no keypress event. (E.g., user may
> not be able to close dropdown on such page!)

At least on Chrome for Windows and IE 11, I couldn't operate <select> elements if I called preventDefault() in the keydown event handler. I can agree that it is a bad UX, but our new behavior is not "for more serious compatibility with the other browsers" because it is less compatible with other browsers.

>  not preventing default of keydown event during dropdown open

Again, I'm saying about the case of <select> elements without dropdown. We shouldn't prevent the default action in this case because it looks like that Enter key has no default actions (except for firing a keypress event) for the default keydown handler of <select> elements.
To summarize what I'm proposing, we should fire a keypress event if we meet all following conditions:
* The dropdown menu of the <select> element is close or is not present at all,
* the Enter key is pressed,
* and the keydown handler didn't call preventDefault() to prevent the default action.

Nakano-san, do you have any other unclear points?
Flags: needinfo?(masayuki)
(In reply to Masatoshi Kimura [:emk] from comment #15)
> Thank you, I recalled what was the problem I thought here.
> 
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #8)
> > If you think that keypress event should be fired even if a default action
> > occurs, I don't agree with this. See bug 903925. Such behavior causes
> > "double action". I don't think that it's better for users.
> 
> To be clear, I'm not proposing this.

Okay, then this case is really INVALID as I said.

> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #11)
> > On the other browsers, even if preventDefault() of a keydown event
> > is called, it doesn't cause preventing default action of <select> element.
> 
> It is untrue at least on Chrome for Windows and IE 11 if the dropdown is not
> open/present and Enter key is pressed. If preventDefault() of the keydown
> event is called, both browsers prevented the default action (if my
> understanding is correct, firing a keypress event is a part of the default
> action of the keydown event).

Okay, and yes, your understanding is correct.

> > If we still used keypress event handler for handling default action, a call
> > of preventDefault() of keydown would cause only our users cannot operate
> > <select> element on such page because of no keypress event. (E.g., user may
> > not be able to close dropdown on such page!)
> 
> At least on Chrome for Windows and IE 11, I couldn't operate <select>
> elements if I called preventDefault() in the keydown event handler. I can
> agree that it is a bad UX, but our new behavior is not "for more serious
> compatibility with the other browsers" because it is less compatible with
> other browsers.

Indeed. But I don't want to accept the "bad UX" at least in critical cases (especially, cannot close dropdown is system-wide attack). And I think that this is out of scope of this bug. If we need to "improve" this behavior, let's separate bug.

> >  not preventing default of keydown event during dropdown open
> 
> Again, I'm saying about the case of <select> elements without dropdown. We
> shouldn't prevent the default action in this case because it looks like that
> Enter key has no default actions (except for firing a keypress event) for
> the default keydown handler of <select> elements.

(In reply to Masatoshi Kimura [:emk] from comment #16)
> To summarize what I'm proposing, we should fire a keypress event if we meet
> all following conditions:
> * The dropdown menu of the <select> element is close or is not present at
> all,
> * the Enter key is pressed,
> * and the keydown handler didn't call preventDefault() to prevent the
> default action.
> 
> Nakano-san, do you have any other unclear points?

Only Enter key case? I don't think that it makes sense. If we need to change the behavior, I believe that we should not prevent following keypress events of all keydown events which don't cause any default action. I mean, we should change the behavior as: if default action handler of <select> doesn't handle keydown event, it shouldn't call preventDefault(). Isn't this your suggest?
Flags: needinfo?(masayuki)
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #17)
> Indeed. But I don't want to accept the "bad UX" at least in critical cases
> (especially, cannot close dropdown is system-wide attack).

It's impossible that users can't close dropdown because the dropdown is not open :)

> Only Enter key case? I don't think that it makes sense. If we need to change
> the behavior, I believe that we should not prevent following keypress events
> of all keydown events which don't cause any default action. I mean, we
> should change the behavior as: if default action handler of <select> doesn't
> handle keydown event, it shouldn't call preventDefault(). Isn't this your
> suggest?

Yes, Enter key is just an example. We should investigate what keys will consume the event and what keys will not. At least arrow keys and tab key didn't fire keypress events on other browsers (and obviouly they have default actions).
Note that some keys (including Enter) will consume keydown events only if the dropdown is open.
Okay, we now get agreement how we should fix this bug, changing the summary.
Summary: select onkeypress size → <select> element shouldn't consume key events which don't cause any default action
Hmm, if multiple attribute is specified, Enter key is used for selecting only focused option. So we can change the behavior only when <select size="n"> (n>=2).
Comment hidden (me-too)
(In reply to Liviu PADURARU from comment #21)
> The bug has not been resolved either FF 25.0.1.

Please don't post such spam comment. You should check the bug status.
Comment hidden (me-too)
Liviu:

Please don't post such spam comment.
Depends on: 962029
Created attachment 8362907 [details] [diff] [review]
part.1 Don't consume key event on <select> element if it's never handled
Assignee: nobody → masayuki
Status: REOPENED → ASSIGNED
Created attachment 8362908 [details] [diff] [review]
part.2 Add new tests
Created attachment 8362909 [details] [diff] [review]
part.2 Add new tests
Attachment #8362908 - Attachment is obsolete: true
Comment on attachment 8362907 [details] [diff] [review]
part.1 Don't consume key event on <select> element if it's never handled

Unfortunately, the newest part.2 patch in my tree hasn't been tested with B2G. However, this is a regression. Let's fix first. I'd like to land this during 29 cycle.

This patch doesn't call preventDefault() if the key is never handled with the <select> element instance. If we call preventDefault() only when the key event does something default action (e.g., except arrow keys which doesn't actually change selected item), web apps may be confused because it really depends on our behavior change. Therefore, I believe that we should always call preventDefault() if the key event may cause default action.
Attachment #8362907 - Flags: review?(enndeakin)
Keywords: regression

Comment 29

5 years ago
Comment on attachment 8362907 [details] [diff] [review]
part.1 Don't consume key event on <select> element if it's never handled

Sorry, I don't think I should review this.
Attachment #8362907 - Flags: review?(enndeakin)
Comment on attachment 8362907 [details] [diff] [review]
part.1 Don't consume key event on <select> element if it's never handled

Thanks, Enn. Smaug, could you review this? See the previous my comment for the detail.

I'd like to land this patch in 29 cycle.
Attachment #8362907 - Flags: review?(bugs)
Comment on attachment 8362907 [details] [diff] [review]
part.1 Don't consume key event on <select> element if it's never handled

>     default: // printable key will be handled by keypress event.
>+      incrementalSerchResetter.Cancel();
Serch? I guess it should be Search

>+  AutoIncrementalSearchResetter incrementalSerchResetter;
Search



This is rather scary stuff. Could cause regressions.
Attachment #8362907 - Flags: review?(bugs) → review+
(In reply to Olli Pettay [:smaug] from comment #31)

Thanks!

> This is rather scary stuff. Could cause regressions.

Indeed. But we need to try do behave similar as other browsers as far as possible...
https://hg.mozilla.org/integration/mozilla-inbound/rev/5b206fc5ecb6
status-firefox26: --- → wontfix
status-firefox27: --- → wontfix
status-firefox28: --- → wontfix
status-firefox29: --- → affected
status-firefox-esr24: --- → unaffected
Whiteboard: [bugday-20131111] → [bugday-20131111][leave open]
Created attachment 8364928 [details] [diff] [review]
part.2 Add new tests

https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=fe74b0be7bc7

Okay, finally, the new tests passed on all environment.

See bug 962029 for the detail of disabling test_bug746272-2.html. This causes moving the boundary between test_bug746272-1.html and test_bug746272-2.html.
Attachment #8364928 - Flags: review?(bugs)
Attachment #8362909 - Attachment is obsolete: true
Comment on attachment 8364928 [details] [diff] [review]
part.2 Add new tests

I was expecting to see tests for the cases when preventDefault() is called.
Could you add still defaultPrevented() checks.
Attachment #8364928 - Flags: review?(bugs) → review-
Created attachment 8366422 [details] [diff] [review]
part.2 Add new tests
Attachment #8366422 - Flags: review?(bugs)
Whiteboard: [bugday-20131111][leave open] → [bugday-20131111]
Ah, I'll remove the diff of dom/tests/mochitest/localstorage/mochitest.in since the test has been disabled on Android (bug 962029 comment 4). Please ignore it.
Attachment #8366422 - Flags: review?(bugs) → review+
status-firefox29: affected → fixed
https://hg.mozilla.org/mozilla-central/rev/1ab856ce8afe
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla29

Updated

5 years ago
Keywords: dev-doc-needed, site-compat
(Reporter)

Comment 42

4 years ago
It was resolved in Mozilla Firefix 29.0.
v. per comment #42.
Status: RESOLVED → VERIFIED

Updated

4 years ago
Depends on: 1008244
Not sure how Bug 1008244 is resolved but added this to the site compat doc:
https://developer.mozilla.org/en-US/Firefox/Releases/29/Site_Compatibility#Events
https://twitter.com/FxSiteCompat/status/463447343023783937
Keywords: dev-doc-needed → dev-doc-complete
(Reporter)

Comment 46

4 years ago
The error is resolved in Mozilla Firefix 29.0(select with size=2,3 etc.), but it does not work any more select with size=1.
(Reporter)

Updated

4 years ago
status-firefox29: fixed → ---
(Reporter)

Updated

4 years ago
Severity: normal → major
(In reply to Liviu PADURARU from comment #46)
> The error is resolved in Mozilla Firefix 29.0(select with size=2,3 etc.),
> but it does not work any more select with size=1.

Your issue, apparently a regression, is tracked in Bug 1008244. This bug itself is fixed in 29.
Severity: major → normal
status-firefox29: --- → fixed
(Reporter)

Comment 48

4 years ago
All the errors are resolved in Mozilla Firefox 30.0.
You need to log in before you can comment on or make changes to this bug.