Open Bug 71074 Opened 24 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)

defect

Tracking

()

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: nsbeta1nsbeta1-
*** Bug 128680 has been marked as a duplicate of this bug. ***
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: nsbeta1nsbeta1-
*** 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

Created:
Updated:
Size: