On Window, Ctrl+Enter is seen as a Ctrl+J. It should be seen as a Ctrl+Enter or Ctrl+Return This is needed by Message compose to map Ctrl+Return or Ctrl+Enter to Send Message (see bug 20336). I wrote a sample XUL file to test CTRL+Return/Enter, I'll attach it to this bug. Linux has a similary problem filed as bug 50252.

I have a fix for that, I have tested on a english system and seems to works very well. However, I wonder if it will be compatible with IME! ftang, can you test it?

I applied the patch and tested on Japanese NT. IME is working fine (just a basic testing, input few Japanese characters). Also tested some keybord short cut, Ctrl+M, Ctrl+N, Ctrl+B, Ctrl+X, Ctrl+V, Ctrl+Z, Ctrl+Y they worked fine.

oh great news. Thanks. Also, rods reviewed the patch

I can't plus this under current criteria. We should bring our case to the pdt to get an exception.

Can this be checked into the trunk?

Why can't we get this plussed?

JF, who should review and super-review? Please get the patch into the trunk if it's a good fix to this bug. /be

I've already got two reviews (rods@netscape.com and nhotta@netscape.com). Also, I am running with this pach for almost two months without any problem. The Unix version of this bug has been already checked in for a while (see bug 20336). Now I just need a super review...

I have trouble to locate the right reviewer. Which of them is a Windows expert? are you Brendan?

I think buster can sr= or name a delegate for this case. /be

It looks a very dangerous change. Is there any reason we need to change this by rtm ? I recommend we don't take this change. I know the code is very messy there. But is that possible that we can have a safer change ?

Is that possible to make a small chage change to fix control+ENTER and Contrl+RETURN wihtout touch other code ?

Frank, we are not planning to check this fix in the Branch (RTM), it's only for the trunk. The fix remove the old ugly & messy code and replace by a generic one that does the right thing. It's has been tested for a long time now...

The patch is a diff -b -u. Maybe we should see the diff -u and a diff -wu if you fixed up tab and indentation problems. Frank, can be specific about what's "dangerous"? /be

your attachments 1 "Sample test file" does not work at all. I save it into a local file tst.xul. Without your fix, I expect to see at least control+shift+j change the "waiting..." to "j" . However, nothing happen there.

This is totally normal because Windows doesn't generate a WM_CHAR or WM_SYSCHAR for CTRL+SHIFT+nn sequence, this is a OS limitation. However, you can cath it using onkeydow or onkeyup.

rtm-, not a stop-ship. timeless, if you want to nominate a bug you should include a comment explaining why the fix is hugely important enough to take any risk into the rtm build. When you don't bother to even add a comment, I don't believe you're serious about the nomination.

Sorry, I mark bugs rtm because i think they're important and should be considered. Since I don't own the bug i don't feel i'm the best person to explain why a bug should be fixed. I have a solution for mail news. I'll make the send message Ctrl+J, and then smart users can use ctrl enter, and silly users can flame me for every time they accidental send a message w/ ctrl+enter.

<http://bugzilla.mozilla.org/showattachment.cgi?attach_id=11645#Ctrl+> says, ctrl-j is "Unix reserved shortcut, sort of like alt+tab in windows".

That's fine, i'll make it for windows since this bug claims to be windows. On linux i can try accel-enter like a well behaved bleh.

Please, add anymore non spesific windows stuff in this bug. Thanks

Frank, I am still waiting for your review... any other worry about this fix?

for some reason, my fix broke recently. Control key sequences are not recognized anymore. When I press CTRL+c, it generate a char code of 67 (uper case c) when apparently the application now expect to get a char code of 99 (lower case c).

Reassigning to trudelle. It looks like every time JF tries to fix this the code gets changed. Is there someone on XP Toolkit who can help us out?

->saari

nominating for nsbeta1. It sounds like if we want to make Ctrl-Enter work (which we do) that we need for this to get fixed.

Clearing milestone to get this triaged. MailNews folks need it. Adding access keyword.

CC'ing joki in hopes that he can take this. I'm kinda busy...

aaronl, would you be able to help on this...?

->aaronl

aaronl (or anyone!): Any chance of you getting to this? This patch has been sitting here for _six_months_ now, and it blocks a mostfreq bug... ducarroz: Can you make sure this patch still applies cleanly? Gerv

maybe this has already been fixed (by another patch) and nobody noticed?

Nope, it's still broken. I attach an updated test case. This rather breaks the correct working of Mail's Send shortcut (which some may consider to be a good thing...) Gerv

In addition to control-enter, control-home and control-end seem to be broken as well (windows-only). Control-Backspace was also broken but I think Tony has a fix for that. Tony--does your fix for backspace also fix the problems with enter, home, and end (and maybe others)?

yeah, however there is a seperate bug(73867) filed againts rods because on windows, backspace + CTRL does not generate the correct key code/char code. anthonyd

reassign to rods; he has other bugs like this (including a duplicate)

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

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

Please ignore the two #defines I turn off, I will remove them before checking in. The patch above fixes <ctrl> Return and <ctrl> Backspace. I don't see a way on Windows to fix the <ctrl>J problem itgenerates exactly the same event that <ctrl> enter does. Frank, please review.

Rods, look at my original patch(http://bugzilla.mozilla.org/showattachment.cgi?attach_id=13452), I was able to make any control keys working correctly on Windows using MapVirtualKey().

Ok, ducarroz previous patch wouldn't apply for some reason so I applied it by hand and that is what I just attached. I didn't use it before because I thought frank was worried about it, but Kathy said that was probably because we were close to RTM. Anyway, I tried it out and it seems to work great. I'll r=rods because it is ducarroz's patch.

sr=attinasi

fixed

Nope, I should have tested that fix more. The lastest patch can cause double character events to be egenerated. moving bug bug 0.9.3, unless somebody wants to fix the patch, because I don't have time right now.

Rod, if you can explain what you mean a little more, I can see if I can dig anything up.

yokoyama- can you verify this patch on your system and make sure it does not break non English keyboard, non western keyboard (say Russian/Greek) nor IME? I delegate the review to yokoyama

ctrl+enter is generating 0 as the keycode for me at the moment...Rod, shouldn't this work now with your patch (albeit with the sporadic problem you mention)? Or did you back it out, or what?

blake--did you check the "charcode" of the keypress event? I think enter should be generating a "character" rather than keycode. You might check another OS and see if your xul/js works somewhere else (?).

Shift+Enter and Alt+Enter both generate the correct keyCodes for me, but for Ctrl+Enter it's 0. Conversely, Shift+Enter and Alt+Enter generate charCodes of 0, but for Ctrl+Enter it's 106. Sounds like something got a little messed up here?

I can't verify bug 57967 until this bug is fixed...thanks!

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

I'd like to help, but I'm not quite sure where this is at. If someone can tell me where we're at with this, and even better provide a test case for all keys and key combinations that are currently broken, I can take a run at it. No promises, though.

moving to 0.9.6

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

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

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

This bug is probably why my patch for bug 79047 (shift+space to page up) doesn't work.

OK, I made up a more complete testcase that displays all keydown, keypress, and keyup events. One thing I notice is that when pressing a normal alpha key, the key is uppercase for keydown and keyup, but lowercase for keypress. I wonder if that's related to the Ctrl+j for Ctrl+Enter problem. 1) keydown charCode=65 keyChar=|A| 2) keypress charCode=97 keyChar=|a| 3) keyup charCode=65 keyChar=|A| The spacing in the output may look a little weird. I put a blank line after any keyup event, which gets a little weird if you have modifier keys pressed.

Is it just me, or is this really screwed up? Here's the keydown-keypressed-keyup sequence from pressing '\': 50) keydown charCode=220 keyChar=|Ü| 51) keypress charCode=92 keyChar=|\| 52) keyup charCode=220 keyChar=|Ü| Shouldn't the charCode be the same for all three events?

Dean, the code in keydown/keyup events is the keyboard scan code, required to detect special characters such as arrow keys. Keypress events generate characters. Mind you that doesn't explain what happened when I pressed ` on my UK keyboard: 1) keydown charCode=223 keyChar=|ß| 2) keypress charCode=0 keyChar=|<glyph cannot be copied>| 3) keypress charCode=96 keyChar=|`| 4) keyup charCode=223 keyChar=|ß|

I know this bug seems out of hand, but it blocks ctrl-enter for sending a message in windows. If a bug specific to ctrl-enter were filed, could that get a target milestone prior to 1.0?

It is intended that charcode be 0 unless it's a "keypress" event with a printable character. If a charcode is set, the keycode should *not* be set. Here are some examples: arrow keys would never generate a charcode (event type does not matter) charcode would be set for shift-m keypress event other events would set keyCode note: I think there are a few exceptions to handle 4.x compatibility (joki would know what the exceptions are). If people want this fixed, (since rods has pushed it off for the time being), I recommend you find someone who understands Windows native events and help them develop and test a patch.

This is part of my problem. I'd try to fix this, but I have no idea what's supposed to come through in what combinations. Can someone please generate a detailed description of when a charcode is supposed to come through, when a keychar is supposed to come through, and when they're both supposed to come through?

sorry I wasn't clearer above: you should never get *both* keycode and charcode set in the same event (possibly a few exceptions which you'd have to ask Joki tho). keyCode is set for everything except keypress events with actual characters (is that a simple enough rule?)

