Open Bug 71074 Opened 23 years ago Updated 2 years ago

Commands should fire on key down, not keypress (because keypress includes keyrepeat)

Categories

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

Product:
Core
Component:
DOM: UI Events & Focus Handling
Type:
defect

Tracking

()

Milestone:
mozilla1.1alpha

People

(Reporter: bugzilla, Unassigned)

References

(Blocks 2 open bugs)

Details

(Keywords: helpwanted)

Attachments

(1 obsolete file) 

many thanks to nbaca for finding this! orginally seen on linux [2001.03.06.08
opt comm bits]; she's currently checking to see if this also occurs on win32 and
mac.

would this be an event issue, perchance? it's as if holding down the keys
generates more than one event --which it shouldn't. this would be bad for people
who are slow typers/have RSI/disabilities, imho...

there are several ways to see this.

recipe #1:
1. open a browser window.
2. view source by holding down ctrl+U for *at least* 2 seconds.
result: instead of a single source window, i got dozens of them!

recipe #2:
1. open mail 3-pane window.
2. compose an email by holding down ctrl+M for *at least* 2 seconds.
result: instead of a single mail compose window, i got dozens of 'em. in fact, i
couldn't close them fast enough, and the app crashed.
Keywords: nsbeta1
Um, don't do that? :-)

Keys autorepeat when held down.  That's normal, and desirable (e.g. we depend on
editing keys autorepeating).  Do we have a way to flush the key event queue
after doing an operation?  And if so, how do we know on which operations to
flush the queue?  (Only when a new window is being brought up from a key event?
 Other times?)

4.x does the same thing mozilla does (brings up a new window for each time the
key autorepeats (at least on linux).
Build 2001-03-06-05: NT4, Mac 9.04
The problem can be reproduced on windows and the mac.
Hardware: PC → All
at the very least, this belongs to aaron
Assignee: alecf → aaronl
okay, removing my nomination after chatting w/akkana. i guess the workaround for
this would be to turn off/tune down autorepeat setting for a given system.
Keywords: nsbeta1
cc'ing smfr, and other folx who might have further comments/input on this.

actually, i tested the recipes in 4.x --interesting results:

linux: holding down alt+U [view source] only brings up 1 window; holding down
alt+M [compose email] brings up multiple windows [but doesn't spew as many as
N6.x/mozilla].

mac: cmd+U is n/a. holding down cmd+M only brings up 1 mail compose window.

winNT: holding down ctrl+U brings up only 1 source window. holding down ctrl+M
brings up multiple mail compose windows [again, doesn't spew as many as
N6.x/mozilla].

now am kinda tempted to bring back a nomination for beta1/mozilla0.9...
Keywords: 4xp
OS: Linux → All
We should not fire commands on autokey events. We should only fire them on the 
key down. Having users tune down their key repeat rate is *not* a suitable 
workaround.

The ones that only bring up one window do so because there is only one window of
that type in the app.  It's bringing up the same window repeatedly instead of
making new windows.

Simon: are you saying that these key bindings should be on KeyDown rather than
KeyPress?  If UI people agree with that, then we need a list of key bindings
that need to be changed to KeyDown.  Clearly not all of them should be, e.g.
editing keys *should* autorepeat.  And KeyPress needs to fire on each
autorepeat, otherwise autorepeat wouldn't work for text insertion or deletion.
akk: yes, that's what I'm saying. That's how must other Mac apps behave.

cc'ing german and jglick.
This could conflict with accessibility requirements. Would love to hear aaronl's
opinion on this. 
I am skeptical on why key autorepeat would be useful for anything other than
inside an editor window and I am tending towards thinking there should only be
one instance of e.g. a source view window for any given browser window.
i don't understand why key repeat should happen while modifiers are active.

There's one class of exceptions: left/right which are word and page navigation 
that should repeat after interval. if someone wants to be able to Undo or Paste 
repeatedly i'll listen to their argument. beyond that i don't see reasons for 
modified keys to be repeated.

Note: modified keys that result in characters are not modified keys in my book 
(ie, shift+1 which is ! or something which generates a foreign character)
Key repeat is done by the OS and has nothing to do with whether modifier keys
are pressed.  Thank goodness, since I'd be upset if bindings ^H (delete prev
char) or ^N/^P (next/previous line) didn't autorepeat.

