Open Bug 1019630 Opened 10 years ago Updated 2 years ago

preventDefault not working with select/option tags for mouse/keyboard events

Categories

(Core :: DOM: UI Events & Focus Handling, defect)

29 Branch
x86_64
Windows 7
defect

Tracking

()

UNCONFIRMED

People

(Reporter: kram.nbert, Unassigned)

References

Details

Attachments

(3 files, 1 obsolete file)

User Agent: Opera/9.80 (Windows NT 6.1; WOW64; AppleWebKit/535.1) Presto/2.12.388 Version/12.17

Steps to reproduce:

Just try the following page

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<select id="aSelect" size="6" multiple="true" style="width:150px; height:15em;">
	<option id="aSelect_01" label="01" value="01">01</option>
	<option id="aSelect_02" label="02" value="02">02</option>
	<option id="aSelect_03" label="03" value="03">03</option>
</select>
</body>
<script type="text/javascript">
function disable_event(event) {
	event.preventDefault();
	event.stopPropagation();
	console.log(event.type+" disabled on "+event.target.value+" cancelable="+event.cancelable+" prevented="+event.defaultPrevented);
	return false;
}
document.getElementById("aSelect").onmousedown = disable_event;
document.getElementById("aSelect_01").onmousedown = disable_event;
document.getElementById("aSelect_02").onmousedown = disable_event;
document.getElementById("aSelect_03").onmousedown = disable_event;
document.getElementById("aSelect").onkeydown = disable_event;
document.getElementById("aSelect_01").onkeydown = disable_event;
document.getElementById("aSelect_02").onkeydown = disable_event;
document.getElementById("aSelect_03").onkeydown = disable_event;
</script>
</html>


Actual results:

click, ctrl+click and other mouse related selections operate "normally" and the options are selected as if the event handlers were not present

keyboard operations with arrows change the selected status of options but <space> key is "prevented" as expected



Expected results:

no selection should be possible
I can confirm this starting from version 25 onwards.
Was going to log this, but fortunately the bugzilla engine found this entry.
Here's a simpler example:
<html><head><title></title></head><body>
    <select size="3">
        <option onmousedown="event.stopPropagation();event.preventDefault();" value="1">1</option>
        <option onmousedown="event.stopPropagation();event.preventDefault();" value="2">2</option>
        <option onmousedown="event.stopPropagation();event.preventDefault();" value="3">3</option>
    </select>
</body></html>

This would prevent selection prior to version 25.
In an extension I maintain, we use the arrow keys for spatial navigation. When doing so, we preventDefault() so that <select> boxes don't change value as we're blurring them and focusing another element in response to the arrow keys. This behavior broke after upgrading form firefox 16 to 28. At some point I found a bug that indicated this change in behavior was intended and was implemented in Firefox 25, as jasonhpchu@gmail.com pointed out in comment 1. Bugzilla search fails me at this moment.

I believe that the answer (at least for the keyboard aspect) lies in layout/forms/nsListControlFrame.cpp in nsListControlFrame::KeyDown, where we find the comment:
2081   // Don't check defaultPrevented value because other browsers don't prevent
2082   // the key navigation of list control even if preventDefault() is called.

This is a system-group event listener for keydown on <select> elements which is responsible for changing the selected value in a <select> element in response to the arrow keys (among other keyboard behaviors). It fires in the system-group, after the default-group (which is accessible to javascript), but it does not respect event.preventDefault() called from the default-group handlers.

I believe that the claim that other browsers also implement this behavior is not 100% true. I can reproduce my expected result in Chromium Version 36.0.1985.125 Ubuntu 12.04 (283153).
Steps to Reproduce:
1. Check "nuke key events"
2. Give keyboard focus to the select box
3. Press down arrow

Expected Result:
1. The selected value does NOT change

Actual Result:
1. The selected value DOES change

In Chromium Version 36.0.1985.125 Ubuntu 12.04 (283153), the Expected Result is what actually happens.