I am willing to try any testprograms on mixed Windows OS-ex (win98/NT/2000/XP at hand, although 98/2000 should be enough) to get you the right key events. Even Outlook and Outlook Express support it, even via a terminal server. Sadly I am not good enough at coding, so I better stay out of patching.

OK, Kathy was able to clear up a few things. My new test case displays the charCode, keyCode, and actual character. 1) keydown charCode=0 keyCode=77 character=|| modifiers=Ctrl+Shift 2) keypress charCode=77 keyCode=0 character=|M| modifiers=Ctrl+Shift 3) keyup charCode=0 keyCode=77 character=|| modifiers=Ctrl+Shift For keydown and keyup the character will most likely be garbage. I also don't display any keydown or keyup information for Ctrl, Shift, or Alt. I found it got in the way of debugging multiple keypresses.

Here is what happens in a standard Windows event loop, i.e. while (GetMessage(&msg, NULL, 0, 0)) { TranslateMessage(&msg); DispatchMessage(&msg); } The keyboard driver posts WM_KEYUP and WM_KEYDOWN (or WM_SYSKEYUP and WM_SYSKEYDOWN if the ALT key is or was down) messages to the application. These messages appear to map directly to keydown and keyup events. The wParam of the Windows message appears to map to the keyCode. The TranslateMessage function asks (via MapVirtualKeyEx) the keyboard layout (as returned by GetKeyboardLayout) to translate the keyCode into a charCode. If successful the char code is posted as the wParam of a WM_CHAR (or WM_SYSCHAR if the ALT key is down) message. Note that the TranslateMessage function will translate VK_RETURN to '\r' with or without SHIFT, or to '\n' with CTRL only. It might therefore be an idea not to call TranslateMessage (or equivalent) for VK_RETURN messages, so as to avoid confusion with CTRL+M and CTRL+J.

Is it possible to change the currently assigned keyboard accelerators (by tinkering with config files, but not source)? It may sound selfish, but since this bug's been around for over a year and doesn't look any closer to being resolved, I want to put a workaround in my system to make send now ctrl+j and make it work, and I'm sure many other users will work. This is a windows system so I'm not worried about Ctrl+J being reserved, it seems to just generate a \n in most other apps anyway. (What does it do in Unix? I've never heard of it after several years of Unix use, I suspect it's just another way to get a newline char just like Ctrl+H is another way to get backspace). Instructions as to what config files to mod or what settings to ad to user.js would be much appreciated.

I agree with Victor Bajanov. If this bug is far from fixed, please give outside testers a work-around, such as ctrl-j. If this bug can get fixed in the short term, it should get the priority it needs, being a serious regression from N4.

Requesting a new keybinding for sending mail is a different issue (not the same as this bug). Either a new bug should be filed (if an open bug doesn't already exist) or bug 56696 should be reopened. (The new bug is probably a preferred route rather than reopening a bug that isn't broken.) Assign a new bug for the keybinding to jglick@netscape.com.

Sorry - After reading all the pain behind bug 56696, I guess I was wrong to do anything other than cheer for this bug to get fixed. See the original entry for below - this bug was trying to get Ctrl-Enter to Send Mail, which is what is important.

Information on how to set customized key bindings is at http://www.mozilla.org/unix/customizing.html#keys It doesn't list specific mail/news commands like how to send now, because nobody involved with maintaining that file knows what the list of valid mail/news commands are. If someone figures it out, please update the customizing document, or send the info to me and I'll update it, since people are frequently asking for help setting up mail/news key bindings.

Unfortunately the custom bindings mechanism in nsXBLWindowHandler.cpp only appears to differentiate between browser and editor windows, but if it works then you need the command usually bound to Ctrl+Enter which is called cmd_sendWithCheck.

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

Note that the TranslateMessage function will translate VK_RETURN to '\r' with or without SHIFT, or to '\n' with CTRL only, and VK_BACK to '\b' with or without SHIFT, or to '\0177' with CTRL only, and VK_ESC to '\033' without SHIFT. It might therefore be an idea not to call TranslateMessage (or equivalent) for VK_RETURN, VK_BACK or VK_ESC messages, so as to avoid confusion with CTRL_+H, CTRL+J, CTRL+M and CTRL+[.

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

WORKAROUND for those who (like me) *must have* Ctrl+Enter working to send right now. The file to edit is platformHTMLBindings.xml, however it isn't in the .jar files in the chrome directory as the customising guide (link posted by Akkana) suggests. It's in res/builtin/ (at least on my system, which is a default installation). I added the following line to the browser section (adding it to the editor section does nothing). <handler event="keypress" key="j" modifiers="accel" command="cmd_sendWithCheck"/> This makes Ctrl+Enter pop up the dialog box shown in Bug 56696 (I was surprised, I thought the fix was introduced more recently than I downloaded Mozilla, but evidently not). To clarify, by "browser section" I mean the block opened by <binding id="browser"> I don't know what effect this will have on other parts of Mozilla - hitting Ctrl+Enter in the browser window seems to do nothing, I'll test what happens when it's pressed with the cursor in a form text field when I finish typing this (I bet you can guess which form and which text field =)). To answer the other obvious question, pressing Ctrl-j also prompts to send the message, and pressing Ctrl+Shift+j or Ctrl+Shift+Enter does nothing (in message composer window). Logical, since I didn't add a line for the send later command (which I never use anyway). Needless to say if you don't like Ctrl+Enter, you can use this to test out other potential shortcut keys and see if you like them - there was a lot of debate about whether Ctrl+Enter should be changed in bug 20336. Also, Akkana, could you please update the customising document, as I have no idea how to do it. Cheers, --Victor

Ctrl+Enter in form text field while using workaround also does nothing. (Logical, but the point of testing is to find illogical behaviour...)

Victor: Submitting forms via Ctrl-Enter in a Textarea is bug 102539.

OK, I got some emails informing me that the shortcut only worked if the text cursor wasn't in the message editing text area (but in the to/subject fields). To fix this, add the same line as before ALSO to the block opened by <binding id="editor"> Now you should have the line in both the editor and browser blocks. I suspect the browser block is used when the cursor is in the to/subject fields, and the editor block when the cursor is in the text editing window. Also, I don't know how the workaround will behave on Windows NT (or whether it will work at all). Also, I am using build 2001101117, make sure you have a build that has the fix to bug 56696 implemented.

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

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

Okay, seeing as how this bug is still helpwanted and there is an apparent workaround, can we get the workaround checked in? Should we open up another bug and attach the workaround info?

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

I'd rather not check in such a work-around. It only fixes one manifestation of this much-larger issue.

I agree, I didn't intend for the workaround to be checked in, only used by some people who read this forum =)

Yes, but meanwhile windows users can't hit ctrl-enter to send messages, and this bug is nowhere near being fixed. If there are no disadvantages, check in the workaround and take it out when this bug is fixed. I'm sure plenty of workarounds are already in mozilla--end user experience is what counts.

It is noted elsewhere that the key combination Ctrl+J is marked as reserved on unix platforms. I'm not sure whether this means that the workaround would simply not function on those platforms (which aren't an issue anyway as this bug doesn't apply to them) or would cause other problems on them (I guess the idea is that a window manager may use ctrl+j as a standard key combination, at which point it would never get as far as Mozilla's event handling).

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

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

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

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

Any chances this bug nomination is reconsidered? Looks like this is one of the most wanted.

I actually just nominated it. It has yet to be triaged.

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

Nominating for 0.9.9

--> me.

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

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

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

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

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

Just voted for this... maybe the amount of duplicates could also be a sign of something :)

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

It appears that this bug also breaks Ctrl+] and Ctrl+[ in the Mail compose window. (Shortcuts for increase and decrease indent). Can someone confirm this? If this is not the same problem, please re-open http://bugzilla.mozilla.org/show_bug.cgi?id=122366

Two observations, re. today's comments from Ere and Robert: Ctrl+[ and ctrl+] don't seem to work here (Moz. 0.9.7, on Win XP.) The number of duplicates is probably for two reasons: The menus say Ctrl+return will work (same for ctrl+[ and ctrl+]) and becuase anyone coming from Netscape 4.x will be used to ctrl+enter acting as send message.

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

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

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

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

--> back to rods.

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

Ctrl-Enter is quite a common key sequence for sending messages also in other programs than Netscape 4.x. Therefore I feel that this is one the small bugs that are really eating the acceptability of Mozilla/N6 by "converted" people. Now I'm really a bit frightened seeing that the target milestone is 1.2 (where did 0.9.9 go?). Is there anything I could do to help this? I can find my way through Windows programming (earning my living there..), so I could try to do some fixing if I were given the direction first...

Marking nsbeta1+

This just special cases the <ctrl><return> and makes sure it send a keycode of 13

The other patch was the wrong patch

Comment on attachment 68596 [details] [diff] [review] Correct patch r=kmcclusk@netscape.com Make sure ftang reviews this patch to confirm that it does not break IME.

Coincidently I have been working on this bug and had come up with the attached patch about the same time Rod did. I tested Rods patch and one thing I noticed is that Ctrl+J now works the same as Ctrl+Enter in the Mail Application. That is Ctrl+J sends mail. The fix I have attached catches the Ctrl+Enter in the onkeydown method and sends it to DispatchKeyEvent from there instead of sending it from the OnChar method. What I have determined is that, even in native Windows, if one processes the WM_CHAR message (OnChar method) one will always receive the same value for Ctrl+J and Ctrl+Enter. This is not true in the OnKeyDown or WM_KEYDOWN message. This is either a bug deep in Windows or in some OEM code somewhere. This patch will let Ctrl+enter pass to DispatchKeyEvent from KeyDown, and Ctrl+J will pass to DispatchKeyEvent from OnChar. Ctrl+Enter will also pass thru OnChar to DispatchKeyEvent, but since KeyDown happens first, the Ctrl+Enter will already have been processed before the key combination is sent to OnChar method. I have only tested this patch on Win NT, I have no access to any other systems. If anyone wants to try please be my guest. Also, if there are other Ctrl+ combinations that need to be fixed I will be happy to work on them. It's a little hard to discern what exactly needs to be fixed from the above messages. Seems like Ctrl+[ and Ctrl+] might be one. Please advise. Bernie

I suggest we give this bug to Bernie since he has a fix that differentiates between the two keys. There are times when we'll need to use Accel+J as well as Accel+Enter. Bernie can use this is one of his 3 bugs needed to gain CVS privileges. Bernie, I looked at the patch really briefly - I want to mention that we use PRInt32 instead of int, and we don't use prefix letters except for: g = global m = member variable a = method argument s = static I suggest you take a look at http://www.mozilla.org/projects/nspr/reference/html/index.html This describes the NSPR (Netscape Portable Runtime). NSPR helps give us platform independence. Chapter 2 talks about the types that we use.

Comment on attachment 68596 [details] [diff] [review] Correct patch Doesn't allow use of Ctrl+J

This is Bernie's patch plus an extra bit. His patch generates the key event correctly like theother special events. But those events don't generate the SM_CHAR which the <cntrl><enter> does. So the other part of this patch is to discard the what would be the KEY_PRESS event from the <ctrl><enter>

Rod, in the better patch, by throwing away ctrl+x0a, you will be throwing away ctrl+J keypresses. Is this what you want?

I am willing to test anything (honestly). Just send the binaries, I cannot compile and I am not good enough in C(++). *finally there is something moving with that bug*. Here I have access to Windows98 and WindowsXP, at work I have access to any version.

Bernie, I think we need to throw away the <ctrl>J and the expense of the <ctrl><return>, we cannot generate two KEY_PRESS events for a single key stroke.

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

A correct fix for this bug needs to make Ctrl+j available as well as Ctrl+Enter, each for their own separate commands.

I made the mistake of looking at a section of the windows keyboard handling code. The gist of it seems to be that the WM_KEYDOWN handler sends two events for keys that it doesn't think are going to be processed by the WM_CHAR handler. Wouldn't it be simpler for the WM_KEYDOWN handler to send both events? That way you could avoid turning ENTER into Ctrl+J by mistake. The ToAscii function might help here.

I am confused. Which scenario below is correct A or B? Scenario A: When a user presses a <ctr><enter> it sends: KEY_PRESS <ctrl>J and KEY_PRESS <ctrl><enter> Scenario B: When a user presses <ctrl>J it sends: KEY_PRESS <ctrl>J (only) When a user presses a <ctr><enter> it sends: KEY_PRESS <ctrl><enter> (only)

scenario B is the correct behavior Some of the test cases attached to this bug can help verification that we do the right thing (such as http://bugzilla.mozilla.org/attachment.cgi?id=55849&action=view) Be sure to test things like: a control-a control-shift-a control-j 3 control-2 control-shift-2 control-[ control-shift-] control-enter pageup shift-pagedown control-pageup down arrow control-down arrow end control-home control-delete control-backspace ... Also be sure to test the alt-menu behavior (alt-F activates File menu).

Comment on attachment 68642 [details] [diff] [review] better patch Rods is working on a better patch...

This is a little hacky, but it works for both <ctrl>J and <ctrl><enter>. If someone can use ToAscii and come up with a better patch I am all for that, but at the moment I don't have the time (poor excuse, I know) to figure that one out.

Rod, I will look into some alternatives on this today. In your message 136, assuming that the it you are reffering to is Windows then Scenario A is actually what is happening (see below). Scenario B is what we would like, I beleive. We need to look at what messages are generated by windows to understand this problem. To oversimplify this: Windows generates a WM_KEYDOWN and WM_KEYUP for every keyboard action. In ADDITION Windows generates a WM_CHAR message for keyboard keypresses that have character codes associated with them. In the case of the ctrl+enter combination, Windows generates a WM_KEYDOWN message with a virtual key code of 0x13 (enter) AND a WM_CHAR message with a key code of x0a. Which is also the key code (x0a) that is generated by the ctrl+J key press action. What windows is trying to do in the case of ctrl+enter is to pass a WM_CHAR message indicating a new line (\n which is also 0x0a). I will look at alternatives to processing these messages by using ToAscii, and/or looking at the use of TranslateMessage. TranslateMessage is the function that is changing the ctrl+enter virtual key code from x13 to x0a.

Note that keyboard access to the system menu requires TranslateMessage.

Bernie, you have summed up the issue exactly. Also, while you are poking around, note that <ctrl>[ doesn't work right either.

fwiw i filed bug 124843 about problems copying from the testcase

Ok, here is a patch that appears to fix the ctrl+enter and ctrl+[ and ctrl+] problems. The problem with ctrl+[ and ctrl+] seemed a little strange to me but I'm not that familiar with this product yet. The method that would process the keypress for these two (ctrl [ ]) needed a uniCode of the [ plus an upper case A (regardless of the shift key). I would like to look at this a little more but I'm somewhat lost finding the code that actually process the key press - can someone help with the location of this code? Anyhow, this patch seems to work for my testing, try it out on other platforms and let me know. Bernie

OK, here's what's probably a dumb question, and I've probably worked this out in my head before. Why are we even calling OnChar in WM_CHAR if wParam is 10 (x0a)? Shouldn't we be calling OnChar only for valid character keys? Or maybe a different question. if wParam in WM_CHAR is x0a how does that get translated to 106 (x6a) == 'j'?

Dean, Hope I can answer this easily. OnChar is called because a WM_CHAR message comes in from Windows. When one presses a ctrl+enter combination, Windows generates a WM_KEYDOWN message with a virtual key code of VK_RETURN (0x0d) and it generates a WM_CHAR message with a character code of 0x0a. (I think windows thinks it's helping by generating what would be the character representation of a new line (\n =0x0a) as a WM_CHAR message. The code always calls OnChar if a WM_CHAR message is received. Now the problem is: if one presses the key combination of ctrl+j Windows generates a WM_KEYDOWN message with a virtual key code of 0x0a AND a WM_CHAR message with a character code of 0x0a. Which is guess what, the same as it did when the operator pressed ctrl+enter. See the problem? So the OnChar needs to differentiate between the 0x0a generated by ctrl_enter and the one generated by the ctrl+j. Bernie McGuire

Thanks Bernie, that helped. At first, it sounded like we should skip WM_CHAR processing for any WM_KEYDOWN with a wParam < 'A'. I wrote a little Win32 app to investigate that, but I couldn't find any other Ctrl+<key> combination that triggered a WM_CHAR message. Looks like we still generate keypress events for key combinations that don't generate a WM_CHAR, which is fine. Instead of throwing away the WM_CHAR, shouldn't we be generating a keypress event with a charCode of 0 and a keyCode of 13? That's what I get when I press Enter without holding down Ctrl.

The patch looks good r=rods

Comment on attachment 69119 [details] [diff] [review] Patch to fix ctrl+enter AND ctrl+[ AND ctrl+] >+ static PRPackedBool mSkipWMCHARProcessing; > static BOOL sIsRegistered; > static BOOL sIsPopupClassRegistered; You've a tab there, I think. Why use PRPackedBool for a static member? Manipulating single bytes will likely be slower than a BOOL (as is used in the other static members), and I can't imagine that you'll save space outside of a struct/class: if you want to save space, you need to have more than one in a sequence. Also, you probably want to name that static member with an |s| prefix, rather than an |m|.

I don't think this patch is right. It won't generate a keypress at all for Ctrl+Enter, which is what we need. See the second half of comment 147.

Comment on attachment 69119 [details] [diff] [review] Patch to fix ctrl+enter AND ctrl+[ AND ctrl+] Did someone mention tabs? + if (mIsControlDown && (virtualKeyCode == 0x1d || virtualKeyCode == 0x1b)) { + // Left or Right square bracket + uniChar = virtualKeyCode -1 + 'A'; + virtualKeyCode = 0; + } Can you put this after tha A-Z test, and change the condition to <= 0x1F so that you catch other control codes such as Ctrl+\? Why did Ctrl+A to Ctrl+Z ever map to a-z :-( If they mapped to A-Z then you could have just used + if (mIsControlDown && virtualKeyCode <= 0x1F) { + // Control codes: A-Z, brackets, etc. + uniChar = (virtualKeyCode & 0x1F) + ('A' - 1); + virtualKeyCode = 0; + } Ctrl+Shift+- on my keyboard generates two keypress events :-)

Dean, This part of the patch, in OnKeydown, generates the keypress charcode 13: + // Fix for 50255 (<ctrl><enter> not working + // Send keypress event for <ctrl><enter> keyboard action + // Set mSkipWMCHARProcessing to true so the 0x0a char code sent to OnCHar + // will be thrown away. + if (mIsControlDown && aVirtualKeyCode==VK_RETURN ) + { + mSkipWMCHARProcessing = PR_TRUE; + DispatchKeyEvent(NS_KEY_PRESS, 0, aVirtualKeyCode, aKeyData); + } Bernie

Rod, What do you say about Mike's comments in #149. Did you have anything else in mind when you chose the PRPackedBool data type and member name? Should I make a change? Bernie

Neil, I could move and change the code as you suggest, but I'm not sure what exactly is going on here with other control codes. I was just trying to get the ctrl+square backets working. If there are other codes broken I will look into them if someone can tell me their function so I can test them. I would also like to look at the method that does the work for these key strokes if someone could give me some direction on where to find the code. For example, what does ctrl+/ do? Is it broken now? Bernie

I don't think anyone uses Ctrl+\ but it's definitely broken according to the test page: it generates an invalid code in sequence between the invalid Ctrl+[ and Ctrl+], so if you fix those two you might as well fix Ctrl+\ as well. As for Ctrl+^ and Ctrl+_ they're tricky because they're normally shifted keys: currently they generate two keypress events but I don't know which is wrong...

I was asked to sr this patch, but I am confused. First, the static must be changed to a PRBool and renamed, and the tabs have to be removed. Second, does the patch work or is there some contention about that? It looks liek there is a bit more work to do, so I'll wait for another patch.

I no longer have contention with Ctrl+Enter and the keypress event. I must have been smoking something weird yesterday. I think we should investigate Neil's concerns further, though. If it's easy to expand the Ctrl+square bracket fix out to other characters, then we should. Alternatively, we could fix only the Ctrl+Enter bug right away, and satisfy almost every Windows Mail & News user, and split the other Ctrl issues out to a new bug to investigate more thoroughly.

Here is an updated patch that addresses all the issues raised. Name has been changed, tabs removed, ctrl+\ and ctrl+_ and ctrl+^ to be processed. Ctrl+_ and ctrl+^ are now differeniated from ctrl+6 and ctrl+- in that the former are actually Ctrl+Shift+_ and Ctrl+Shift+^. Please advise on any other issues. Bernie

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

Will the Alt versions of all these keystrokes work? I'm curious how Ctrl+\ and other modified punct keys will fare on international keyboards. Perhaps someone can test that?

> Will the Alt versions of all these keystrokes work? No Ctrl+Alt keystrokes currently work in Win32 Mozilla. That's bug 69954.

>Will the Alt versions of all these keystrokes work? Aaron, I tested the Alt+ combination of these Ctrl+ combinations and the code generated the 'same' keypress events, with the same keycodes, the difference being the modifier was = Alt (which is what one wants I presume), not Ctrl. Naturally, the function assigned to the Ctrl+ combination did not execute when the Alt+ combination was pressed, which I don't think is desireable, is it? For example Ctrl+] is increase indent in the mail application. Alt+], sent a keypress event, but the increase indent function did not execute, it's not susposed to, is it?

It sounds like you're seeing the correct behavior all over. Have you looked at attachment 33327 [details] [diff] [review], which is the proposed fix for bug 69954? What do you think of the different way of determining mIsControlDown and mIsAltDown. It sounds like you don't need that with this fix.

Regarding my testing of the Alt+ keystrokes: I'm only testing on a US keyboard, and it seems like bug 69954 is for Ctrl+Alt combinations. Dean, I think you are correct, I dont need the fix for 69954 for this bug. But I will go to that bug and look at it next. Let's discuss that problem over there. Who knows what else needs to be done so we can close this (50255) bug out.

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

Ctrl+Enter seems to work now, which is a Very Good Thing. But I'm not getting a keypress event for Ctrl+Shift keypresses with non-alpha characters. eg. Ctrl+Shift+1 only generates keydown then keyup, the same for Ctrl+Shift+=. They do in the nightlies. And Bernie, contrary to what I said before, I say keep this bug separate from the fishcam bug. Fix this first, then let's worry about the other one.

I don't have a build but those of you that do should know that the correct keypress event for Ctrl+Shift+Hyphen is character="_" modifiers="ctrl,shift"

I'm confused, could someone let me know the scope of this bug. Is it to make all keypresses generate keypress events? If so, I need to talk to someone about the general design of keyboard event processing. I see things in the current code like: if this is the CTRL+SUBTRACT key, Dispatch a keypress event with the virtualKeyCode = virtualKeyCode-64. Why subtract 64 from the keycode? In another case like a Semicolon, comma, period etc, the keypress is Dispatched as is. There are other cases where the keycode is converted to a Unicode character before sending it on. I can get any keypress combination to generate an event, but I need to know what the code down line is expecting. I hope someone can give some guidance. Bernie McGuire

Ths scope is really just <ctrl><enter> if by chance you could get <ctr>[] to work great. I thought the patch http://bugzilla.mozilla.org/showattachment.cgi?attach_id=69119 worked fine for these issues, so... if so, say so, and Marc can sr it. This really needs to be checked in by Tues for 0.9.9 If there are other problems then open a different bug, but let's stop beating this to death.

I, at least, have outstanding questions about the 69119 patch. Could you address them?

As I said in comment 157, I'm fine in fixing only Ctrl+Enter in this bug and splitting the other issues off to another bug. After, of course, addressing any concerns Shaver has.

Mike, your concerns regarding the tabs and name and type were changed in the 69297 patch. Were there any other concerns? Bernie

Dean, lets either open another bug or add the requirements to bug 69954. I can address additional keypress issues when I create the patch for that bug. The changes will be in the same source module.

Comment on attachment 69297 [details] [diff] [review] Updated Patch to fix Ctrl+Enter and other Ctrl+ keystrokes Yeah, that's better. (PRBool? BOOL? Bah, who cares.)

Comment on attachment 69297 [details] [diff] [review] Updated Patch to fix Ctrl+Enter and other Ctrl+ keystrokes sr=attinasi

Comment on attachment 69297 [details] [diff] [review] Updated Patch to fix Ctrl+Enter and other Ctrl+ keystrokes r=dveditz

Thanks Bernie!

No, attachment 69297 [details] [diff] [review] is NOT ready to go. Does anyone even read comments before reviewing?? See comment 166, I applied it to my tree and it fixed Ctrl+Enter but broke a bunch of other key presses. Do NOT check this patch in. I think we should go back to something based on attachment 69119 [details] [diff] [review], plus Shaver's comments. Then a few people should test it, apparently including me since I seem to be the only person the did regression testing on the last patch, and we'll go from there.

Oh good. This already got checked in. Nothing like breaking existing functionality!!

Reopening per Dean's comments. While I'm here, a layman's question: why do we have to do so much special-casing and hacking to get this to work? Surely not every other win32 app has had to do this to make ctrl+enter (and ctrl+j?) work...?

Blake, I did a test app and the wParam of WM_CHAR is the same for both 'J' and Enter. That doesn't really answer your question, does it? I've done some thinking about the Ctrl+Enter problem specifically, and I've come up with a couple of alternative, perhaps simpler, approaches. 1. Check the state of the J key in WM_CHAR when the character code is 10 and Ctrl is pressed. if (wParam == 0x0a && IS_VK_DOWN(VK_CONTROL) && !IS_VK_DOWN('J')) OutputDebugString("Ctrl+Enter pressed\n"); I can see a slight drawback to this in that if there's somehow a processing delay before this gets called, the J key may have been released and the IS_VK_DOWN() call would return false. One place I could think of this bein disastrous is in mail. If we have an HTML formatting command with a Ctrl+J shortcut, the situation I just described could end up sending the message (Ctrl+Enter) when Ctrl+J is pressed. Having said all that, I similuated a processing delay before this "if", and IS_VK_DOWN still seemed to return the correct value for the state of the 'J' key at the start of the WM_CHAR processing. I'm still a little apprehensive about it, though. 2. Use the scan code from the lParam of the message to determine the actual key that was pressed. if (mIsControlDown && wParam == 0x0a) { // key pressed could be either J or Enter, because // both send a wParam of 0x0a when used with Ctrl. key = MapVirtualKey(LOBYTE(HIWORD(lParam)), 1); if (key == VK_RETURN) OutputDebugString("Ctrl+Enter pressed\n"); else OutputDebugString("Ctrl+J pressed\n"); } This works well for me, but I'm not sure how it works on on international keyboards. In theory it would be exactly the same, that's what MapVirtualKey() is for.

Would this be the bug to complain about ctrl++ not working? :-)

The reason this is a not problem for most windows apps is that they look for WM_KEYDOWN events for accelerators and WM_CHAR events for typing characters. The use of WM_CHAR allows the keyboard driver to compute the result of a shifted character e.g. shift+2 is " on a UK keyboard but @ on a US keyboard. However what Mozilla wants to be able to do is to generate keypress events for all keydown events. The keypress event (as far as I can determine from Linux IRC users) should wherever possible use the nearest relevant character, thus Ctrl+Shift+6 should generate a keypress event of charCode 94. Since this is a nonstandard use of the keyboard the only way I can currently see to do this is to only intercept WM_KEYDOWN events and perform manual character translation, possibly using the toAscii function, except for certain keys such as Enter, Backspace and Escape which should never be translated to characters, plus it is necessary to temporarily turn off Ctrl and Alt if no suitable mapping is found such as a charCode of less than 32.

What's the status of this bug? Is the patch getting backed out or not? If not, then why is this reopened instead of just openign other bugs?

If the currently checked in patch fixes the Ctrl-Enter for sending mail and does not cause equally bad or worse regressions elsewhere, I'd vote for opening other bugs.

This update to 69297 should allow ctrl+shift+numerics to pass thru as keystrokes as they did before. With this patch the following are fixed and/or still work: ctrl+enter ctrl+shift+enter ctrl++ ctrl+- ctrl+_ ctrl+] ctrl+[ NOTE: ctrl++ on the numeric keypad does not work and didn't work before this patch. If it needs to it is a different bug.

Sorry, patch 70134 was not diffed against the current trunk. This attachment is and does what is referenced above.

per test comment #137: the "down" arrow (wintel) doesn't work with Address auto-completion picker -- "up" arrow does work as expected. build #2002021803. thx -GA (pls forgive ignorance if due to completely different issue)

Garetta: that was due to the check in for bug 115686, and there's a fix posted in that bug.

I am really confused as to what is checked in and what isn't checked in.

Is this the bug that's causing plain old Enter to submit the "Edit Attachment as Comment" form when trying to review a patch in today's build?

Although I am not quite up to date on this bug, I don't think this is it. It's a different bug.

Rod, Patch 69297 was checked in and it fixed: Ctrl+Enter,Ctrl+Shift+Enter,Ctrl+[,Ctrl+],Ctrl+_ It did cause Ctrl+Shift+numeric (1,2,3,4,5,7,8,9,0) not to generate a keypress. NOTE Ctrl+numeric worked before and after patch 69297. The patch 70140 allows Ctrl+Shift+numeric will dispatch a keystroke, this patch needs to be tested and reviewed.

Comments Anyone: I used a static BOOL, sSkipWMCHARProcessing, set in OnKeyDown (for ctrl+enter) to allow me to bypass processing the same keystroke (ctrl+enter) in OnChar. In comment 181 a suggestion (#2) was made to possibly use MapVirtualKey() in the OnChar instead. Anyone know if using MapVirtualKey() as in suggestion #2, would work if an international keyboard was being used? What about keyboards that do not have a 'J', is there another key that generates a 0x0A code?

i think bug 106048 is either fixed by this, or should be.

"Anyone know if using MapVirtualKey() as in suggestion #2, would work if an international keyboard was being used? What about keyboards that do not have a 'J', is there another key that generates a 0x0A code?" My understanding is that MapVirtualKey will get us around the 0x0a problem. Once we're in there, it uses the scan code to determine the actual key that was pressed. <aside> what if we used MapVirtualKey for every key press, instead of trying to do keycode-specific processing? Would that be a Good Idea or a Bad Idea? </aside>

"<aside> what if we used MapVirtualKey for every key press, instead of trying to do keycode-specific processing? Would that be a Good Idea or a Bad Idea? </aside>" Probably would be a good idea but I'm not sure what you mean by 'instead of keycode-specific processing'. Do you mean, don't test the keycode just test the result of MapVirtualKey()? I think this whole area needs to be worked on. But before anyone does I think some answers to some of the questions I posed in comment 168, plus many more, need to be forthcomming. I think anyone with the interest and knowlege about MapVIrtualKey and Mozilla should jump right in here, code it up, and put this bug to bed.

If ctr++ isn't part of the "some control key sequences don't generate the correct event" bug (see summary), then what is going on with ctr++? Btw, I meant both the + on they numeric keypad and the + that's above the = (so ctrl+shift+=). Anyway, I'll file a new bug if you feel it doesn't fall within the summary's scope.

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

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

Marking nsbeta1-. The original issue has been fixed. ctrl-enter works.

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

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

This patch eliminates the need for using the PRBool sSkipWMCHARProcessing flag by using MapVirtualKey() as Dean suggested.

I looked over the patch and I'm a little concerned (I need to see more context to know if the patch is really ok or not). Please see my comments above (comment 137) for testing which should be done: http://bugzilla.mozilla.org/show_bug.cgi?id=50255#c137 The testing in comment 137 is a *MINIMUM* for any changes in this area of the code. No reviewer or super-reviewer of any patches in this area should permit changes without asking if those tests have successfully been performed.

Bernie, can you attach the patch in cvs diff -u format?

Sorry, here is the patch in dif -u format. NOTE this patch needs to be applied to the code in the current trunk. Bernie

Here are test results from the test cases in msg 137 (and more) using the 'better test case' HTML attahced to this bug: 1) keydown charCode=0 keyCode=65 character=| | 2) keypress charCode=97 keyCode=0 character=|a| 3) keyup charCode=0 keyCode=65 character=| | 4) keydown charCode=0 keyCode=65 character=| | modifiers=Ctrl 5) keypress charCode=97 keyCode=0 character=|a| modifiers=Ctrl 6) keyup charCode=0 keyCode=65 character=| | modifiers=Ctrl 7) keydown charCode=0 keyCode=65 character=| | modifiers=Ctrl+Shift 8) keypress charCode=65 keyCode=0 character=|A| modifiers=Ctrl+Shift 9) keyup charCode=0 keyCode=65 character=| | modifiers=Ctrl+Shift 10) keydown charCode=0 keyCode=74 character=| | modifiers=Ctrl 11) keypress charCode=106 keyCode=0 character=|j| modifiers=Ctrl 12) keyup charCode=0 keyCode=74 character=| | modifiers=Ctrl 13) keydown charCode=0 keyCode=51 character=| | 14) keypress charCode=51 keyCode=0 character=|3| 15) keyup charCode=0 keyCode=51 character=| | 16) keydown charCode=0 keyCode=50 character=| | modifiers=Ctrl 17) keypress charCode=50 keyCode=0 character=|2| modifiers=Ctrl 18) keyup charCode=0 keyCode=50 character=| | modifiers=Ctrl 19) keydown charCode=0 keyCode=50 character=| | modifiers=Ctrl+Shift 20) keypress charCode=50 keyCode=0 character=|2| modifiers=Ctrl+Shift 21) keypress charCode=64 keyCode=0 character=|@| modifiers=Ctrl+Shift 22) keyup charCode=0 keyCode=50 character=| | modifiers=Ctrl+Shift 23) keydown charCode=0 keyCode=221 character=| | modifiers=Ctrl 24) keypress charCode=93 keyCode=0 character=|]| modifiers=Ctrl 25) keyup charCode=0 keyCode=221 character=| | modifiers=Ctrl 26) keydown charCode=0 keyCode=221 character=| | modifiers=Ctrl+Shift 27) keyup charCode=0 keyCode=221 character=| | modifiers=Ctrl+Shift 28) keydown charCode=0 keyCode=13 character=| | modifiers=Ctrl 29) keypress charCode=0 keyCode=13 character=| | modifiers=Ctrl 30) keyup charCode=0 keyCode=13 character=| | modifiers=Ctrl 31) keydown charCode=0 keyCode=33 character=| | 32) keypress charCode=0 keyCode=33 character=| | 33) keyup charCode=0 keyCode=33 character=| | 34) keydown charCode=0 keyCode=34 character=| | modifiers=Shift 35) keypress charCode=0 keyCode=34 character=| | modifiers=Shift 36) keyup charCode=0 keyCode=34 character=| | modifiers=Shift 37) keydown charCode=0 keyCode=33 character=| | modifiers=Ctrl 38) keypress charCode=0 keyCode=33 character=| | modifiers=Ctrl 39) keyup charCode=0 keyCode=33 character=| | modifiers=Ctrl 40) keydown charCode=0 keyCode=40 character=| | 41) keypress charCode=0 keyCode=40 character=| | 42) keyup charCode=0 keyCode=40 character=| | 43) keydown charCode=0 keyCode=40 character=| | modifiers=Ctrl 44) keypress charCode=0 keyCode=40 character=| | modifiers=Ctrl 45) keyup charCode=0 keyCode=40 character=| | modifiers=Ctrl 46) keydown charCode=0 keyCode=35 character=| | 47) keypress charCode=0 keyCode=35 character=| | 48) keyup charCode=0 keyCode=35 character=| | 49) keydown charCode=0 keyCode=36 character=| | modifiers=Ctrl 50) keypress charCode=0 keyCode=36 character=| | modifiers=Ctrl 51) keyup charCode=0 keyCode=36 character=| | modifiers=Ctrl 52) keydown charCode=0 keyCode=46 character=| | modifiers=Ctrl 53) keypress charCode=0 keyCode=46 character=| | modifiers=Ctrl 54) keyup charCode=0 keyCode=46 character=| | modifiers=Ctrl 55) keydown charCode=0 keyCode=8 character=| | modifiers=Ctrl 56) keypress charCode=127 keyCode=0 character=|?| modifiers=Ctrl 57) keyup charCode=0 keyCode=8 character=| | modifiers=Ctrl 58) keydown charCode=0 keyCode=67 character=| | modifiers=Ctrl 59) keypress charCode=99 keyCode=0 character=|c| modifiers=Ctrl 60) keyup charCode=0 keyCode=67 character=| | modifiers=Ctrl 61) keydown charCode=0 keyCode=13 character=| | modifiers=Ctrl 62) keypress charCode=0 keyCode=13 character=| | modifiers=Ctrl 63) keyup charCode=0 keyCode=13 character=| | 64) keydown charCode=0 keyCode=49 character=| | modifiers=Ctrl+Shift 65) keypress charCode=49 keyCode=0 character=|1| modifiers=Ctrl+Shift 66) keyup charCode=0 keyCode=49 character=| | modifiers=Ctrl+Shift 67) keydown charCode=0 keyCode=50 character=| | modifiers=Ctrl+Shift 68) keypress charCode=50 keyCode=0 character=|2| modifiers=Ctrl+Shift 69) keypress charCode=64 keyCode=0 character=|@| modifiers=Ctrl+Shift 70) keyup charCode=0 keyCode=50 character=| | modifiers=Ctrl+Shift 71) keydown charCode=0 keyCode=51 character=| | modifiers=Ctrl+Shift 72) keypress charCode=51 keyCode=0 character=|3| modifiers=Ctrl+Shift 73) keyup charCode=0 keyCode=51 character=| | modifiers=Ctrl+Shift 74) keydown charCode=0 keyCode=52 character=| | modifiers=Ctrl+Shift 75) keypress charCode=52 keyCode=0 character=|4| modifiers=Ctrl+Shift 76) keyup charCode=0 keyCode=52 character=| | modifiers=Ctrl+Shift 77) keydown charCode=0 keyCode=53 character=| | modifiers=Ctrl+Shift 78) keypress charCode=53 keyCode=0 character=|5| modifiers=Ctrl+Shift 79) keyup charCode=0 keyCode=53 character=| | modifiers=Ctrl+Shift 80) keydown charCode=0 keyCode=54 character=| | modifiers=Ctrl+Shift 81) keypress charCode=54 keyCode=0 character=|6| modifiers=Ctrl+Shift 82) keypress charCode=94 keyCode=0 character=|^| modifiers=Ctrl+Shift 83) keyup charCode=0 keyCode=54 character=| | modifiers=Ctrl 84) keydown charCode=0 keyCode=55 character=| | modifiers=Ctrl+Shift 85) keypress charCode=55 keyCode=0 character=|7| modifiers=Ctrl+Shift 86) keyup charCode=0 keyCode=55 character=| | modifiers=Ctrl+Shift 87) keydown charCode=0 keyCode=56 character=| | modifiers=Ctrl+Shift 88) keypress charCode=56 keyCode=0 character=|8| modifiers=Ctrl+Shift 89) keyup charCode=0 keyCode=56 character=| | 90) keydown charCode=0 keyCode=57 character=| | modifiers=Ctrl+Shift 91) keypress charCode=57 keyCode=0 character=|9| modifiers=Ctrl+Shift 92) keyup charCode=0 keyCode=57 character=| | modifiers=Ctrl+Shift 93) keydown charCode=0 keyCode=48 character=| | modifiers=Ctrl+Shift 94) keypress charCode=48 keyCode=0 character=|0| modifiers=Ctrl+Shift 95) keyup charCode=0 keyCode=48 character=| | modifiers=Ctrl+Shift 96) keydown charCode=0 keyCode=109 character=| | modifiers=Ctrl+Shift 97) keypress charCode=95 keyCode=0 character=|_| modifiers=Ctrl+Shift 98) keyup charCode=0 keyCode=109 character=| | modifiers=Ctrl+Shift 99) keydown charCode=0 keyCode=61 character=| | modifiers=Ctrl+Shift 100) keypress charCode=61 keyCode=0 character=|=| modifiers=Ctrl+Shift 101) keyup charCode=0 keyCode=61 character=| | modifiers=Ctrl+Shift 102) keydown charCode=0 keyCode=67 character=| | modifiers=Ctrl 103) keypress charCode=99 keyCode=0 character=|c| modifiers=Ctrl 104) keyup charCode=0 keyCode=67 character=| | modifiers=Ctrl 105) keydown charCode=0 keyCode=67 character=| | modifiers=Ctrl 106) keypress charCode=99 keyCode=0 character=|c| modifiers=Ctrl 107) keyup charCode=0 keyCode=67 character=| | modifiers=Ctrl

Why does Ctrl+Shift+2 generate two keypresses, one for '2' and one for '@', and Ctrl+Shift+3 only one, for '3'? Currently Ctrl+Shift+2 generates a keypress only for '@', and Ctrl+Shift+3 generates no keypress at all. Is the correct behavior to send the character as '2' or as '@'?

Bernie--At a glance, I see some inconsistencies: Lines 20 and 21 (these should be the same? or only one?) Line 63 (why no modifier as 61 and 62 have?) Lines 68 and 69 worry me as 20/21 above Lines 81 and 82 worry me as well Line 89 (why no modifier; similar to 63)

Dean: @ is not shift+2 on all keyboards. On a UK layout, shift+2 is " and @ is shift+', which is 2 keys to the right of return on the middle row... So, on a UK keyboard, crtl+shift+2 should generate a keypress of '\"' and not one for '@' As to whether it should send '2' or '@' (or possibly '\"'...) I would say both, cos is ctrl+@ (or ctrl+" on a UK keyboard) meant, or is ctrl+shift+2 what's wanted? Don't you just love all the different keyboard layouts out there? :-)

Brade & Dean, + Line 63 (why no modifier as 61 and 62 have?) Must have been a cut & paste problem, here is a new result: 1) keydown charCode=0 keyCode=13 character=| | modifiers=Ctrl 2) keypress charCode=0 keyCode=13 character=| | modifiers=Ctrl 3) keyup charCode=0 keyCode=13 character=| | modifiers=Ctrl + Line 89 (why no modifier; similar to 63) Must have been a cut & paste problem, here is a new result: 4) keydown charCode=0 keyCode=56 character=| | modifiers=Ctrl+Shift 5) keypress charCode=56 keyCode=0 character=|8| modifiers=Ctrl+Shift 6) keyup charCode=0 keyCode=56 character=| | modifiers=Ctrl+Shift + Lines 20 and 21 (these should be the same? or only one?) + Lines 68 and 69 worry me as 20/21 above Both of the above are the same. I think we should just pass in the virtual key code for the keypressed (see above comment on confusion between keyboards) - not the character (2 or @). I think this is the whole point of virtual key codes. Dean and Brady what do you say, I'll make this change. I don't know enough about the down stream processing of these keystrokes to say. Please advise. + Lines 81 and 82 worry me as well Same as above.