Bindings which we want to be immune to autorepeat can be put on KeyDown, as
Simon suggests; our spec says that autorepeat generates only repeated KeyPress
events, though there used to be a bug that gtk autorepeat sent us repeated
KeyDowns as well (not sure if that was ever fixed).
This isn't a good bug for me to work on, I'm trying to focus on the
accessibility stuff I'm booked up with. Can someone else take it?
Keywords: helpwanted
Suppose you hold Ctrl+N in browser window A, you get a new browser window B.  
When key repeat starts to kick in, which window get the second keypress event?

This bug is a dup of bug 65137 or vice-versa.
*** Bug 65137 has been marked as a duplicate of this bug. ***
This doesn't seem to occur for me in Win2k with Accel+U anymore, but still 
happens with Accel+M and Accel+N. Did someone change fix the view source command 
to only work on keydown?

Here is where the accesskey for new navigator window is coded:
<menuitem label="&browserCmd.label;" accesskey="&browserCmd.accesskey;" 
key="key_newNavigator" command="cmd_newNavigator"/>

Perhaps we need another attribute, such as autorepeat="off"

Does anyone have a list of all the commands that autorepeat should be turned off 
for?
Severity: major → minor
Priority: -- → P2
Target Milestone: --- → mozilla1.1
> Does anyone have a list of all the commands that autorepeat should be turned
> off for?

All of them, except for those involved in editing *text*. (Even editing items 
in a tree, e.g. deleting bookmarks, should require separate key-presses for 
each action.)

I think this rule is rather clear-cut, so there shouldn't be a need for app 
authors to specify an auto-repeat attribute for each command -- that would just 
invite errors. Instead, I suggest some magic be done to determine whether or 
not the command applies to a text field, and only pay attention to
auto-repeating in that case.
What about shift-down-arrow to extend selection in a mail pane? What about 
Pagedown or down-arrow in the browser or mail panes? Is a generic rule going to 
bite us in the rear later?



I just ran into an evil manifestation of this problem: in mail, if you hold down 
the spacebar in the thread pane to go to an unread message in another folder, it 
spawns multiple 'do you want to go to the next unread' dialogs.

This is a very evil problem, and I think we should fix it in mozilla 0.9 or .9.1
Severity: minor → major
Summary: holding down accel+key >2sec opens multiple windows → Commands should fire on key down, not keypress
Adjust summary, major severity.

Is the summary still accurate for this bug? Won't firing commands off key down
hose IME systems?

That said, what do you think the right fix is?
I agree, moving ALL commands to keydown sounds like a scary prospect.  I don't
have a better suggestion, though.  Do we need to have a change to the event
model to allow turning off key repeat until the next keydown?  Or just have a
mode to turn off typeahead temporarily (as some Unix apps, e.g. trn, do)?  Joki,
has the event working group ever considered this problem?
Keywords: nsCatFood
Ok, how about this rule:
*   if the command edits text, it auto-repeats;
*   if the command includes a navigation key (Left, Right, Up, Down, Page Up,
    Page Down, or Tab), it auto-repeats;
*   if it's anything else, it doesn't auto-repeat.

IMO this is a trivial bug, not major. Many other apps -- including AppleWorks, 
SimpleText, WorldText, the QuickTime Player, iTunes, and Internet Explorer -- 
suffer chronically from this problem, but it doesn't affect their usability to 
any great extent. 4.x doesn't have the problem, though there is some weirdness 
with the menu bar when you hold down the key for a command.

Holding down Space bringing up multiple go-to-next-folder alerts would be a 
separate bug, as that would also be possible from pressing Space twice in quick 
succession, not just from auto-repeat.
Space should also be excluded, at least when it's used for navigation or 
scrolling.
I disagree with Jesse here - I hold down space all the time in LXR to get 
further down in a long file. I want autorepeat for space = pagedown.
I meant that space should be excluded from the change from keypress to 
keydown.  So we don't disagree :)
On Windows, accelerators repeat if you hold them down.  Hold down CTRL+N with 
IE (or any Windows app), and you'll see it popping up window after window.  I 
don't think we need to fix this for Win32, since we are obeying the OS 
behavior.  

Also, not to be a pain in the ass, but I will fight tooth and nail any attempt 
to change when our key bindings fire for Mozilla 1.0.  We all know how many 
regressions we'd be dealing with if we did that.

Also, a theory... the reason you probably see some commands like View Source 
bringing up only one window is that the key binding is probably firing over and 
over again, and there's just code that only allows one window of that type to 
be open at any given time for that particular version of the app on that 
platform.

hyatt: it may be the case that other windows app's fire commands on auto-repeat, 
but my guess is that they execute the commands synchronously (i.e. the second 
Ctrl-N is only picked up when the first window has finished being created), so 
the user has a much longer time period within which they can release the key.

We, on the other handle, execute commands asynchronously (we just fire off 
events), so the consequences of executing commands on autorepeat are much worse.

One issue which I haven't seen brought up yet is that we might see problems with  
a command being handled on keydown and then another command being handled on 
keypress.  "Handling" the keydown event doesn't affect the handling of a keypress 
event since they are different events.  This model is desirable in some cases / 
examples.  :-/

What about creating two groups of functions:
1. actions, what make sense being repeated (down arrow, backspace, new browser
window, etc)
2. others, what can be wrong, or annoying if repeated (popping up another "The
text you entered was not found" message for Find Again, when the previous wasn't
dismissed (see bug 88827),  just about any modal dialog, etc)

Then after generating an event in the queue, disable generation of only that
event. When the related action happens (like a dialog pops up), leave some time
for the user to recognize it (wait for a user adjustable amount of miliseconds).
 After that, if the action belongs to group 1 (see above), re-enable the
generation of that event, if it belongs to group 2 then leave re-enabling at the
 end of the function (like pressing the OK button).

I'm not a programmer (and not a native English speaker), so my description could
be hard to understand, but I hope you can "get it". I can write some examples if
required for clarification. (I mean autorepeated keypresses, and even
mousedown/mouseclicks under the term "event" above.)


This approach could help usability on slow computers, while maintain performance
for power users, and most importantly, have the user always in control.
*** Bug 88827 has been marked as a duplicate of this bug. ***
info about the space problem that sfraser mentioned.

we've got this, in mailWindowOverlay.xul

<keyset id="mailKeys">
  <key id="space" key=" " oncommand="SpaceHit()"/>

would this be covered by the eventual fix to this bug?

ideally, we need the command to be fired on keydown, not on the repeated firing
of keypress.
Blocks: 90318
Speaking from a Windows perspective, I don't see any examples of "bad" behavior 
that don't currently happen in Windows apps.  Open up MS Word and hold down 
Ctrl+N.  What happens?  A whole bunch of blank documents open up.  Open up NS 
4.7x news and hold down space.  The active message scrolls and when it's done 
the next message opens, and so on.

I'm with Hyatt on this.  Don't mess with it when we're this close to 1.0.  If 
we do, it's going to cause more problems than we're ready to deal with.
Firing commands on key press (hence autokey) seems just wrong. Reasons:
* By holding down Ctrl-N for a period of time, you can't control how many new
  windows pop up (or how many mail messages you delete; actions could be
  destructive).

* People with motor disabilities would have particular issues here.

