110800 - arrow keys should not fire onchange (drop-down listbox)

Status: VERIFIED FIXED

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

Description

[Jesse Ruderman]

Many web developers seem to think the semantics of onchange for a <select> 
element is "when the user makes a selection" rather than "when the user changes 
the value shown in the select", and I think that's a reasonable assumption.

Steps to reproduce:
1. Find a page that uses a drop-down <select> element (with javascript) for 
navigation.  For example, http://www.idocs.com/tags/forms/_SELECT_onChange.html.
2. Focus the <select> using the keyboard.
3. Hit the down arrow key.

Result: you're suddenly taken to the second page on the <select>'s list.
Expected: you should be able to choose which page you want to go to, then hit 
tab or enter to fire onchange.

Note that fixing this bug might make forms containing <select>s more confusing, 
if modifying the value of a <select> changes other parts of the form.  I don't 
think that's as common as navigation <select>s.

This bug might have been fixed before in bug 42301.

Comment 1

[Boris Zbarsky [:bzbarsky]]

jkeiser?  thoughts?

Comment 2

[John Keiser (jkeiser)]

The reasoning makes sense to me, although it seems like this will depend on
other browsers' behavior.  I can't get NS4 to load the page, what does IE do?

Comment 3

[Jesse Ruderman]

IE and Opera both fire onchange as soon as you hit an arrow key.  I can't get 
onchange to work for <select>s in Netscape 4.

Comment 4

[Jesse Ruderman]

Never mind, I was using a half-crashed Netscape 4.  Netscape 4 also fires 
onselect right away.

Comment 5

[Jesse Ruderman]

Attachment: testcase

Comment 6

[rods (gone)]

I think the behavior is correct at the moment, especially because it matches 
IE, Opera and Nav 4.x.

But their is a regression from 6.2. Whenever an arrow key is pressed an onchange 
fires. 

Here is the fix:
Index: nsListControlFrame.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/html/forms/src/nsListControlFrame.cpp,v
retrieving revision 1.227
diff -u -r1.227 nsListControlFrame.cpp
--- nsListControlFrame.cpp      2001/11/14 13:39:59     1.227
+++ nsListControlFrame.cpp      2001/11/20 16:44:16
@@ -3220,7 +3220,7 @@
     PerformSelection(newIndex, isShift, PR_FALSE);
     // Dispatch event
     if (mComboboxFrame && mIsAllFramesHere) {
-      mComboboxFrame->UpdateSelection(PR_TRUE, PR_TRUE, newIndex);
+      mComboboxFrame->UpdateSelection(PR_TRUE, PR_FALSE, newIndex);
     } else {
       UpdateSelection();
     }

Comment 7

[Marc Attinasi]

sr=attinasi for patch inline at
http://bugzilla.mozilla.org/show_bug.cgi?id=110800#c6

Comment 8

[dcone (gone)]

r=dcone

Comment 9

[Aaron Leventhal]

-> John, can you do me a favor and check in rods' fix when the tree reopens?

Comment 10

[John Gaunt (redfive)]

checked in yesterday

Comment 11

[sairuh (rarely reading bugmail)]

arrow key just display the select list's items now. vrfy'd fixed using
2002.01.07.0x comm bits on linux rh7.2, winnt and mac 10.1.2.

however, i know that alt+down will drop down the select list --but i'm curious
why the first time i hit with just the down arrow doesn't drop down the list? is
that covered by another bug? thx.

Comment 12

[Jesse Ruderman]

Still broken in 2002 012008: pressing arrow keys at 
http://www.idocs.com/tags/forms/_SELECT_onChange.html causes the newly selected 
item to load immediately.  Reopening.

Sairuh: on Windows, it's normal for up/down to change the selection without 
making the entire list appear...

Comment 13

[John Gaunt (redfive)]

hmmm.... just tried this with a build from the tip: ( today's pull around noon )

works just fine. I can press up/down in the select without loading the next page
and I can use alt-up/alt-down to select without going to the next page. Only
once I hit enter does it take me on to the next page.

Comment 14

[John Gaunt (redfive)]

tried again, I was looking at the wrong type of widget. 

there is an input tag on the page linked to which is what I was testing against. 

So I tested the attachment and tested the select on the linked page. Both of
these cases where there was an onchange handler defined we process the onchange
as soon as the down arrow/ up arrow is pressed ( or if you are fast you can
scroll 2-3 items ).

changing the summary to represent the actual discussion in the bug.

Comment 15

[John Keiser (jkeiser)]

This is fixed in the upcoming patch to 112241 (currently in review)

Comment 16

[John Keiser (jkeiser)]

Ooops.  Not fixed.  In fact, it's fixed /back/ to 4xp in the new bug.

We *cannot* just get rid of the onChange without doing something else.  The
problem is, the user can change the value, click on some other control, and
onChange will never be fired.  So if we are going to fix it to not constantly
fire (and I think this is good) we need to *also* make sure it fires when the
control loses focus.

We should not put in any fixes that do this halfway.  It's
data-integrity-destroying dangerous.

Comment 17

[John Gaunt (redfive)]

seems to me the arrow keys should cycle through the list and when you get to the
one you want you press enter (or space). Just like when you drop the list down.

Currently, if you alt-down cycle to a new option and then alt-up, onchange is
_not_ fired, but if you alt-down cycle to a new option then hit enter an
onchange _is_ fired

ccing aaronl for input on behavior

Comment 18

[Jesse Ruderman]

jgaunt: we're both talking about the drop-down listbox, right?  That's what I 
meant by <select size=1> in the summary.

I agree that enter should fire onchange for a drop-down listbox if its value 
was just changed using the arrow keys.  If enter also causes the form to be 
submitted, onchange should fire before onsubmit.

Comment 19

[John Gaunt (redfive)]

re-assigning to jkeiser since he just rewrote a bunch of this stuff and has a
better handle on the situation.

Comment 20

[John Keiser (jkeiser)]

I am fine with enter or space causing onChange to fire (if the box value has
changed), in fact I think that's a good idea.  But if we do that, tabbing out of
the dropdown or clicking somewhere else *also* needs to fire onChange.  We can't
just assume the user is going to be good and hit enter for us.  If we implement
this half-assed, data entry and query pages will get broken when people do this.
 At least this way data integrity is ensured, even if it's not perfect for
navigational pages.

So in short, we need to do all (no onChange on up/down, onChange on enter or
lost focus) or nothing (the way it is).  A third possibility is to have up and
down drop the combobox down.  That would fix it for everybody.

Comment 21

[John Keiser (jkeiser)]

I broke down and implemented this when bug 97543 showed up on my list.  Patch
forthcoming.

Comment 22

[John Keiser (jkeiser)]

Attachment: Patch

This fixes this problem by (a) not firing onChange for comboboxes, (b) firing
onChange onBlur iff the selected index has changed since onFocus or since
onChange was last fired.

Also adds more comments and makes it list/combobox incrementally more readable.

Comment 23

[John Keiser (jkeiser)]

Oops, hit enter mid-change, sorry for spam

Comment 24

[rods (gone)]

r=rods

Comment 25

[Kevin McCluskey (gone)]

Marking nsbeta1+

Comment 26

[Kevin McCluskey (gone)]

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

Comment 27

[Kevin McCluskey (gone)]

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

Comment 28

[kinmoz]

Comment on attachment 67780 [details] [diff] [review]
Patch

sr=[e-mail]

Comment 29

[John Keiser (jkeiser)]

Fix checked in.

Comment 30

[sairuh (rarely reading bugmail)]

tested using 2002.02.11 comm bits on linux rh7.2, win2k and mac 10.1.2. this is
what i see:

a. up/down arrows select items in the listbox, but do not activate 'em.
b. however, after using the the arrows, if i hit tab to get out of the listbox
and go to the next link on the page, the selection is activated instead!

should i reopen this because of latter issue (b)? or file another bug [unless
it's already known]? thx!

Comment 31

[John Keiser (jkeiser)]

If you change the listbox and then try to leave the listbox, onChange is going
to be fired, yes.  That is deliberate.  onChange *will* be fired whenever the
dropdown is changed.  Making an exception to this for navigation would cause
dataloss on applications that use onChange for validation.  That is not acceptable.

Now, if you hit ESC or change the listbox back to the value it originally was
before you tab out, onChange won't be fired.

Comment 32

[sairuh (rarely reading bugmail)]

ah, good to know! marking vrfy'd, then.

Comment 33

[Jesse Ruderman]

Filed bug 150759, "Enter should fire onchange for drop-down listbox changed with
arrow keys".  I mentioned that it should in comment 0 but didn't test it when
this bug was marked as fixed.  D'oh!

Comment 34

[Jesse Ruderman]

Several people have filed bugs about this change: bug 126379, "Select onChange
not called using down arrow key".