Unless I'm reading it wrong, http://mozilla.org/editor/key-event-spec.html tells me that line 21 is correct and line 20 is not. Similar to pressing Shift+2, Ctrl+Shift+2 should generate a charCode of '@'.

I agree with your reading of that page. That means that ctrl+shift+2 cannot be reliably trapped in keyPress, because, with a UK keyboard, ctrl+shift+2 will generate a charCode of '\"'. from http://www.ifbbc.com/html/____________8_different_countr.htm we can see that, on the keyboards illustrated, ctrl+shift+2 will generate either '\"' or 'é' (French) and to generate a US-style ctrl+shift+2 keyPress would need different key combinations on 5 of the 8 keyboards shown, some on which would include AltGr, and so it wouldn't be a ctrl+shift modifier any more...

Dean, Thanks for that link, I've been wanting to read something like that. I can see your interpretation of the write up. I could interpret it that way also but I'm not sure. IMHO the writeup does not clearly address how to handle 'commands' or 'accelerators'. It does say that if a-z 0-9 are a command that the modifiers need to remain set. It implys that charcode should be set not the key code. Again, IMHO, this is a problem under Windows. Based on my testing over the last month or so here are some of the issues not addressed. Depending on what key is pressed, what modifier (Alt, Ctrl, Shift..), Windows may or may not generate a WM_CHAR message. This is important because the WM_CHAR message is the only message where the actual char code (ex @ in the case of shift+2) is sent to the application. Now one might think that one could get the keypress in the WM_KEYDOWN event and process it there for these other cases. True enough, but in WM_KEYDOWN one is working with virtualkey codes and scan codes. If MapVirtualKey is used to convert the virtual key code to a char code, the code that is returned in the key code in UN-Shifted state. The only place to get the char code in a shifted state is in the WM_CHAR message from windows. Now, you might ask why did line 21 have an @ in it. Well, this was pure luck and coincidence. When ctrl+shift+2 was pressed a zero was sent to WM_CHAR as the virtualKeycode and there was some code there that did this: if(mIsControlDown && (virtualKeyCode <= 0x1A)) // Ctrl+A Ctrl+Z, see Programming Windows 3.1 page 110 for details { // need to account for shift here. bug 16486 if ( mIsShiftDown ) uniChar = virtualKeyCode - 1 + 'A' ; This code resulted in uniChar (0 - 1 + 65) = 64 which is the Ascii code for @. Also, from my testing, I've noticed that once the Ctrl, Alt, or Ctrl+Alt keys are pressed it is difficult if not impossible to differentiate between shifted and unshifted characters. As an aside, this is the part of the reason the 'fish cam bug' exists. In that bug, the XML is expecting a lowercase 'f' in combination with Ctrl+Alt. Once Ctrl+Alt are pressed only a Keydown event is generated by windows with a virtual key code of 70 key. MapVirtualKey will only convert the the virtual key code to an upper case 'F'. All this and more, I haven't even mentioned the mapping that is done from DOM virtual key codes to ns key codes in OnKeyDown in the call to MapFromNativeToDOM (aVirtualKeyCode), shows me that we are getting pretty far afield from the original problem this bug was to address - CTRL+ENTER. Since Ctrl+Enter has been fixed I think this bug should be closed and a new one opened to address this issue of "Proces all Keystrokes as specified in http://mozilla.org/editor/key-event-spec.html". This has been mentioned many times before so I think I'll open the new bug and put a link to it here when I've set the new bug up.