Check the console for information about what events javascript land sees.
umm..Francis, can you remove my e-mail from the post itself?
e-mail address for other users are visible to people logged in, otherwise the post itself visible to the world and spambots.
thanks.
(In reply to jasonhpchu from comment #4)
> umm..Francis, can you remove my e-mail from the post itself?
> e-mail address for other users are visible to people logged in, otherwise
> the post itself visible to the world and spambots.
> thanks.

Oh good lord, I'm sorry! I assumed that was your handle on here. I don't see an edit button for my comment. (sorry, it's been about half a decade since I used bugzilla with any regularity)
yea, not sure why after logging in my e-mail's shown instead of my name....maybe it's a bug with bugzilla...haha....
hopefully a moderator picks this up, not seeing a way to find a moderator.
(In reply to Jason from comment #6)
> yea, not sure why after logging in my e-mail's shown instead of my
> name....maybe it's a bug with bugzilla...haha....
> hopefully a moderator picks this up, not seeing a way to find a moderator.

Apparently this is an open issue with bugzilla (bug 540, bug 282657). Open since 1998. It showed your email instead of your name initially, but now that I reloaded the page it shows your name instead of your email. Strange. Again, I'm sorry about that; it wasn't clear to me that bugzilla was showing me something it wouldn't show to people who aren't logged in.
When I noticed that behavior I've re-saved my account profile.  It seemed to have kicked it into effect.  Again, not sure why it would do it.  Since people not logged in would just see "Jason" just fine.

Anyhoo, HELLO MOD!!!
(tempted to swear in here hoping it'd flag a mod to look haha)
I've checked on a windows 7 machine now:
* chrome 36.0.1985.143
* IE 10.0.9200.17054

Also, on OS X Mavericks:
* Chrome 36.0.1985.143
* Safari 7.0.6

In ALL of the above cases, the Expected Behavior is what actually happens.

It is ONLY in Firefox that the Actual Behavior is what actually happens.
Mike, jesup on IRC recommended I needinfo you on this. I've got a patch that I think addresses this issue. I can't get mozilla-central to build, though. I've followed the Build Instructions on MDN for building on Linux, including installing the pre-reqs via the python script. I've avoided doing much with my mozconfig, because I'm new to this. I've built and run 31.0 release successfully with and without the patch.

I see 12 of these (I think they're all the same) later on in the build process of a clean checkout of mozilla-central:
20:42.45 TEST-UNEXPECTED-FAIL | check_stdcxx | We do not want these libstdc++ symbols to be used:
20:42.45 0000000000000000      DF *UND*	0000000000000000  GLIBCXX_3.4.9 _ZNSo9_M_insertIlEERSoT_
20:42.45 0000000000000000      DF *UND*	0000000000000000  GLIBCXX_3.4.9 _ZNSo9_M_insertImEERSoT_

Some more info for you:
 $ hg summary
parent: 201744:0753f7b93ab7 tip
 Bug 745283 - rev UUID again r=me a=kwierso
branch: default
commit: (clean)
update: (current)

 $ cat .mozconfig 
. "$topsrcdir/build/unix/mozconfig.linux"

CC="/usr/bin/gcc"
CXX="/usr/bin/g++"

mk_add_options MOZ_MAKE_FLAGS="-j4"

 $ uname -a
Linux deathcab 3.8.0-44-generic #66~precise1-Ubuntu SMP Tue Jul 15 04:01:04 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux

 $ gcc --version
gcc (Ubuntu/Linaro 4.6.3-1ubuntu5) 4.6.3
Copyright (C) 2011 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
Flags: needinfo?(mh+mozilla)
(In reply to Francis from comment #10)
>  $ cat .mozconfig 
> . "$topsrcdir/build/unix/mozconfig.linux"

Don't do this.
Flags: needinfo?(mh+mozilla)
(In reply to Mike Hommey [:glandium] (out from Sep 6 to Sep 22) from comment #11)
> (In reply to Francis from comment #10)
> >  $ cat .mozconfig 
> > . "$topsrcdir/build/unix/mozconfig.linux"
> 
> Don't do this.

Mike, thanks for your help, that got it building as expected. I'm confused as to why, perhaps you could help educate me? I'm especially confused since the MDN indicates that sourcing the pre-defined platform config file is the right approach (https://developer.mozilla.org/en-US/docs/Configuring_Build_Options#Example_.mozconfig_Files)
I'm working on a proposed patch. However, I don't seem to have privileges in bugzilla to assign this ticket to me.
Attached patch bug-1019630.patch (obsolete) — Splinter Review
Proposed patch, causes nsListControlFrame::KeyDown to respect keyEvent->mFlags.mDefaultPrevented
Assignee: nobody → fferrell
Component: Untriaged → Event Handling
Product: Firefox → Core
This code was originally added by masayuki's patch in bug 501496, would be good for him to weigh-in / review here.
Blocks: 501496
Flags: needinfo?(masayuki)
I believe this is a regression of bug 291082. I think it is related to the fact that arrow keys no longer fire a keypress event, only keydown and keyup now, and the fix for bug 291082 involved respecting event.preventDefault() in the keypress handler.
updated patch that adds new test for keyboard navigation of select elements, removes keyboard navigation aspects from existing test for incremental search of select elements
Attachment #8485015 - Attachment is obsolete: true
Attachment #8488852 - Flags: review?(bugs)
Can Esc, Alt+ArrowUp or ArrowDown, F4 (Windows only) close dropdown even with your patch? If not so, it's very serious regression. Our dropdown window is a front-most window. Therefore, if it's impossible to close a dropdown, it could be a security issue.

And why don't you ignore the mouse case?

Additionally, please use |-U 8 -p| for hg diff.
Flags: needinfo?(masayuki)
Comment on attachment 8488852 [details] [diff] [review]
bug-1019630-with-test.patch

Needs answers to masayuki's very valid questions.
Attachment #8488852 - Flags: review?(bugs)
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) (away: 9/13 - 9/15, JST) from comment #18)
> Can Esc, Alt+ArrowUp or ArrowDown, F4 (Windows only) close dropdown even
> with your patch? If not so, it's very serious regression. Our dropdown
> window is a front-most window. Therefore, if it's impossible to close a
> dropdown, it could be a security issue.
> 
> And why don't you ignore the mouse case?
> 
> Additionally, please use |-U 8 -p| for hg diff.

A patch for the mouse behavior will be separate. I have to get up to speed on writing an automated test for mouse interaction before I tackle it.

Is there a page on MDN I can refer to for best practices with hg? Please bear in mind that I'm new here.

I don't have access to a Windows machine nor Mac, so can only test on Linux. With my patch, the default behavior of closing the dropdown, Esc, Alt+Up and Alt+Down all works. However, with my patch, when an event handler is attached that calls preventDefault(), Esc, Alt+Up, and Alt+Down do NOT close the dropdown. How is this a security issue?
(In reply to Francis from comment #20)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) (away: 9/13 - 9/15,
> JST) from comment #18)
> > Can Esc, Alt+ArrowUp or ArrowDown, F4 (Windows only) close dropdown even
> > with your patch? If not so, it's very serious regression. Our dropdown
> > window is a front-most window. Therefore, if it's impossible to close a
> > dropdown, it could be a security issue.
> > 
> > And why don't you ignore the mouse case?
> > 
> > Additionally, please use |-U 8 -p| for hg diff.
> 
> A patch for the mouse behavior will be separate. I have to get up to speed
> on writing an automated test for mouse interaction before I tackle it.

Indeed, it's better to separate patches for keyboard and mouse handlers. You should use mq of hg.

> Is there a page on MDN I can refer to for best practices with hg? Please
> bear in mind that I'm new here.

https://developer.mozilla.org/en-US/docs/Mercurial_FAQ
https://developer.mozilla.org/en-US/docs/Mercurial_Queues

> I don't have access to a Windows machine nor Mac, so can only test on Linux.

nsListControlFrame is an XP part. So, except #ifdef XP_*, it's almost enough to test on a platform.

> With my patch, the default behavior of closing the dropdown, Esc, Alt+Up and
> Alt+Down all works. However, with my patch, when an event handler is
> attached that calls preventDefault(), Esc, Alt+Up, and Alt+Down do NOT close
> the dropdown. How is this a security issue?

Yes, it's too bad. Nobody shouldn't be able to prevent closing dropdown.
I don't believe that the desired mouse behavior originally described in this bug is actually desirable. If no selection should be allowed, then the disabled attribute should be used.

For preventing selection of anything in an entire select element, put disabled on the select element.

For preventing a specific option from being selected, put the disabled attribute on that option element. This has the added benefit that the option cannot also be selected via the keyboard. If you were to just put a mousedown handler on the option, and it did prevent selection via clicking, the user could still give keyboard focus to the select element and use the arrow keys or type the value to select the "unselectable" option.

I believe that the Actual Result, described below, is correct and should not be changed.

== Expected Result ==
This is the expected result, as I have interpreted it from the bug description, by kramnbert, and from comment #1, by Jason.

For the first select box ("preventDefault on select"):
* clicking on the select box does NOT summon the dropdown

For the second select box ("preventDefault on option"):
* clicking on "fine" and "yet another" options selects them
* click on "unavailable" does NOT select that option

== Actual Result ==

This behavior is observed on all of the following platforms:
* Windows 7 chrome 37.0.2062.120
* Windows 7 IE 11.0.9600.16428
* Windows 7 32.0.1
* Mac chrome 37.0.2062.120
* Mac safari 7.0.6 (9537.78.2)
* Mac firefox 32.0.1
* Ubuntu chromium 37.0.2062.94
* Ubuntu firefox 32.0

For the first select box ("preventDefault on select"):
[OK] clicking on the select box does NOT summon the dropdown

For the second select box ("preventDefault on option"):
[OK] clicking on "fine" and "yet another" options selects them
[FAIL] click on "unavailable" DOES select that option
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) (away: 9/20 - 9/23, JST) from comment #21)
> Indeed, it's better to separate patches for keyboard and mouse handlers. You
> should use mq of hg.
> 
> Yes, it's too bad. Nobody shouldn't be able to prevent closing dropdown.

Masayuki, should I create a new bug to deal with the keyboard issue? I really don't believe that the behavior originally described in this bug (mousedown preventDefault) should be changed.

I'll have to heavily revise my patch to respect Esc, Alt+Up, Alt+Down, and F4 on Windows. Is there a way to know, in javascript, whether or not the dropdown is shown on a select box? If not, I cannot write an automated test for this.
(In reply to Francis from comment #22)
> If no selection should be allowed, then the disabled attribute should be used.

Why is this false in keyboard case? Although, we should behave similar to the other browsers.

> I believe that the Actual Result, described below, is correct and should not
> be changed.

Just conforming to DOM Events spec, web applications should be able to prevent *any* default action. However, one of our mission is that we need to protect from "evil" web apps. In other words, if there is no security reason and it doesn't need complicated patch, we should behave similar to other browsers if they work similarly (but we shouldn't break the standard spec even if other browsers do same wrong behavior).

So, the important thing here is, how the other browsers behave for a call of peventDefault() of mousedown, click and mouseup.

> == Actual Result ==
> 
> This behavior is observed on all of the following platforms:
> * Windows 7 chrome 37.0.2062.120
> * Windows 7 IE 11.0.9600.16428
> * Windows 7 32.0.1
> * Mac chrome 37.0.2062.120
> * Mac safari 7.0.6 (9537.78.2)
> * Mac firefox 32.0.1
> * Ubuntu chromium 37.0.2062.94
> * Ubuntu firefox 32.0
> 
> For the first select box ("preventDefault on select"):
> [OK] clicking on the select box does NOT summon the dropdown
> 
> For the second select box ("preventDefault on option"):
> [OK] clicking on "fine" and "yet another" options selects them
> [FAIL] click on "unavailable" DOES select that option

So, our behavior is exactly same as all other browsers? Then, yes, we don't need to change the behavior at handling mouse events.

(In reply to Francis from comment #23)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) (away: 9/20 - 9/23,
> JST) from comment #21)
> > Indeed, it's better to separate patches for keyboard and mouse handlers. You
> > should use mq of hg.
> > 
> > Yes, it's too bad. Nobody shouldn't be able to prevent closing dropdown.
> 
> Masayuki, should I create a new bug to deal with the keyboard issue? I
> really don't believe that the behavior originally described in this bug
> (mousedown preventDefault) should be changed.

If you're right, the mouse part of this report is INVALID or WONTFIX. However, the keyboard part seems valid. Let's keep using this bug for the keyboard issue.

> I'll have to heavily revise my patch to respect Esc, Alt+Up, Alt+Down, and
> F4 on Windows. Is there a way to know, in javascript, whether or not the
> dropdown is shown on a select box? If not, I cannot write an automated test
> for this.

Use "popupshowing" and "popuphiding" events. They are fired asynchronously.

This test might help you:
http://mxr.mozilla.org/mozilla-central/source/layout/forms/test/test_bug935876.html?force=1
When I provide an example of a not working default prevention on <select>, would you pick up the issue again?
(In reply to Julian from comment #25)
> When I provide an example of a not working default prevention on <select>,
> would you pick up the issue again?

I have no intention of continuing work on this bug. Anyone interested can work off of my patch if they see fit. However, it was written against Firefox 29-ish; I'm not sure how cleanly the patch would apply to HEAD.
[Tracking Requested - why for this release]:

The issue still exists, as mentioned in the reported bug of mine #1428992, which has been reported as duplicate of this bug. With .preventDefault() not all actions can't be prevented, and it differs between macOS and Windows. However, all other recent browsers do prevent the default action.
Is there any intention of fixing this by someone?
Bz, do you think we should do anything about this issue? Thanks
Assignee: fferrell → nobody
Flags: needinfo?(bzbarsky)
We should once someone has time to look at this.
Flags: needinfo?(bzbarsky)
On my Windows 7 computer, Firefox (version 59) is the only browser in which the dropdown of a select element opens after pressing the Space key and calling event.preventDefault() in code. In Chrome, Opera and IE11, calling preventDefault prevents the dropdown from opening.
I'm also seeing this bug on my Windows 10 machine.
To see the bug, go to this URL:
https://catamphetamine.github.io/react-responsive-ui
Click "Select" in the menu, and you'll see a custom select component.
Click the "Select" header so that it gets focus.
Then press Tab, so that the select component has the focus.
Press Spacebar and see that the default select does expand even though `event.preventDefault()` is called on a Spacebar keydown event.
Do the same thing in Google Chrome and Microsoft Edge and see that the default select does not expand.
So Firefox lags behind even Microsoft on this one.
Component: Event Handling → User events and focus handling
See Also: → 1552419
Webcompat Priority: --- → ?
Webcompat Priority: ? → revisit

So I can't remember I assigned the webcompat priority here. But probably I imagine that I was looking for issues in between preventDefault and select/option for another bug, I met this one, which seems to have interop issues. But I don't have a webcompat issue associated with it.

Webcompat Priority: revisit → ---
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: