432951 - Ctrl+'foo' doesn't send same charCode as Meta+'foo' on Cocoa

Status: NEW

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

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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] (he/him)(JST, +0900)]

Attachment: Patch v1.0

fix.

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: 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)]

Attachment: attachment 320198 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)]

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]

Nominating blocking1.9, I am not sure we need to take it but we need to make a decision.

Comment 6

[Karl Tomlinson (:karlt)]

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]

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] (he/him)(JST, +0900)]

(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] (he/him)(JST, +0900)]

Attachment: Patch v3.0

KCHR tests don't work now.

Comment 10

[Atsushi Sakai]

Is this related to Bug 416227?

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #10)
> Is this related to Bug 416227?

yeah, right.

Comment 13

[Karl Tomlinson (:karlt)]

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]

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] (he/him)(JST, +0900)]

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]

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]

(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]

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] (he/him)(JST, +0900)]

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]

(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).

Comment 21

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Resetting assignee which I don't work on in this several months.