please re-nominate once the final patch is r/sr'd and baked on the trunk.

this is still important, dean/brade: wanna give it a shot?

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

Has something for this bug recently been checked in? According to bug 198976, the ctrl-enter shortcut no longer works on any platform.

I just ran acorss this bug with build Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.4a) Gecko/20030401. Specifically in regards to sending a mail message (Ctrl-return). This didn't exist in any prior releases for me.

That's a different bug. See bug 198976, fixed after your build.

I've just created bug 203231, re: On Windows using a US keyboard layout, Ctrl+Shift+2 and Ctrl+Shift+6 generate a keypress event while none of the other Ctrl+Shift+<number> keyboard commands create an event. The test case in this bug shows the same behavior. The latest patch for _this_ bug has rotted quite a bit. I'm willing to help write a new one, but I'm not 100% sure what the right answer is. Some thoughts: 0) Regardless of how you read http://mozilla.org/editor/key-event-spec.html, the following paragraph: """Each keyboard input results in 3 separate key events: KeyDown, followed by KeyPress, followed KeyUp. All platforms must generate these events in this sequence for every keyboard input (except IME events). """ along with the test case in bug 203231 show that the current build (well, I'm at (Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4b) Gecko/20030418) is not up to the spec (note that I've tested on 0.9.5 and the spec wasn't being followed in a different way there -- sometimes, _two_ keypress events were being generated). (more on this below). 1) Ctrl+Shift+2 is ill-specified since depending on the keyboards some 2's can be shifted and others can't, and the native keycode sent will differ even if they have digits "at the bottom". I'm willing to live with having to listen for Ctrl+Shift+@ and the like. I'm even willing to put "Ctrl+Shift+@" in the acceltext for the relevant menuitems, even though that will suprise many users. At least it will be _correct_ even for UK or German users. One concern is that it won't be correct for French users, where (IIRC) @ is not shifted, and for which the "natural" event would be "Ctrl+@". (more on this later) 2) Here are some interesting bits that should be helpful in building a patch. in nsWindow.cpp, around line 3137, if you change: else if ((mIsControlDown && !mIsShiftDown) && ((( NS_VK_0 <= aVirtualKeyCode) && (aVirtualKeyCode <= NS_VK_9)) || to: else if (mIsControlDown && ((( NS_VK_0 <= aVirtualKeyCode) && (aVirtualKeyCode <= NS_VK_9)) || Then you can rescussitate the 0.9.5 behavior: Ctrl+Shift+1 -> keypress Ctrl+Shift+1 Ctrl+Shift+2 -> keypress Ctrl+Shift+2 AND -> keypress Ctrl+Shift+@ Ctrl+Shift+3 -> keypress Ctrl+Shift+3 Ctrl+Shift+4 -> keypress Ctrl+Shift+4 Ctrl+Shift+5 -> keypress Ctrl+Shift+5 Ctrl+Shift+6 -> keypress Ctrl+Shift+6 AND -> keypress Ctrl+Shift+^ Ctrl+Shift+7 -> keypress Ctrl+Shift+7 Ctrl+Shift+8 -> keypress Ctrl+Shift+8 Ctrl+Shift+9 -> keypress Ctrl+Shift+9 If you follow up with a change like adding: case NS_VK_0 : asciiKey = mIsShiftDown ? ')' : '0'; break; case NS_VK_1 : asciiKey = mIsShiftDown ? '!' : '1'; break; case NS_VK_2 : asciiKey = mIsShiftDown ? '@' : '2'; break; case NS_VK_3 : asciiKey = mIsShiftDown ? '#' : '3'; break; case NS_VK_4 : asciiKey = mIsShiftDown ? '$' : '4'; break; case NS_VK_5 : asciiKey = mIsShiftDown ? '%' : '5'; break; case NS_VK_6 : asciiKey = mIsShiftDown ? '^' : '6'; break; case NS_VK_7 : asciiKey = mIsShiftDown ? '&' : '7'; break; case NS_VK_8 : asciiKey = mIsShiftDown ? '*' : '8'; break; case NS_VK_9 : asciiKey = mIsShiftDown ? '(' : '9'; break; to the switch statement around line 3152, you end up with: Ctrl+Shift+1 -> keypress Ctrl+Shift+! Ctrl+Shift+2 -> keypress Ctrl+Shift+@ AND -> keypress Ctrl+Shift+@ Ctrl+Shift+3 -> keypress Ctrl+Shift+# Ctrl+Shift+4 -> keypress Ctrl+Shift+$ Ctrl+Shift+5 -> keypress Ctrl+Shift+% Ctrl+Shift+6 -> keypress Ctrl+Shift+^ AND -> keypress Ctrl+Shift+^ Ctrl+Shift+7 -> keypress Ctrl+Shift+& Ctrl+Shift+8 -> keypress Ctrl+Shift+* Ctrl+Shift+9 -> keypress Ctrl+Shift+( Which is obviously a hack since we're going back from "keyboard-neutral" VK_2 to a keyboard-specific character, so if at all this would have to be done using some Windows API to do the mapping back according to the right keyboard layout. [I'm not proposing the above as a patch, only as tidbits]. A possibly scary proposal: - on US Keyboards, Ctrl+Shift+2 should yield a Ctrl+@ event. - on UK Keyboards, Ctrl+Shift+2 should yield a Ctrl+" event. - on French Keyboards, Ctrl+Shift+é should yield a Ctrl+2 event. In other words, the fact that the Shift key is down should be "absorbed" by the fact that the character was changed by the shift. It's the only way that I can see making sense of the international soup. A more immediate question: Does anyone understand why Ctrl+Shift+2 and Ctrl+Shift+6 generate keypress events but the others don't (or with my patch two events instead of one)?