* The first and subsequent key presses could have different results (if the first
  keypress takes you into a "mode". e.g. the first spacebar press in mailnews
  brings up the 'Do you want to go to the next folder with unread messages' 
  dialog. The next spacebar press confirms that dialog (by bonking the default
  OK button in the dialog). Thus, holding down the spacebar here causes this
  dialog to flash up and then be instantly confirmed, which seems wrong.

Almost no applications on Mac execute commands on autokeys; the exceptions are 
well-controlled and useful (e.g. repeated Find Again in a text editor).

I'm unconvinced by the "people with motor disabilities" argument: if someone has
enough difficulty with a keyboard that they can't release a key quickly enough
to prevent autorepeat, shouldn't they increase their autorepeat interval or turn
off autorepeat?  Don't most OSes allow for that?  Otherwise they'll constantly
have problems in typing as well.

I'm not arguing that this bug shouldn't be fixed (assuming someone cares enough
to go through all of our key bindings and decide which ones need to be changed
and which don't, and to make sure that XBL and XUL key bindings are all kept in
sync during the change, and to test all the resulting bindings), just that it
doesn't seem like an accessibility issue since there are better solutions to the
problem which affected users should invoke anyway since other people have
already mentioned that we're not the only app with this problem.  

Unless, that is, there are OSes which don't offer this sort of control, in which
case, an easy solution would be for mozilla's low-level platform key event
handlers to allow changing the interval or turning off autorepeat entirely. That
would be fairly easy (adopting the timer code that some platforms already use
for multi mouse clicks).
I don't think we'd need to go through all our key bindings. I think we should 
just make 'oncommand' fire on key down, not key press.

windows, win-r; notepad; <tab> <space-don't-release> <escape>.

results: dialog is aborted. conclusion: if space is related to oncommand then 
oncommand shouldn't be firing on keydown.

It seems like we have a few issues here (keypress v. keydown; repeat)... can't 
we just change the macos code not to fire keyrepeats for commands?

or perhaps we should change the event that repeat sends to be its own command 
which people could decide to handle or ignore. onrepeat="dump('help i'm being 
repeated')"
Disclaimer: everything proposed in this bug and even my comments below still 
strike me as a band-aid over a broken bone... :-(


Maybe this isn't the entire solution but here is my suggestion...

Maybe the issue is that keypress events should not repeat/trigger if the original 
destination (window) has changed.  (The event system shouldn't pass along 
keypress events to the new window until it had a keydown event for that window.)

This won't solve all of the issues so maybe it's not a good idea.  It would 
certainly solve some of the problems where multiple dialogs are popping up 
unexpectedly.


I like timeless' idea of having a separate key event for repeat events.
Joki--is it too late to change the DOM spec for this stuff?  Could we have:
  keydown
  keypress
     keyrepeat*
  keyup

 (* note that this even wouldn't always be triggered when the user presses a key; 
only when the OS has determined that the user is in auto-repeat mode.)

i like brade's suggestion: The event system shouldn't pass along keypress 
events to the new window until it had a keydown event for that window.
Simon's "fire oncommand on key down" suggestion refers to xul bindings only?  So
it wouldn't affect XBL bindings?  We definitely don't want XBL bindings to fire
only on keydown, since it would stop motion keys (e.g. arrows) and deletion keys
from autorepeating, which would bite more users than it would help.

If we made XUL bindings work on keydown, and XBL bindings work on keypress, we'd
have to be a lot more careful than we are now about conflicting bindings. 
That's generally a good thing ... but we'd have problems with bindings like
ctrl-W or ctrl-F in linux, which do different things depending on where the
focus is ... currently, if focus is in a text control, the keydown is handled by
the text control's XBL so the event doesn't bubble up to the window's XUL
handler.  We'd somehow have to stop the keydown event from bubbling up if a
keypress event (which hadn't happened yet) was going to be handled.  Maybe we
could just put a null XBL keydown handler corresponding to all the existing
keypress handlers ... but that increases the complexity of the XBL binding list
and might slow down typing performance.

Just trying to illustrate that this isn't simple.  I like Kathy's suggestions
better -- either one might be a nice clean solution to the problem which
wouldn't require revisiting all our existing bindings (trying to keep XBL and
XUL bindings in sync is a nightmare, as I'm sure Kathy would agree).
I also think that people with motor disabilities will be using the utilities
that come with their operating system to help with typing. For example, in
Windows Control Panel - Accesibility Options - Keyboard, you can select sticky
keys, filter keys and/or toggle keys. Each of them have detailed settings.

By controling these OS settings, the user can minimize these problems across
their entire environment.

However, if we end up on a lot of web appliances, this would be useful.
That said, I don't think it should be done right now.

From the code standpoint, it sounds like Simon understands the problem.
Assignee: aaronl → sfraser
Severity: major → normal
This is an XP toolkit issue.
Assignee: sfraser → trudelle
Component: Keyboard Navigation → XP Toolkit/Widgets
QA Contact: sairuh → jrgm
This bug is a serious usability issue, and also affects accessibility. See, for 
example, bug 114659. It needs some attention.

Comment on attachment 35383 [details] [diff] [review]
Patch v1.1 - includes build capability using midl compiler

This patch does not apply to the bug, right?
Attachment #35383 - Attachment is obsolete: true
*** Bug 114659 has been marked as a duplicate of this bug. ***
Keywords: nsbeta1
Do not check anything in on this bug until I have had a chance to review it.  

I'll be back Jan 1. :)

nsbeta1- per Nav triage team
Keywords: nsbeta1 → nsbeta1-
*** Bug 128680 has been marked as a duplicate of this bug. ***
This problem is mentioned in the Aqua Human Interface Guidelines:
http://developer.apple.com/techpubs/macosx/Essentials/AquaHIGuidelines/AHIGUserInput/Type_Ahead__Auto_Repeat.html
Depends on: 91592
From the Apple page:

> An application can tell whether keystrokes are generated by auto-repeat or
> by the same key being pressed numerous times. Your application can disregard 
> auto-repeat keystrokes; it should ignore them in keyboard equivalents.

nsKeyEvent does not have a way to tell.  Perhaps it should.  I can see an
argument that XUL key bindings not fire on autorepeat events (though we do want
autorepeat to fire XBL bindings -- for instance, backspace should autorepeat).

Implementing this in the gtk handler might be some work (our gtk event handler
used to have a problem distinguishing autorepeat events: I remember a bug where
it was firing keydown/keyup for each one).  But that doesn't affect the decision
on whether it would be the right thing to do.  Would it make sense on other
platforms?

Also: would it be desirable to have a "flush event queue" command, and call that
whenever we pop up a new window or dialog?  That, too, would have to be
implemented on each platform; shouldn't be too hard for gtk/xlib.
I'm in favor of having a flag for "isRepeating" (or whatever we decide); I don't
know how that fits into the proposed standard yet (I'll look into that shortly).

I'm not as sure about the flush event queue.  That should probably be covered in
a separate bug from the isRepeating issue.
*** Bug 167551 has been marked as a duplicate of this bug. ***
->keybd nav, renominating.
Assignee: trudelle → aaronl
Component: XP Toolkit/Widgets → Keyboard Navigation
Keywords: nsbeta1- → nsbeta1
QA Contact: jrgm → sairuh
cc patricec for UE input.
As some of the comments mention, repeat is okay but not on command keys.
Blocks: 121884
adt: nsbeta1-
Keywords: nsbeta1 → nsbeta1-
*** Bug 205462 has been marked as a duplicate of this bug. ***
Commands should not auto-repeat, and that includes function keys.   An example
is F5, which can be used to commit a Denial-of-Service attack on a local server
(I have plenty of evidence of this).  This specific problem is described more
fully in bug 224026.
Martin (comment 60): can you clarify what makes a command a command (in your
opinion)?  Is "scroll down" a command?  I think we need a specification or clear
definition for what is or isn't a command OR we need to decide that some
commands can repeat and others should not repeat.
Well, let's start with a quick look at the Browser menus.

File - disable repeats
Edit - autorepeat is ok for everything I think!
View - disable repeats
Go - disable repeats
Bookmarks - disable repeats
Tools - disable repeats
Windows - disable repeats

All function keys - disable repeats

Just that would go a long way towards usability.  

Have you ever held down Ctrl-D recently?!  You can't see anything happening, but
then go and look at your bookmarks menu.
On Unix, ctrl-D does the editor command cmd_deleteCharForward, which very
definitely should autorepeat.  Even on nonunix platforms, this command is still
bound to a key, as are lots of other editor commands.

It would be interesting to know whether there are commands outside the editor
which should autorepeat.  Listing menu contents won't tell us, since lots of
commands aren't exposed in a menu.  We'd need a listing of commands.  There's a
start (incomplete and probably out of date) somewhere under
http://www.mozilla.org/unix/customizing.html#keys -- search for "Legal commands
for use in custom key bindings".  Browser and mailnews commands aren't stored in
any centralized location, so it may take some work to compile them all.
Summary: Commands should fire on key down, not keypress → Commands should fire on key down, not keypress (because keypress includes keyrepeat)
*** Bug 231151 has been marked as a duplicate of this bug. ***
I'm having the same issue again with version 2.0.0.11 and the upcoming 3.0 on Linux.

Key repeat is enabled and many times when I press ctrl-t, the result is tens to hundreds of new tabs. The same is true for ctr-w and the other shortcuts.

This makes using the shortcuts unworkable for me. It all worked fine until version 2.0.0.11. 

Thanks
WP 
Mass un-assigning bugs assigned to Aaron.
Assignee: aaronleventhal → nobody
QA Contact: bugzilla → keyboard.navigation
Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information
Priority: P2 → P3
Component: Keyboard: Navigation → User events and focus handling
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: