When a <select> has focus, many keyboard shortcuts doesn't work. Steps to reproduce: * Open attached testcase * Select one of the <select>s * Press Ctrl + W, Ctrl + N, Ctrl + T, Ctrl + 2, etc.

Forgot to mention: This is build 2002011103.

*** This bug has been marked as a duplicate of 72352 ***

I don't think this is a dup. Bug 72352 seems to be about being able to turn off emacs keybindings on Linux/Unix and being able to turn them on on other platforms. The reporter of bug 72352 mentions not being able to close window by pressing Ctrl+W when focus is an a textfield, but that works fine for me here on Win2k. I'm talking about the <select> element - list boxes and dropdown lists.

bryner?

See also bug 123008, "Many keyboard shortcuts doesn't work after switching themes from View menu".

[fixing my stupid grammatical error in summary. sorry for the spam.]

-> 1.0, nominating for beta1.

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

nsbeta1- per ADT triage team

Potentially related to bug 128995.

Actually, forget I said that. I can't reproduce this using 2002032003 and XBL form controls.

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

This problem is more general and not specific to Windows, see the dupe. Updating summary and OS accordingly.

Ctrl+N wfm in a textarea. I think that's a Linux-specific problem. *No* shortcuts work for me in <select> elements (Ctrl+N, Ctrl+T, etc) and I've run into this bug several times.

Jesse: on what OS? Shortcuts work fine for me in <select>s using 2002033009 on Win2K.

Well, they don't work here -- Win2k, build 2002-03-26-03. Are you sure your focus is on a <select> element, e.g. the Target Milestone dropdown on this page? I don't think bug 122989 is a dup of this, btw...

Oh... tee hee. I forgot that I'm using XBL form controls. Works fine with them, though!

If this bug doesn't occur with XBL form controls, we can mark it FIXED once XBLFC is turned on by default. Marking dependent.

We still need this fixed for non-XBL form controls, which are likely to be used in a variety of important embedding products. Don't assume bug 57209 will fix this.

Removing dependency, then. I was under the impression that the old form controls would be removed once the XBL ones were turned on.

So was I, and more than a couple of bugs have been marked as fixed since they work using XBLFC. Which is it?

We need to find those bugs that were marked fixed because of XBLFC and reopen them, ASAP. XBLFC is not as far enough along as was hoped.

Things like the focus indicator for the active item in a listbox, for example.

Aaron: Look at bug 57209's OtherBugsDependingOnThis field. A lot of them should be removed from that field if non-XBL form controls will continue to exist.

marking nsbeta1, adt2. Removing minus since xbl form controls not being turned on has changed the playing field

Nav triage team: nsbeta1+/adt2

This should be reassigned if the bug is to be fixed in the current form controls.

rods, for retriage

Tom, is this another manifestation of the event targeting bug?

same as bug 131921?

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

Isn't this a dup of another bug somebody is working on?

Are you thinking of bug 128995? That's for XBL form controls.

nsbeta1-. engineers are overloaded with higher priority bugs.

Also the case when focus is on a text or password input. Mac OS 10.1.5/Mozilla1.1a build ID:2002061014.

-->

Perhaps we're eating keystrokes and not giving them to other people

> Perhaps we're eating keystrokes and not giving them to other people Nooooo! Think of the starving children in Africa!

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

the function PreventBubble() will eat up the "Control" key event, so skip it if "Control" pressed.

seek r=

Simford, good try! But this bug is not for control key only, it's for all shortcut keys, such as Ctrl+D, F11. I guess it not only blocks the shortcuts in UI, but also the accesskey attribute in HTML page.

Due to comment 43, I suggest skip the call PreventBubble().

seek r= again.

Works as expected now, Ctrl-t, F11, etch all do their job even when a form control element is selected. Nice job.

What about selecting options in the <select> using the first letter of the item? Are there any adverse effects from that?

It'll be better that use a flag here to see whether or not we handled the keyevent, then call PreventBubble() if really handled. Otherwise, you might mess up the keyboard shortcuts.

seek r=

That doesn't seem right to me. We also need to handle anything for selecting items by the first letter (eg. using 'W' in the Platform select on this page). Can it be done in a better way than hard-coding keys? Babbling... What about putting the PreventBubble in the default section of the switch, maybe by adding an else somewhere around the |if (wasChanged)| check? Alternatively, maybe after |if (!isControl)| check. We could add an |isAlt| flag and have |if (!isControl and !isAlt and !isShift) nsevent->preventBubble|. Or are these too late in the process? Is there a specific reason that preventBubble is called before anything else? I notice we have code in |#ifdef FIX_FOR_BUG_62425| section that calls preventBubble and preventDefault separately.

It is a good idea to put PreventBubble() after |if (wasChanged)| Hope it works this time

Thanks Kyle! seek r=

Comment on attachment 103563 [details] [diff] [review] better one Index: html/forms/src/nsListControlFrame.cpp =================================================================== RCS file: /export/src/cvs/mozilla_1/mozilla/layout/html/forms/src/nsListControlFrame.cpp, v retrieving revision 1.1.1.1 diff -u -r1.1.1.1 nsListControlFrame.cpp --- html/forms/src/nsListControlFrame.cpp 2002/10/21 00:17:08 1.1.1.1 +++ html/forms/src/nsListControlFrame.cpp 2002/10/21 10:03:42 @@ -3323,14 +3323,9 @@ return rv; } - nsCOMPtr<nsIDOMNSEvent> nsevent(do_QueryInterface(aKeyEvent)); - - if (nsevent) { - // We are handling this so don't let it bubble up - - nsevent->PreventBubble(); - } - + // Whether we need PreventBubble or not + PRBool needPreventBubble = PR_TRUE; + // Whether we did an incremental search or another action PRBool didIncrementalSearch = PR_FALSE; @@ -3456,6 +3451,8 @@ } return NS_OK; } + + needPreventBubble = PR_FALSE; PRUnichar uniChar = ToLowerCase(NS_STATIC_CAST(PRUnichar, charCode)); @@ -3505,13 +3502,25 @@ if (wasChanged) { UpdateSelection(); // dispatch event, update combobox, etc. } + needPreventBubble = PR_TRUE; break; } } } - } // while + } // for } break;//case } // switch + + //if current key has been processed then we need to call PreventBubble() + if (needPreventBubble){ + nsCOMPtr<nsIDOMNSEvent> nsevent(do_QueryInterface(aKeyEvent)); + + if (nsevent) { + // We are handling this so don't let it bubble up + + nsevent->PreventBubble(); + } + } // If we didn't do an incremental search, clear the string if (!didIncrementalSearch) {

jkeiser, could you r=?

Comment on attachment 103563 [details] [diff] [review] better one Wow, we suck, thanks for catching this. First of all, we should do preventDefault() instead of preventBubble() to keep shortcuts from working. PreventDefault() indicates that the event has been handled by the browser already, but does not stop it from propagating to user onKeyPress handlers. Second, there are *many* cases that need preventDefault() in this method, not just the A-Z, though I understand why you did this ... could you add a PreventDefault() to the other cases in this method as well?

jkeiser, simford uses |needPreventBubble| to check whether we need to call |preventDefault| and set it to TRUE by default, so it covered the all case in |switch(keyCode)|, not only the A-Z.

Ah, I see, you are right. It still needs to call PreventDefault() instead of PreventBubble(). That's the only thing that needs changing, everything else is good. All keyboard shortcuts should work properly with that on (they are supposed to check whether we called PreventDefault() before doing anything). This will make it so that onkeypress handlers get called properly.

make changes due to comment 57 seek r= & sr=

Comment on attachment 104261 [details] [diff] [review] patch r=jkeiser

Comment on attachment 104261 [details] [diff] [review] patch sr=bzbarsky

Comment on attachment 104261 [details] [diff] [review] patch a=roc+moz for trunk

checked into trunk!

vrfy'd fixed with 2002.11.19 comm trunk bits.