About Ctrl+6 and other control keys... TranslateMessage tries to match a key with a character. This means that Ctrl+A to Ctrl+Z (and Ctrl+Shift+A to Ctrl+Shift+Z) generate character codes of 1-26, then codes 0 and 27-31 are generated by Ctrl+@, Ctrl+[, Ctrl+\, Ctrl+], Ctrl+^ and Ctrl+_. Now some of these keys are shifted. Thus the question: should Ctrl+^ generate Ctrl+^ or Ctrl+Shift+6? (Obviously on an international keyboard with an unshifted ^ Ctrl+^ should generate Ctrl+^). The other point is that Mozilla tries to guess what TranslateMessage will do, which is why someone suggested to use MapVirtualKeyEx in the WM_KEYDOWN handler and ignore WM_CHAR altogether.

Neil, thanks for the answer, it's very helpful. (Sheepishly, I have to admit that reading the whole history of the bug helps as well). Your comments about TranslateMessage helped me understand the WM_CHAR_LATER macro -- in fact, I can hackily fix things for US keyboards by applying both of the patches mentioned earlier and tweaking that macro to skip VK_2 and VK_6. It's clearly the wrong approach. [...] > Thus the question: should Ctrl+^ generate Ctrl+^ or Ctrl+Shift+6? > (Obviously on an international keyboard with an unshifted ^ > Ctrl+^ should generate Ctrl+^). I'd like a ruling on that from someone with authority. It seems clear to me that there's no way to have key bindings for such keys work regardless of keyboard if anything but Ctrl+^ is generated, and a ruling on this issue is clearly needed before a patch can be proposed. > The other point is that Mozilla tries to guess what TranslateMessage will do, > which is why someone suggested to use MapVirtualKeyEx in the WM_KEYDOWN > handler and ignore WM_CHAR altogether. Right. To the extent that I'm grokking things, it does seem like the Right Thing To Do. It would mean ignoring the extra events generated by Windows in its TranslateMessage process, and generating every keypress event on keydown. I may poke around with more experimental patches. Does anyone here know enough about the unicode handling that's going on in OnChar() to provide some test cases I could use w.r.t. Unicode input? --david

An additional data point: on windows at least, 'a' generates 'keypress-a', but 'A' generates 'keypress-Shift-A'. As I've argued, I think that's wrong, but I'm worried that changing that will be a backwards compatibility nightmare. Does anyone on this bug know what the situation is on other platforms? --david

A question for those who understand MapVirtualKeyEx better than I do. - on WM_KEYDOWN (OnKeyDown()), we get a virtual key code. - MapVirtualKeyEx lets us get from the virtual key code either to the scan code (which corresponds to the position on the keyboard regardless of layout), OR to the "unshifted character value". What this means is that when the virtual key code corresponding to Ctrl+Shift+6 is being run through MapVirtualKeyEx, what I get back is 6! I'm not sure how one is supposed to figure out that 6 with-the-shift-key-down corresponds to ^ on my keyboard, without accessing the current key state and using ToAsciiEx, which seems wrong (the keyboard state may have changed since the key was pressed, no?). In other words, what is the actual logic behind TranslateMessage?