Last Comment Bug 432951 - Ctrl+'foo' doesn't send same charCode as Meta+'foo' on Cocoa
: Ctrl+'foo' doesn't send same charCode as Meta+'foo' on Cocoa
Status: ASSIGNED
[key hell]
:
Product: Core
Classification: Components
Component: Keyboard: Navigation (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal with 3 votes (vote)
: ---
Assigned To: Masayuki Nakano [:masayuki] (Mozilla Japan)
:
: Andrew Overholt [:overholt]
Mentors:
: 416227 (view as bug list)
Depends on: 631165
Blocks: 429510
  Show dependency treegraph
 
Reported: 2008-05-08 23:08 PDT by Masayuki Nakano [:masayuki] (Mozilla Japan)
Modified: 2011-03-08 19:51 PST (History)
13 users (show)
mtschrep: blocking1.9-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1.0 (13.27 KB, patch)
2008-05-09 02:17 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Splinter Review
Patch v2.0 (13.64 KB, patch)
2008-05-09 09:39 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Splinter Review
attachment 320198 minus the fix for bug 433030 (9.60 KB, patch)
2008-05-09 12:38 PDT, Karl Tomlinson (:karlt)
karlt: review+
Details | Diff | Splinter Review
Patch v3.0 (10.04 KB, patch)
2008-05-09 22:17 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Splinter Review

Description Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-08 23:08:57 PDT
Ctrl+'foo' sends ASCII control character or normal character. If ASCII control character, we replace the charCode to alphabet now. Therefore, on most keyboard layout, Ctrl+'foo' and Meta+'foo' send same charCode. However, if the keyboard layout doesn't send numeric when 'foo' is numeric, the users cannot use Ctrl+Num on the web apps. And also non-Alphabet and non-Numeric keys have same problem.

E.g., on attachment 316954 [details], at pressing Ctrl+1,2,3,4,5,7,8, the charCode is numeric, but they are not same char at Meta+same key. And Ctrl+6,9,0 are not printable characters.
Comment 1 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-09 02:17:38 PDT
Created attachment 320151 [details] [diff] [review]
Patch v1.0

fix.
Comment 2 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-09 09:39:07 PDT
Created attachment 320198 [details] [diff] [review]
Patch v2.0

ok. we should prefer the shiftedChar at Eszett key case on German.
I.e., Cmd+Shift+SS and Ctrl+Shift+SS send '?', not '/'. (but '/' is sent as an alternative charCode for internal.)
Comment 3 Karl Tomlinson (:karlt) 2008-05-09 12:38:55 PDT
Created attachment 320246 [details] [diff] [review]
attachment 320198 [details] [diff] [review] minus the fix for bug 433030

This applies either to trunk or on top of attachment 320245 [details] [diff] [review] with -p1 -F8
Comment 4 Karl Tomlinson (:karlt) 2008-05-09 14:59:30 PDT
Comment on attachment 320246 [details] [diff] [review]
attachment 320198 [details] [diff] [review] minus the fix for bug 433030

+    // German SS key has 3 char.
+    testKey({layout:"German", keyCode:27, ctrl:1, chars:"\u00df", unmodifiedChars:"\u00df"},
+            "\u00df", true);

"chars" here should be the "characters" field of the NSEvent.
Should it be "3"?

(NSPR_LOG_MODULES=nsCocoaWidgets:5 may be helpful.)
Comment 5 Josh Aas 2008-05-09 16:05:30 PDT
Nominating blocking1.9, I am not sure we need to take it but we need to make a decision.
Comment 6 Karl Tomlinson (:karlt) 2008-05-09 16:35:22 PDT
Comment on attachment 320246 [details] [diff] [review]
attachment 320198 [details] [diff] [review] minus the fix for bug 433030

+          PRUint32 unmodifiedCh =
+            [unmodChars length] > 0 ? [unmodChars characterAtIndex:0] : 0;

Just do this when [unmodChars length] = 0.

+      PRBool replaceCharCode = outGeckoEvent->isMeta &&
+               !(outGeckoEvent->isControl || outGeckoEvent->isAlt);

Looks like you don't need the outGeckoEvent->isControl any more.

+      // When Ctrl key is pressed, the most keys send a control characters
+      // in ASCII. Then we should replace the charCode with Cmd key pressed time
+      // char. And also some keys send a normal character to us. But when the
+      // character can be inputted without Ctrl, we should replace charCode too.

"When Ctrl key is pressed, most keys send control characters in ASCII.  In
 that situation, we should replace the charCode with that produced with the
 Cmd key.  Also, for some keys, Ctrl is used to send (alternative) normal
 characters to us.  But we can also replace the character in the situations
 where the character can be input without Ctrl."

+        // XXX if relaceCharCode is false, we should consume Ctrl key here.
+        // Because editors of Gecko don't handle the key events with Ctrl key.

"replaceCharCode"

+        // On some keys on some keyboard layouts, the ASCII character is only
+        // changed the position, so, we should always replace if the character
+        // is ASCII character.

This sentence makes it sound there are some keyboard layouts for which we
should not replace when the character is ASCII.  Are you saying that all
keyboard layouts have all ASCII printable characters that can be produced with
Ctrl, available on some other key (as far as you can tell)?

If that's what you mean and the other issues are resolved then I'm happy with
this (but I'm hoping on a plane so won't be available for a day or two).

Please make sure "chars" is correct in all the tests, then make a case based
on the tests that fail without this patch.
Comment 7 Mike Schroepfer 2008-05-09 16:39:07 PDT
Talked to Karl a bit - he thinks the least risky path is not to take this. 
Since we are trying to close down for RC1 I'm going to minus this.
Comment 8 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-09 21:38:47 PDT
(In reply to comment #6)
> +        // On some keys on some keyboard layouts, the ASCII character is only
> +        // changed the position, so, we should always replace if the character
> +        // is ASCII character.
> 
> This sentence makes it sound there are some keyboard layouts for which we
> should not replace when the character is ASCII.  Are you saying that all
> keyboard layouts have all ASCII printable characters that can be produced with
> Ctrl, available on some other key (as far as you can tell)?

Yes.

Looks like some keyboard layout switch the keyboard layout to US layout only some keys at Ctrl being pressed (E.g., Spanish 'ñ' -> ':', ';' -> '"'). 

I'm thinking that the editor of Gecko doesn't accept the Ctrled inputting. And the editor of xcode and safari are so too. It seems that we can replace the charCode always if Ctrl is pressed. However, the editor of cocoa (native input field) accepts Ctrled inputting. It's strange...

> Please make sure "chars" is correct in all the tests, then make a case based
> on the tests that fail without this patch.

O.K. I'm checking that.
Comment 9 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-09 22:17:18 PDT
Created attachment 320331 [details] [diff] [review]
Patch v3.0

KCHR tests don't work now.
Comment 10 Atsushi Sakai 2008-05-21 05:55:47 PDT
Is this related to Bug 416227?
Comment 11 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-21 07:08:55 PDT
(In reply to comment #10)
> Is this related to Bug 416227?

yeah, right.
Comment 12 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-21 07:09:13 PDT
*** Bug 416227 has been marked as a duplicate of this bug. ***
Comment 13 Karl Tomlinson (:karlt) 2008-05-28 01:44:31 PDT
Comment on attachment 320331 [details] [diff] [review]
Patch v3.0

+        // On some keys on some keyboard layouts, the ASCII character is only
+        // changed the position, so, we should always replace if the character
+        // is ASCII character.
+        replaceCharCode = outGeckoEvent->charCode < 0x80 ||

I'm trying to understand this.

I can understand replacing the charCode < 0x20.

I think you are saying that 0x20 <= charCode < 0x80 are being replaced under
the assumption that the user could input those character is some other way.
Would this comment describe your thinking?:

  "ASCII control codes (<0x20) are not useful.

   ASCII characters 0x20 to 0x7f can be input without the Control key on most
   keyboard layouts, so, if the charCode is ASCII assume that the user intends
   the Control key to indicate a shortcut rather than any modification of the
   character."

+          (unshiftedChar && unshiftedChar == outGeckoEvent->charCode) ||
+          (shiftedChar && shiftedChar == outGeckoEvent->charCode);

What is you motivation for this code?  Is this to handle Shift being ignored
in the "characters" field of the NSEvent?  Or is it a quick test to see
whether the character can be generated without Control?  Or is it there
because of Arabic/Persian layouts providing strange results for unshiftedChar?

(In reply to comment #8)
> I'm thinking that the editor of Gecko doesn't accept the Ctrled inputting. And
> the editor of xcode and safari are so too. It seems that we can replace the
> charCode always if Ctrl is pressed. However, the editor of cocoa (native input
> field) accepts Ctrled inputting. It's strange...

Do you think we should perhaps just always ignore "characters" when Ctrl is
down?

+        // Arabic/Persian keybaord layout don't return correct key for
+        // unshiftedChar on 10.4.

correct "character"

+            replaceCharCode = outGeckoEvent->charCode == unmodifiedCh ||
+                              (shiftedChar && shiftedChar == unmodifiedCh) ||
+                              (unshiftedChar && unshiftedChar == unmodifiedCh);

What is the purpose of the (un)shiftedChar == unmodifiedCh tests?

+    testKey({layout:"Arabic", keyCode:18, ctrl:1, chars:"\u0661", unmodifiedChars:"\u0661"},
+            "\u0661");
+    testKey({layout:"Arabic", keyCode:18, ctrl:1, chars:"\u0661", unmodifiedChars:"\u0661"},
+            "1", true);

Looks like there is an argument missing on the first testKey.
This should activate, right?
Comment 14 Martin Stubenschrott 2009-01-23 04:45:04 PST
Any update on this? Many users of my extensions are complaining that the ctrl-] mapping does not work for them on the Mac while it does on Windows/Linux. Do I really need to add hacks to my extension just to have it work on all supported platforms?
Comment 15 Masayuki Nakano [:masayuki] (Mozilla Japan) 2009-01-23 08:17:14 PST
I'll restart the key input related works in 1.9.2 cycle.

But I have a question for comment 14. Shouldn't your extension use Cmd key instead of Ctrl key? Generally, shortcut keys on Mac should not use Ctrl key.
Comment 16 Ted Pavlic 2009-01-23 08:19:36 PST
Additionally, a control-modified space character sends charCode 64 (which is @). So <C-@> and <C-S-Space> look identical to Firefox.

On the other hand, <M-@> and <M-S-Space> are distinct.

There's clearly lots wrong here.
Comment 17 Ted Pavlic 2009-01-23 08:21:51 PST
(In reply to comment #15)
> But I have a question for comment 14. Shouldn't your extension use Cmd key
> instead of Ctrl key? Generally, shortcut keys on Mac should not use Ctrl key.

The extension (Vimperator) mimics Vim, and so Vim keybindings are expected (i.e., lots of Cntrl).

I don't understand this aversion FirefOS/X devs have to using Cntrl. Is there a good reason for it? 

Nevertheless, if the user passes a Cntrl key, it doesn't seem unreasonable for Firefox to represent it uniformly across all platforms.
Comment 18 Ted Pavlic 2009-01-23 08:33:16 PST
On a related note, I was under the impression that "Option" and "Alt" were identical as far as Firefox was concerned. However, because Mac inserts accented characters with the Option key, the Alt+CHAR keycodes are a mess.

It would be nice if Firefox could give plugin devs the ability to decipher Option+CHAR as Option+CHAR. (for example, Option+7 inserts a paragraph character (¶), and Firefox reports that Alt+(charCode 182) was pressed; figuring out that Alt+(182) is identical to Alt+(55) is nontrivial)
Comment 19 Masayuki Nakano [:masayuki] (Mozilla Japan) 2009-01-23 08:45:46 PST
See bug 306585 for the Option cases. If it will be fixed, the key elements will work fine.

# if you are using charCode value of key events, you cannot internationalize your application, see https://developer.mozilla.org/en/Gecko_Keypress_Event
Comment 20 Ted Pavlic 2009-01-23 09:21:38 PST
(In reply to comment #16)
> Additionally, a control-modified space character sends charCode 64 (which is
> @). So <C-@> and <C-S-Space> look identical to Firefox.

Actually, if you "diff <C-@> <C-S-Space>", you get:

176c176
< originalTarget: [object HTMLInputElement]
---
> originalTarget: [object XULElement]
182,183c182,183
< rangeOffset: 0
< rangeParent: [object HTMLDivElement]
---
> rangeOffset: 1
> rangeParent: [object XULElement]

However, I don't know what these represent and I can't find specifications for them on MDC. I'm particularly interested in "rangeOffset".

> On the other hand, <M-@> and <M-S-Space> are distinct.

Doing a "diff <M-@> <M-S-Space>" reveals that only the charcodes and "which"es differ, as expected (64 with @, 32 with space).

Note You need to log in before you can comment on or make changes to this bug.