429510 - Web apps cannot handle Ctrl+foo/Alt+foo key on keypress event

Status: RESOLVED FIXED

(Core :: Widget, defect)

Description

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

bug 359638 breaks Web app compatibility, I think.

See bug 359638 comment 146.

Now, we don't replace the charCode of keypress event always. However, e.g., on Win32, Ctrl+'foo' doesn't give the charCode of 'foo'. Therefore, web apps cannot handle Ctrl+'foo' on keypress event, probably. (I don't test it, but it should be so.)

But like bug 414130, there may be some trouble cases.

So, I think that we should replace the charCode at on all widget code (or on nsDOMKeyboardEvent::GetCharCode??) when the original charCode is less than 0xFF. (If some keyboard layout can input an ASCII character only when Ctrl/Alt is pressed, my idea is broken. However, I think that such case is not there.)

If you have some information and opinions, please tell me.

Comment 1

[Damon Sicore (:damons)]

Masayuki, I'm not sure what exactly you are proposing we block on here.  Sounds like you have specific ideas that should have been addressed in the other bugs.  Can you please explain a bit more and re-nom?

Comment 2

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

Damon:

I changed the charCode value of keypress event at Ctrl/Alt/Meta(Command) key being pressed in bug 359638. The reason is for bug 414130. That means Ctrl/Alt/Meta(Command) can be a part of "normal" character inputting key combination. Then, we should not modify the charCode value (current behavior).

So, the Web apps that are checking event.charCode on keypress event/handler are broken by bug 359638. We should go back to use the old charCode value for the compatibility.

Comment 3

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

Attachment: GTK2 part v1.0

First, I created the gtk2 part patch. Because win32/cocoa's patch conflict to other patches. I'll work on them too after the current other review processing are finished. The patch must be simple like this :-)

Karl:

If the charCode is not inputtable ASCII character, I replace the charCode to non-modified character. But if the current kb layout is not latin char inputtable, I re-replace the charCode to non-modified latin char.

However, if the non-latin char is not same as both the non modified characters of the current kb layout, I don't replace it. Because such case is similar to bug 414130.

Comment 4

[Damon Sicore (:damons)]

OK.  Let's get this patch in.  +'ing.

Comment 5

[Karl Tomlinson (:karlt)]

Comment on attachment 316378 [details] [diff] [review]
GTK2 part v1.0

Can you give an example of a situation where this is needed, please?

The Ctrl-key combinations that I've tried don't seem to produce control
characters.

The only necessary change that I'm aware of with GTK is making ShiftLock
handling consistent.  (Currently charCode depends on ShiftLock but
alternativeCharCharCodes don't.)

-                    altCharCodes.mUnshiftedCharCode =
-                        IsBasicLatinLetterOrNumeral(ch) ? ch : 0;

Why are you removing the IsBasicLatinLetterOrNumeral test?

Comment 6

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

(In reply to comment #5)
> (From update of attachment 316378 [details] [diff] [review])
> Can you give an example of a situation where this is needed, please?
> 
> The Ctrl-key combinations that I've tried don't seem to produce control
> characters.

I'm not sure. Isn't there suce case on all keyboard layouts on Linux?

> The only necessary change that I'm aware of with GTK is making ShiftLock
> handling consistent.  (Currently charCode depends on ShiftLock but
> alternativeCharCharCodes don't.)

o.k. ShiftLock is same as CapsLock?

> 
> -                    altCharCodes.mUnshiftedCharCode =
> -                        IsBasicLatinLetterOrNumeral(ch) ? ch : 0;
> 
> Why are you removing the IsBasicLatinLetterOrNumeral test?

Oh, this is my mistake, sorry.

Comment 7

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

Attachment: GTK2 part v2.0

Checking the CaplsLock state. But I couldn't test with AWERTY layout...

Comment 8

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

Attachment: Windows part v1.0

We don't need to check the CapsLock state. Because Win32 gives the CapsLocked characters to us if that is locked.

Comment 9

[Karl Tomlinson (:karlt)]

(In reply to comment #6)
> ShiftLock is same as CapsLock?

Same physical key but slightly different functionality:

http://en.wikipedia.org/wiki/Capslock#Shift_lock

Comment 10

[Karl Tomlinson (:karlt)]

Comment on attachment 316536 [details] [diff] [review]
GTK2 part v2.0

(In reply to comment #6)
> (In reply to comment #5)
> I'm not sure. Isn't there suce case on all keyboard layouts on Linux?

I can't comment for all.  But such situations are not the norm.

I don't think we've done anything beyond nsConvertCharCodeToUnicode in the
past and I haven't seen any such issues.

If there is a different behavior on some layout, I'd like to first understand
what and why, before deciding the best way to handle it.

(Also I doubt the "if (event.charCode)" would pass if the character were not
printable.)

+GetLatinCharForStates(nsAlternativeCharCode &aChars,
+                      PRBool aShiftIsPressed, PRBool aCapsLocked)

This wasn't really what I meant in comment 5.

(In reply to comment #5)
> The only necessary change that I'm aware of with GTK is making ShiftLock
> handling consistent.  (Currently charCode depends on ShiftLock but
> alternativeCharCharCodes don't.)

The issue has little to do with whether the characters are Latin or numerals
or printable.  It is just that GetCharCodeFor doesn't consider ShiftLock state
while event.charCode does.  I can put something together for this GTK issue,
if you are happy for me to do so?

Comment 11

[Karl Tomlinson (:karlt)]

Comment on attachment 316544 [details] [diff] [review]
Windows part v1.0

+          // If the charCode is not ASCII character, we should replace the
+          // charCode with ASCII character. But don't replace the charCode
+          // when the charCode is not same as unmodified characters. In such
+          // case, Ctrl or Alt is used for a part of character inputting key
+          // combination like Shift.

+              IsSameCharInputting(uniChars, numOfUniChars,
+                                  unshiftedChars, numOfUnshiftedChars) ||
+              IsSameCharInputting(uniChars, numOfUniChars,
+                                  shiftedChars, numOfShiftedChars))) {
+            ResetInputData(uniChars, shiftStates);
+            numOfUniChars = numOfShiftStates = 1;
+            uniChars[0] = mIsShiftDown ? shiftedLatinChar : unshiftedLatinChar;
+            shiftStates[0] = currentState;

This doesn't feel to me like the right thing to do.  Can we be certain that no
Web apps will be looking for (non-ASCII) characters in the local language?

With the current behavior, characters from any script (including Latin) can be
produced by selecting the appropriate layout (and most people will be able to
switch to a Latin layout when necessary).  However, with this proposed
behavior, only Latin characters could be produced.

> !isInputtableChar

It looks like this should be handled here:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/widget/src/windows/nsKeyboardLayout.cpp&rev=3.12&mark=152,154,163#144

Have you seen some situations for which gKbdLayout.GetUniChars doesn't provide
printable characters, but there are printable characters without Alt or Ctrl?
If so, why isn't this code already providing those printable characters?

Comment 12

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

(In reply to comment #11)
> (From update of attachment 316544 [details] [diff] [review])
> +          // If the charCode is not ASCII character, we should replace the
> +          // charCode with ASCII character. But don't replace the charCode
> +          // when the charCode is not same as unmodified characters. In such
> +          // case, Ctrl or Alt is used for a part of character inputting key
> +          // combination like Shift.
> 
> +              IsSameCharInputting(uniChars, numOfUniChars,
> +                                  unshiftedChars, numOfUnshiftedChars) ||
> +              IsSameCharInputting(uniChars, numOfUniChars,
> +                                  shiftedChars, numOfShiftedChars))) {
> +            ResetInputData(uniChars, shiftStates);
> +            numOfUniChars = numOfShiftStates = 1;
> +            uniChars[0] = mIsShiftDown ? shiftedLatinChar :
> unshiftedLatinChar;
> +            shiftStates[0] = currentState;
> 
> This doesn't feel to me like the right thing to do.  Can we be certain that no
> Web apps will be looking for (non-ASCII) characters in the local language?

I think there are no such Web apps. Because such Web apps cannot works fine on Fx2.

> With the current behavior, characters from any script (including Latin) can be
> produced by selecting the appropriate layout (and most people will be able to
> switch to a Latin layout when necessary).  However, with this proposed
> behavior, only Latin characters could be produced.

yeah, but that behavior makes the incompatibility with Fx2. If you want the incompatibility, it makes damage for Web app's internationalization. If Web app developers want to handle (to override) Ctrl+C, they need to think all keyboard layout. But it's impossible. However, with the Fx2 behavior, they just need to think whether the key is 'c'.

Comment 13

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

(In reply to comment #10)
> I can put something together for this GTK issue,
> if you are happy for me to do so?

Yes. Please.

Comment 14

[Karl Tomlinson (:karlt)]

(In reply to comment #12)
> (In reply to comment #11)
> > With the current behavior, characters from any script (including Latin)
> > can be produced by selecting the appropriate layout (and most people will
> > be able to switch to a Latin layout when necessary).  However, with this
> > proposed behavior, only Latin characters could be produced.
> 
> yeah, but that behavior makes the incompatibility with Fx2. If you want the
> incompatibility, it makes damage for Web app's internationalization.

Fx2 behavior wasn't completely consistent.  (I'll add some details.)

> If Web app developers want to handle (to override) Ctrl+C, they need to
> think all keyboard layout. But it's impossible.

Not impossible (but maybe more difficult).
If web app developers want to detect Ctrl+<key-corresponding-to-latin-C> then
keydown and keyup events could be used as they have key codes rather that
character codes.

Other browsers (IE6, Safari) don't let web apps override shortcut keys through
"keypress" so I suspect web app developers might have trouble trying to do
this anyway.

> [...] they just need to think whether the key is 'c'.

My understanding was that not all keyboards have Latin letters written on
them.  So there could be some advantage in being able to use shortcuts based
on the letters on the keyboard.

Comment 15

[Karl Tomlinson (:karlt)]

Attachment: charCode test

Results from typing with a Greek keyboard:

FF2 on Mac

                    φ (966)
              Shift Φ (934)
Ctrl                f (102)
Ctrl          Shift F (70)
                    [Find intercepts Cmd+F]
     Meta     Shift F (70)
                    [Option combinations do not set isAlt but instead generate
                     the alternate character.] 

                    ψ (968)
              Shift Ψ (936)
     Meta           c (99)
     Meta     Shift C (67)
Ctrl                c (99)
Ctrl          Shift C (67)

FF2 on Windows

    	            φ (966)
    	      Shift Φ (934)
Ctrl	            f (102)
Ctrl	      Shift F (70)
    	  Alt       φ (966)
    	  Alt Shift F (70)

FF2 on Linux

    	            φ (966)
    	      Shift Φ (934)
Ctrl	            φ (966)
Ctrl	      Shift Φ (934)
    	  Alt       φ (966)
    	  Alt Shift φ (966)


I suspect the behavior resulted from what the OS provides and what the
application wanted for its shortcuts (hence the exception with Alt on
Windows), rather than too much thought about what web apps would want.

More often than not Latin characters were sent when shortcut modifiers were
down, so maybe that's what people expect.  I just don't feel like I have
enough information to make this decision.

Is there someone with experience in l10n and web app development from whom we
could seek advice?

Comment 16

[Axel Hecht [:Pike]]

Asked for help in .l10n.

Comment 17

[Damjan Georgievski]

> FF2 on Linux
>                     φ (966)
>               Shift Φ (934)
> Ctrl                φ (966)
> Ctrl          Shift Φ (934)
>           Alt       φ (966)
>           Alt Shift φ (966)

This behaviour, the CTRL-* one, was very unfortunate on Fx2.. For example, if you spend a lot of time in your non-latin layout (like I do), you can't easily use ctrl-c, ctrl-v to copy/paste some text, or ctrl-t to open a new tab, etc.

Fx3 solved this problem, although I'm experiencing regressions in some nightly builds (using 2008041904 now and works fine).

On the other hand, since Alt is used for accessing menus with the accesskeys - which must be localized - it's better that alt-ф should still work like the Fx2 primer above.

Note, Im only speaking about the Firefox application itself, not about web apps. But I'd expect webapps to work the same as the desktop app, so the same applies.

Comment 18

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

Damjan:

Note that this change should affect to only the Web apps. UI of Gecko applications can handle both characters. I.e., Ctrl+Latin and Ctrl+Localized Char.

Karl:

(In reply to comment #14)
> (In reply to comment #12)
> > (In reply to comment #11)
> > > With the current behavior, characters from any script (including Latin)
> > > can be produced by selecting the appropriate layout (and most people will
> > > be able to switch to a Latin layout when necessary).  However, with this
> > > proposed behavior, only Latin characters could be produced.
> > 
> > yeah, but that behavior makes the incompatibility with Fx2. If you want the
> > incompatibility, it makes damage for Web app's internationalization.
> 
> Fx2 behavior wasn't completely consistent.  (I'll add some details.)

Thank you, I didn't know the behavior of Fx2 on Linux. The behaviors should be more consistent on Fx3 on the three platforms.

I still think that the Fx2 on Win32 and Mac behavior is better than the current nightly build and Fx2 on Linux behavior.

Comment 19

[Karl Tomlinson (:karlt)]

It seems to me that if a web app wants to check whether a _key_ is pressed
(rather than a character) then it should be using keyCode (which is only
available in keydown/keyup event) rather than a charCode (keypress events).

However, maybe there are some web apps that are using keypress.
(Mozilla even uses the keypress event.)

What is possibly more likely an issue is that some XUL apps might be using
charCode, and that may be a reason to send Latin charCodes for what look like
accel keys.

I haven't found evidence of localized accel key characters (from a quick look)
so it seems that having only Latin characters available for accel keys seems
unlikely to be a problem.

Access keys (often used with Alt) however are often localized, and if there is
a javascript implementation of access keys (not using nsEventStateManager)
then it might expect characters from any script.

Masayuki:

How would you feel about sending Latin characters when the events look like
accel keys (usually Ctrl but Cmd on Mac), while keeping the Unicode character
for Alt?

That would be similar to the FF2 Windows behavior, but I'd make Shift-Alt
consistent with Alt unless there is a good reason to be different.
(I haven't thought about Control on Mac.)

Only replacing charCode with a Latin letter when the modifier is not consumed
(as you have done by checking against the shifted/unshifted char) sounds
sensible.  Can you think about whether anyCharMessagesRemoved should be
considered, please?  Does that give an indication about whether the character
is for insertion (or is just part of a shortcut)?

Comment 20

[Karl Tomlinson (:karlt)]

(In reply to comment #19)
> I haven't found evidence of localized accel key characters

This may be some evidence

bug 177508 comment 15

Behnam, do you have any thoughts on whether local or Latin characters should be made available for accel keys (normally Ctrl as distinct from access keys with Alt)?

Comment 21

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

(In reply to comment #19)
> Masayuki:
> 
> How would you feel about sending Latin characters when the events look like
> accel keys (usually Ctrl but Cmd on Mac), while keeping the Unicode character
> for Alt?
> 
> That would be similar to the FF2 Windows behavior, but I'd make Shift-Alt
> consistent with Alt unless there is a good reason to be different.
> (I haven't thought about Control on Mac.)
> 
> Only replacing charCode with a Latin letter when the modifier is not consumed
> (as you have done by checking against the shifted/unshifted char) sounds
> sensible.  Can you think about whether anyCharMessagesRemoved should be
> considered, please?  Does that give an indication about whether the character
> is for insertion (or is just part of a shortcut)?

I agree to your worry that is accesskey Javascript implementation. However, it seems that there are risk of compatibility with Fx2. (Especially, this is late-compat issue.)

How about this? We replace the charCode if the event is not character insertion. And also we will add one or more new APIs for web developers on nsIDOMNSUIEvent. E.g., nsIDOMNSUIEvent::unmodifiedCharCode ?

Comment 22

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

mmm, I think, ideally, nsIDOMKeyEvent::charCode should be the current behavior. And the new APIs should provide the latinCharCode. But there is the compatibility issue after the final beta...

The Karl's suggestion may be good point between compatibility and i18n.

If we will add the new APIs after Gecko1.9, then, both Alt+foo and Ctrl+foo behavior should be same. But it might be no problem on the different behaviors between Alt and Ctrl...

So, I have another suggestion.

1. We are using Karl's suggestion now.
2. We will add the APIs after Gecko1.9. (latinCharCode is better?)

Comment 23

[Damon Sicore (:damons)]

So, are we still blocking on this issue?

Comment 24

[Eyal Rozenberg]

(In reply to comment #17)
> This behaviour, the CTRL-* one, was very unfortunate on Fx2.. For example, if
> you spend a lot of time in your non-latin layout (like I do), you can't easily
> use ctrl-c, ctrl-v to copy/paste some text, or ctrl-t to open a new tab, etc.

The bug is not really just about web apps, it's about all Mozilla apps
themselves... shouldn't the name be changed accordingly?

Comment 25

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

(In reply to comment #23)
> So, are we still blocking on this issue?

Yes. I still think so.

Comment 26

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

Attachment: Windows part v2.0

o.k. We should use Karl's suggestion now. We don't have much time.

This patch also has the part of bug 429970 and bug 429898. Their original author is Karl. And I changed some points of that.

Comment 27

[Damon Sicore (:damons)]

Does this issue affect ctrl-tab and ctrl-shift-tab to change tabs?  Wondering if bug 430499 is a dup of this one.  

Comment 28

[Karl Tomlinson (:karlt)]

Comment on attachment 317679 [details] [diff] [review]
Windows part v2.0

+          if (towupper(unshiftedChars[0]) != DOMKeyCode ||
+              towupper(shiftedChars[0]) != DOMKeyCode) {

Maybe we will need this.  See bug 429970 comment 5.

+              case VK_OEM_COMMA:  ch = ','; break;
+              case VK_OEM_PERIOD: ch = '.'; break;

I think this is fine.  What are these used for?

(In reply to comment #26)
> This patch also has the part of bug 429970 and bug 429898.

I find it easier to look at smaller patches where possible, and the
interaction between these bugs does not seem to prevent that.  It's also nice
to have patches in the bugs that they fix.

+static void
+CopyInputData(PRUint16* aChars, PRUint8* aShiftStates,

Unused.

+static PRBool
+IsSameCharInputting(const PRUint16* aChars1, const PRUint32 aNumChars1,

I think we can do better with this name.  This is really a wide strncmp.  If
there is no such existing function to use then how about "StringNEqual"?

+        if (mIsControlDown) {
+          PRUint8 currentState = mIsControlDown ? eCtrl : eAlt;

Don't test mIsControlDown twice.

+            ResetInputData(uniChars, shiftStates);

Why is this necessary?  Does something read past numOfUniChars?

-
+      printf("%x %c\n", uniChar, uniChar);

I assume you are going to remove this.

Comment 29

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

(In reply to comment #28)
> +              case VK_OEM_COMMA:  ch = ','; break;
> +              case VK_OEM_PERIOD: ch = '.'; break;
> 
> I think this is fine.  What are these used for?

I'm not sure. But they can be localized in some keyboad layouts.

> +static PRBool
> +IsSameCharInputting(const PRUint16* aChars1, const PRUint32 aNumChars1,
> 
> I think we can do better with this name.  This is really a wide strncmp.  If
> there is no such existing function to use then how about "StringNEqual"?

'N' gives me to think. Is not there a better (shorter) word of "case insensitive"?

> -
> +      printf("%x %c\n", uniChar, uniChar);
> 
> I assume you are going to remove this.

Yes, of course.

Comment 30

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

Attachment: Windows part v2.1

Comment 31

[Karl Tomlinson (:karlt)]

Comment on attachment 317841 [details] [diff] [review]
Windows part v2.1

(In reply to comment #28)
> +          if (towupper(unshiftedChars[0]) != DOMKeyCode ||
> +              towupper(shiftedChars[0]) != DOMKeyCode) {
> 
> Maybe we will need this.  See bug 429970 comment 5.

I need to know your intentions for alternativeCharCodes with CapsLock.

If alternativeCharCodes *should not* depend on CapsLock state (as seems to be
the current implementation) then the case-insensitive handling is not
necessary.  (My initial feeling is that alternativeCharCodes should depend on
CapsLock but I might be persuaded otherwise.)

The behavior of towupper depends on current locale but I'm not sure exactly
how.  My fear is that it may depend on the default character set.  Are you
sure this is OK?  Maybe that's OK if all character sets have Latin letters
with ASCII codes?

+            shiftedLatinChar = unshiftedLatinChar = DOMKeyCode;
             unshiftedLatinChar += 0x20;

If alternativeCharCodes *should* depend on CapsLock state then this should be
modified to also depend on CapsLock (as well as the GetUniCharsWithShiftState
calls).

(In reply to comment #29)
> (In reply to comment #28)
> > +              case VK_OEM_COMMA:  ch = ','; break;
> > +              case VK_OEM_PERIOD: ch = '.'; break;
> > 
> > I think this is fine.  What are these used for?
> 
> I'm not sure. But they can be localized in some keyboad layouts.

I don't want to take new code without a known use case.
So leave these out for now unless there is a better reason to include them.

Comment 32

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

(In reply to comment #31)
> (From update of attachment 317841 [details] [diff] [review])
> (In reply to comment #28)
> > +          if (towupper(unshiftedChars[0]) != DOMKeyCode ||
> > +              towupper(shiftedChars[0]) != DOMKeyCode) {
> > 
> > Maybe we will need this.  See bug 429970 comment 5.
> 
> I need to know your intentions for alternativeCharCodes with CapsLock.

I think that alternativeCharCodes should depend on CapsLock(ShiftLock) and NumLock if it is possible.

Comment 33

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

Attachment: Windows part v3.0

preferring the CapsLock state.

Comment 34

[Karl Tomlinson (:karlt)]

Comment on attachment 318103 [details] [diff] [review]
Windows part v3.0

(In reply to comment #33)
> preferring the CapsLock state.

Sounds good, thanks.

+            if ((::GetKeyState(VK_CAPITAL) & 1) != 0)

Just test capsLockState rather than call GetKeyState again.

(In reply to comment #31)
> The behavior of towupper depends on current locale but I'm not sure exactly
> how.  My fear is that it may depend on the default character set.  Are you
> sure this is OK?  Maybe that's OK if all character sets have Latin letters
> with ASCII codes?

I may be worrying about nothing here, but I'd like some reassurance.
I'm looking for an explanation of how towupper depends on the current locale,
so that we know that it will always do the right thing here.

Comment 35

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

(In reply to comment #34)
> (In reply to comment #31)
> > The behavior of towupper depends on current locale but I'm not sure exactly
> > how.  My fear is that it may depend on the default character set.  Are you
> > sure this is OK?  Maybe that's OK if all character sets have Latin letters
> > with ASCII codes?
> 
> I may be worrying about nothing here, but I'd like some reassurance.
> I'm looking for an explanation of how towupper depends on the current locale,
> so that we know that it will always do the right thing here.

Oops, sorry for I forgot this one.

Comment 36

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

Attachment: Windows part v3.1

renaming StringCaseSensitiveEquals -> StringCaseInsensitiveEquals.
using NS_ToUpper for comparing. (Note that we cannot use ToUpperCase here.)

Comment 37

[Karl Tomlinson (:karlt)]

Comment on attachment 318137 [details] [diff] [review]
Windows part v3.1

(In reply to comment #36)
> using NS_ToUpper for comparing.

I can't see NS_ToUpper(PRUint16) defined anywhere so it looks like this would
be truncating for NS_ToUpper(char).

> (Note that we cannot use ToUpperCase here.)

I'm curious as to why?

Is this a possibility?

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/intl/unicharutil/util/nsUnicharUtils.h&rev=1.22&mark=59#53

> renaming StringCaseSensitiveEquals -> StringCaseInsensitiveEquals.

Have you seen Ctrl change the case of the character?
If it doesn't and so this functions doesn't need to be case-insensitive then
we could do the logic ourselves here:

+          if (NS_ToUpper(unshiftedChars[0]) != DOMKeyCode ||
+              NS_ToUpper(shiftedChars[0]) != DOMKeyCode) {
+            shiftedLatinChar = unshiftedLatinChar = DOMKeyCode;
+            if (capsLockState)
+              shiftedLatinChar += 0x20;
+            else
+              unshiftedLatinChar += 0x20;
+          }

with something like

           shiftedLatinChar = unshiftedLatinChar = DOMKeyCode;
           if (capsLockState)
             shiftedLatinChar += 0x20;
           else
             unshiftedLatinChar += 0x20;

	   if (unshiftedLatinChar == unshiftedChars[0] &&
	       shiftedLatinChar == shiftedChars[0]) {
	       shiftedLatinChar = unshiftedLatinChar = 0;
           }

+                     StringCaseInsensitiveEquals(uniChars, numOfUniChars,
+                                                 unshiftedChars,
+                                                 numOfUnshiftedChars) ||
+                     StringCaseInsensitiveEquals(uniChars, numOfUniChars,
+                                                 shiftedChars,
+                                                 numOfShiftedChars))) {

I think this should be something like

StringCaseInsensitiveEquals(uniChars, numOfUniChars,
+                           mIsShiftDown ? shiftedChars : unshiftedChars,
+                           mIsShiftDown ? numOfShiftedChars : numOfUnshiftedChars)

Sorry I didn't spot that before.

+            uniChars[0] = mIsShiftDown ? shiftedLatinChar : unshiftedLatinChar;

uniChars[0] = ch;

Comment 38

[Tony Mechelynck [:tonymec]]

(In reply to comment #35)
> (In reply to comment #34)
[...]
> > I'm looking for an explanation of how towupper depends on the current locale,
> > so that we know that it will always do the right thing here.
> 
> Oops, sorry for I forgot this one.
> 

In Turkish the uppercase counterpart of i (U+0069) is a dotted I (İ, U+0130) while the lowercase counterpart of I (U+0049) is an undotted i (ı, U+0131). There may be other examples but this one is the most famous of locale-dependent case pairs.

Comment 39

[Karl Tomlinson (:karlt)]

(In reply to comment #38)
Thanks, Tony.  That is something to watch out for.

Comment 40

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

Attachment: Windows part v3.2

* Using nsICaseConversion in StringCaseInsensitiveEquals (Thank you, roc!)
* In DOMKeyCode is NS_VK_[A-Z] case, using NS_IsAscii and NS_ToUpper.

Comment 41

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

Attachment: Windows part v3.3

this should be final.

Comment 42

[Karl Tomlinson (:karlt)]

Comment on attachment 318329 [details] [diff] [review]
Windows part v3.3

Great, thanks.

Comment 43

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

The actual problem of this bug:
http://docs.google.com/support/bin/answer.py?answer=66280&topic=8634

Comment 44

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

We have an problem on Mac.

When Ctrl key is pressed, some keys are having ASCII char at some keys. But if we prefer the chars, the users cannot input some chars which are override by the key. So, I suggest that we always ignore the ASCII character which can be inputted with Ctrl key. And then, the charCode is set from Cmd key pressing. So, I'm thinking as:

  if (isCtrl) {
    if (charCode < 0x80) {
      if (!isShift)
        charCode = cmdChar;
      else if (LowerCase(unshiftedChar) == cmdChar)
        charCode = isASCII(shiftedChar) ? shiftedChar : cmdChar;
      else
        charCode = cmdChar;
    }
  } else if (!isAlt && isShift) { // Only Meta key is pressed
    if (LowerCase(unshiftedChar) == cmdChar)
      charCode = unshiftedChar;
    else if (LowerCase(shiftedChar) == cmdChar)
      charCode = shiftedChar;
  }

If we cannot resolve latin char with Shift key, charCode should be same as cmdChar. (I.e., Cmd+foo and Cmd+Shift+foo have same char in such case.)

We don't need to care Cmd+Alt(+Shift) and only Cmd key being pressed cases.

Comment 45

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 318329 [details] [diff] [review]
Windows part v3.3

I'm really, really uncomfortable approving this without tests, as the code has been proven to be quite delicate. What are the chances of generating some testcases around this?

Comment 46

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

(In reply to comment #45)
> (From update of attachment 318329 [details] [diff] [review])
> I'm really, really uncomfortable approving this without tests, as the code has
> been proven to be quite delicate. What are the chances of generating some
> testcases around this?

For creating the tests, we need to generate native events with many keyboard layout. Such system are not there :-(

Comment 47

[Jeff Walden [:Waldo]]

Get a bunch of litmus tests, then, for the meantime.  It's not perfect, but it's better than nothing.

Comment 48

[Damon Sicore (:damons)]

Comment on attachment 318329 [details] [diff] [review]
Windows part v3.3

a1.9+=damons

Comment 49

[Karl Tomlinson (:karlt)]

(In reply to comment #44)
> When Ctrl key is pressed, some keys are having ASCII char at some keys. But if
> we prefer the chars, the users cannot input some chars which are override by
> the key. So, I suggest that we always ignore the ASCII character which can be
> inputted with Ctrl key. And then, the charCode is set from Cmd key pressing.
> So, I'm thinking as:
> 
>   if (isCtrl) {
>     if (charCode < 0x80) {
>       if (!isShift)
>         charCode = cmdChar;
>       else if (LowerCase(unshiftedChar) == cmdChar)
>         charCode = isASCII(shiftedChar) ? shiftedChar : cmdChar;
>       else
>         charCode = cmdChar;
>     }

I don't completely understand the issue.
Can you give an example of "ASCII char at some keys" please?
Is the Ctrl key functioning similarly to the Option key?
Have you thought about what's best for Cmd+Ctrl?

>   } else if (!isAlt && isShift) { // Only Meta key is pressed
>     if (LowerCase(unshiftedChar) == cmdChar)
>       charCode = unshiftedChar;
>     else if (LowerCase(shiftedChar) == cmdChar)
>       charCode = shiftedChar;
>   }

Doesn't this mean that Cmd+Shift++ -> charCode = "=" on US keyboard?
Don't we want something like this:

  } else if (!isAlt && isShift) { // Only Meta and Shift keys are pressed
    if (LowerCase(unshiftedChar) == shiftedCmdChar)
      charCode = shiftedChar;
  }

> If we cannot resolve latin char with Shift key, charCode should be same as
> cmdChar. (I.e., Cmd+foo and Cmd+Shift+foo have same char in such case.)

I've seem some non-Latin layouts where different Latin chars seem to be
produced (Cmd+foo != Cmd+Shift-foo), so I think it's worth using
shiftedCmdChar (or just charCode) with Shift when shiftedChar is not Latin.

Comment 50

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

Windows part landed, thank you.

(In reply to comment #49)
> (In reply to comment #44)
> > When Ctrl key is pressed, some keys are having ASCII char at some keys. But if
> > we prefer the chars, the users cannot input some chars which are override by
> > the key. So, I suggest that we always ignore the ASCII character which can be
> > inputted with Ctrl key. And then, the charCode is set from Cmd key pressing.
> > So, I'm thinking as:
> > 
> >   if (isCtrl) {
> >     if (charCode < 0x80) {
> >       if (!isShift)
> >         charCode = cmdChar;
> >       else if (LowerCase(unshiftedChar) == cmdChar)
> >         charCode = isASCII(shiftedChar) ? shiftedChar : cmdChar;
> >       else
> >         charCode = cmdChar;
> >     }
> 
> I don't completely understand the issue.
> Can you give an example of "ASCII char at some keys" please?
> Is the Ctrl key functioning similarly to the Option key?
> Have you thought about what's best for Cmd+Ctrl?

E.g., on Spanish layout, ';' key is:
Shift:    ':'
Ctrl:     '"'
Cmd:      ':'
Cmd+Ctrl: '"'

If we trust the controlled char, Ctrl+';' should be '"'. However, then, Spanish layout cannot input ';' with Ctrl. However, if we ignore the Ctrl key state, that can input '"' by Cmd+Alt+Ctrl+'2'.

On mac the mod keys priority is: 1. Ctrl, 2. Alt, 3. Cmd. (Shift key affects only when Ctrl is not pressed.)

I think Ctrl+foo combination strictly behavior is not wanted by most people. And we and the web apps use Ctrl key is a part of Command shortcut. So, Ctrl should not be consumed for generating the charCode.

> >   } else if (!isAlt && isShift) { // Only Meta key is pressed
> >     if (LowerCase(unshiftedChar) == cmdChar)
> >       charCode = unshiftedChar;
> >     else if (LowerCase(shiftedChar) == cmdChar)
> >       charCode = shiftedChar;
> >   }
> 
> Doesn't this mean that Cmd+Shift++ -> charCode = "=" on US keyboard?
> Don't we want something like this:
> 
>   } else if (!isAlt && isShift) { // Only Meta and Shift keys are pressed
>     if (LowerCase(unshiftedChar) == shiftedCmdChar)
>       charCode = shiftedChar;
>   }

Ugh, right. I think the same behavior. It's mistake.

> > If we cannot resolve latin char with Shift key, charCode should be same as
> > cmdChar. (I.e., Cmd+foo and Cmd+Shift+foo have same char in such case.)
> 
> I've seem some non-Latin layouts where different Latin chars seem to be
> produced (Cmd+foo != Cmd+Shift-foo), so I think it's worth using
> shiftedCmdChar (or just charCode) with Shift when shiftedChar is not Latin.

Oh, right? Which keyboard layout? I don't find such layout now. And I agree.

Comment 51

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

Attachment: Patch v1.0

now, I'm testing this patch.

Comment 52

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

Attachment: Cocoa part v1.1

This patch also fixes bug 429217. However, we cannot support Dvorak-QWERTY. Because '{' and '}' are not sent to us at Cmd pressing. We need more work for them after Gecko1.9.

This patch decides what are returned by Cmd(+Opt)+'foo' and Cmd(+Opt)+'foo', first.

unshiftedChar -> 'foo'
shiftedChar -> Shift+'foo'
unshiftedCmdChar -> Cmd(+Opt)+'foo'
shiftedCmdChar -> Cmd(+Opt)+Shift+'foo'
unshiftedAltChar -> Opt+'foo'
shiftedAltChar -> Opt+Shift+'foo'
unCapsLockedChar -> 'foo' (ignoring CapsLock state)
unCapsLockedAltChar -> Opt+'foo' (ignoring CapsLock state)

isCmdSwitchLayout means whether the Cmd key switches the key meanings from unCapsLockedChar or unCapsLockedAltChar. I.e., Dvorak-QWERTY and some intl keyboard layouts (Russian, Hebrew, Arabic and Greek).

isDevorakQWERTY means whether the Cmd key switches the key meanings and the unCapsLocked state is also Latin keys. I.e., Devorak-QWERTY like keyboard layout.

If isDevorakQWERTY, we cannot support all ASCII chars at Cmd or Ctrl key being pressed. E.g., ':'. ':' should be Cmd+Shift+';'. However, Cmd+Shift+';' returns ';'. So, we cannot resolve ':' from ';' :-( However, if the char is lower case ASCII alphabet, we can care the case. If CapsLocked, the all alphabets should be upper case even if shift key is pressed. This may be Mac's default behavior. And if not CapsLocked, we should change shiftedCmdChar to upper case.

At non-Devorak-QWERTY layout, and if Opt is pressed, and unCapsLockedAltChar and unshiftedCmdChar (that was gotten with Opt key state) are same value, we are using unshiftedAltChar and shiftedAltChar to unshiftedCmdChar and shiftedCmdChar. If unCapsLockedAltChar and unshiftedAltChar are not same, we cannot resolve Cmd+Opt+Shift, so, we don't modify the values, we should use the values from (UC)KeyTranslate.

If Opt key is not pressed and not isCmdSwitchLayout, unshiftedCmdChar and shiftedCmdChar should be unshiftedChar and shiftedChar.

If Opt key is not pressed and isCmdSwitchLayout, we cannot resolve shiftedCmdChar like Devorak-QWERTY case. However, we can care the alphabet case too.

By this process, shiftedCmdChar has shifted(Alt)Char value if we can resolve it. Then, we can modify the charCode from shiftedCmdChar and unshiftedCmdChar.

If Ctrl is pressed and charCode is in ASCII range, the Ctrl is not used for character inputting, so, that is just a modifier key for a command. Therefore, we can overwrite charCode by (un)shiftedCmdChar. Otherwise, we should not modify charCode.

If Cmd is pressed and Ctrl is not pressed, we can replace charCode by (un)shiftedCmdChar always.

If Opt is only pressed, we should not modify charCode like Win32.

Comment 53

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

Comment on attachment 318756 [details] [diff] [review]
Cocoa part v1.1

oh, this patch is not enough for some cases.

Comment 54

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

Attachment: GTK2 part v3.0

Comment 55

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

On MacOS 10.4, UCKeyTranslate is broken on some keyboard layout. E.g., Persian, Arabic, etc... It might be that we should not use UCKeyTranslate/KeyTranslate...

Comment 56

[Karl Tomlinson (:karlt)]

(In reply to comment #50)
> > I've seem some non-Latin layouts where different Latin chars seem to be
> > produced (Cmd+foo != Cmd+Shift-foo), so I think it's worth using
> > shiftedCmdChar (or just charCode) with Shift when shiftedChar is not Latin.
> 
> Oh, right? Which keyboard layout? I don't find such layout now. And I agree.

"French - numerical" on 10.4 is one such layout.

Comment 57

[Karl Tomlinson (:karlt)]

Comment on attachment 318799 [details] [diff] [review]
GTK2 part v3.0

+                    if (ch && !event.isAlt) {
+                      event.charCode = ch;

I think we should only do this when event.isCtrl.
!(event.isAlt || event.isMeta) might be best.

And we should only do this when

  event.charCode == event.isShift ? shiftedCharCode : unshiftedCharCode"

as on Windows (but Linux doesn't need a case-insensitive test) or we may be
removing the real charCode from the list of alternatives.  You may wish to
record "event.isShift ? shiftedCharCode : unshiftedCharCode" as
unmodifiedCharCode or similar.

Comment 58

[Robert O'Callahan (:roc) (email my personal email if necessary)]

My sendNativeKeyEvent patch would let us test this on Mac and Windows, hopefully GTK soon. I think we shouldn't take any more patches in this bug until that patch has landed and we have tests for the issues in this bug.

Comment 59

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

(In reply to comment #57)
> And we should only do this when
> 
>   event.charCode == event.isShift ? shiftedCharCode : unshiftedCharCode"

Why? ch may be null. Then, we don't need to replace the charCode. On Win32, I also checking whether ch is not null.

> as on Windows (but Linux doesn't need a case-insensitive test) or we may be
> removing the real charCode from the list of alternatives.  You may wish to
> record "event.isShift ? shiftedCharCode : unshiftedCharCode" as
> unmodifiedCharCode or similar.

Do you think it is needed at Gecko1.9?

Comment 60

[Karl Tomlinson (:karlt)]

(In reply to comment #59)
> (In reply to comment #57)
> > And we should only do this when
> > 
> >   event.charCode == event.isShift ? shiftedCharCode : unshiftedCharCode"
> 
> Why? ch may be null. Then, we don't need to replace the charCode. On Win32, I
> also checking whether ch is not null.

Yes.  Check ch.

if (ch && !(event.isAlt || event.isMeta)
    && event.charCode == (event.isShift ? shiftedCharCode : unshiftedCharCode))

> > as on Windows (but Linux doesn't need a case-insensitive test) or we may be
> > removing the real charCode from the list of alternatives.  You may wish to
> > record "event.isShift ? shiftedCharCode : unshiftedCharCode" as
> > unmodifiedCharCode or similar.
> 
> Do you think it is needed at Gecko1.9?

Not sure what you mean here.  We should do the test above (which is similar to Windows).  Currently we don't have separate shiftedCharCode / unshiftedCharCode variables but the information is evaluated.  unmodifiedCharCode was just a suggestion for recording that before overwriting altCharCodes when !isLatin.

Comment 61

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

Attachment: GTK2 part v4.0

Comment 62

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

(In reply to comment #60)
> (In reply to comment #59)
> > (In reply to comment #57)
> > > as on Windows (but Linux doesn't need a case-insensitive test) or we may be
> > > removing the real charCode from the list of alternatives.  You may wish to
> > > record "event.isShift ? shiftedCharCode : unshiftedCharCode" as
> > > unmodifiedCharCode or similar.
> > 
> > Do you think it is needed at Gecko1.9?
> 
> Not sure what you mean here.  We should do the test above (which is similar to
> Windows).  Currently we don't have separate shiftedCharCode / unshiftedCharCode
> variables but the information is evaluated.  unmodifiedCharCode was just a
> suggestion for recording that before overwriting altCharCodes when !isLatin.

Oh, I understand what you said.

Comment 63

[Karl Tomlinson (:karlt)]

Comment on attachment 318972 [details] [diff] [review]
GTK2 part v4.0

We shouldn't wait for the GTK test infrastructure.  (Bug 431503 comment 13)

Comment 64

[Robert O'Callahan (:roc) (email my personal email if necessary)]

For the Cocoa patch, please break it up into as small pieces as possible and put each piece in its own separate new bug. This bug is already overloaded.

Comment 65

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

(In reply to comment #64)
> For the Cocoa patch, please break it up into as small pieces as possible and
> put each piece in its own separate new bug. This bug is already overloaded.

mmm... I need following works on Cocoa:

1 KeyTranslate result should be converted to Unicode.
2 KeyTranslate result may have 2 bytes.
3 Combine KeyTranslate process and UCKeyTranslate process in a static method. (This reduces many lines from final, and we need to write the code just once the logical part. See 5.)
4 Need to check CapsLock state and NumLock state.
5 charCode should be replaced with some modifier keys.
  5.1 Both unshiftedCmdChar and shiftedCmdChar are needed for this bug. If they are not same, we need to replace shiftedCmdChar ourselves if it's possible. And unshiftedCmdChar should prefer CapsLock state.
  5.2 Both unshiftedAltCmdChar and shiftedAltCmdChar are needed when Option is pressed. unshiftedAltCmdChar should prefer CapsLock state too.

Then, I can separate this bug  to three bugs:
1 - 3, 4, 5. 5 is this bug.

And on some keyboard layouts don't work fine (UC)KeyTranslate APIs issue, I give up it on Gecko1.9. I could not find good approach for such bugs of OS :-(

Comment 66

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 318972 [details] [diff] [review]
GTK2 part v4.0

a1.9=beltzner

Comment 67

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

checked-in the gtk2 part.

Comment 68

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

I filed bug 432388 and bug 432389. I'll fix them before this.

Comment 69

[Mike Beltzner [:beltzner, not reading bugmail]]

Decided not to block on this at the triage during the Firefox meeting.

The reason was that it was hard to tell what the user impact is, and the lack of clear testcases makes it hard to determine what success is. Please renominate with a statement of how this bug affects users and web applications if you feel passionate.

Also: is this the sort of thing we can fix on branch?

Comment 70

[Robert O'Callahan (:roc) (email my personal email if necessary)]

We probably would not fix this on branch IMHO.

Comment 71

[chris hofmann]

looking at reporter data for comments that have "keyboard" I stumbled on to
this...   might make the start of a test case reflecting real world use...

Its a report from beta3 user so this particular problem has been there awhile

URL:
http://mail.google.com/mail/#inbox
Host:
Reports for mail.google.com
Problem Type:
Other problem
Behind Login:
No
Product:
Firefox/3.0b3
Gecko Version:
20080205
Platform:
MacIntel
OS/CPU:
Intel Mac OS X 10.4
Language:
en-US
Character Set:
UTF-8 
User Agent:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3)
Gecko/2008020511 Firefox/3.0b3
Build Config:
--target=powerpc-apple-darwin8.8.4
--with-macos-sdk=/Developer/SDKs/MacOSX10.4u.sdk --enable-application=browser
--enable-update-channel=beta --enable-optimize --disable-debug --disable-tests
--enable-update-packaging --enable-official-branding --enable-p
Date Reported:
2008-02-13 14:31:40
Description:
keyboard shortcuts do not work consistently. go to inbox, navigate with j or k,
hit o to open, then hit y to archive. you return to the inbox. now the keyboard
shortcuts no longer work. 

Comment 72

[chris hofmann]

here is another reporter comment on beta 5


URL:
https://mail.google.com/mail/
Host:
Reports for mail.google.com
Product:
Firefox/3.0b5
Gecko Version:
20080326
Platform:
MacIntel
OS/CPU:
Intel Mac OS X 10.5
Language:
fi
Character Set:
UTF-8 
User Agent:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; fi; rv:1.9b5) Gecko/2008032619 Firefox/3.0b5
Build Config:
--target=powerpc-apple-darwin8.8.4 --with-macos-sdk=/Developer/SDKs/MacOSX10.4u.sdk --enable-application=browser --enable-update-channel=beta --enable-optimize --disable-debug --disable-tests --enable-update-packaging --enable-official-branding --enable-p
Date Reported:
2008-04-07 12:49:58
Description:
I use a Finnish keyboard layout, which requires pressing the alt/option or 'AltGR' key for certain punctuation symbols. It is impossible in the most recent beta to enter characters such as @ (alt/option+2), $ (alt/option+4) in password form fields, and certain unicode keyboard layouts are also displayed as greyed out in the keyboard layout select. As it stands now, I must switch keyboard layouts to English in order to type a password including such characters. This only happens in Firefox 3 Beta 5, and if the info isn't provided with this notice, I'm using OS X 10.5.2. It is my opinion that this functionality needs to be returned, as otherwise it will alienate international/non-english-speaking users who require full use of their keyboard in form fields. 

Comment 73

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #69)
> The reason was that it was hard to tell what the user impact is, and the lack
> of clear testcases makes it hard to determine what success is. Please
> renominate with a statement of how this bug affects users and web applications
> if you feel passionate.

As I understand it bug 429217 is one of the bugs that would be fixed by a patch here. If we end up not taking this, we should definitely take the workaround there.

Is this bug really fixed for Windows and GTK2, but not Mac? It would be bad to ship with that kind of inconsistency. 

Comment 74

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

(In reply to comment #73)
> (In reply to comment #69)
> > The reason was that it was hard to tell what the user impact is, and the lack
> > of clear testcases makes it hard to determine what success is. Please
> > renominate with a statement of how this bug affects users and web applications
> > if you feel passionate.
> 
> As I understand it bug 429217 is one of the bugs that would be fixed by a patch
> here. If we end up not taking this, we should definitely take the workaround
> there.

And also Cmd+? needs workaround for some keyboard layouts if we don't fix this bug.

Comment 75

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

(In reply to comment #69)
> The reason was that it was hard to tell what the user impact is, and the lack
> of clear testcases makes it hard to determine what success is. Please
> renominate with a statement of how this bug affects users and web applications
> if you feel passionate.

The impact may be smaller on Mac than other platforms. Because Meta key sends the unshifted Latin characters to us. And at Ctrl key pressing, we have an adhoc process *only* for ASCII alphabets. Therefore, we don't have problem for unshifted Meta key handling and Ctrl+alphabet handling.

However, Ctrl+Num is not working on some keyboard layouts. E.g., on French keyboard layout, Ctrl+6, Ctrl+9 and Ctrl+0 doesn't work fine. Because Ctrl+6,9,0 don't send 6,9,0 to us.

And also non-alphabet/non-numeric characters on many keyboard layout cannot be used with Ctrl key. And also shifted Latin key cannot be used with Meta key on most keyboard layouts. (like bug 429217)

That means there is a serious accessibility issue. We can only use Meta+unshifted chars, it means we should not use non-alphabet characters for any commands on Mac. E.g., if we apply the patch of bug 429217, that only works fine with some keyboard layouts that Shift+'[' is '{'. So, if the user uses the keyboard layout that Shift+'[' is not '{', the shortcut is not Meta+'{'. And if another user uses the keyboard layout that has '{' in unshifted position, the user cannot use Meta+'{' key.

If web app developers only tests on Win or Linux of Gecko, Mac users cannot use some shortcut keys of the web app. It's too bad for Mac users.

So, the problem cases are not many. However, the each problems may be serious.

> Also: is this the sort of thing we can fix on branch?

I think that we *can* do on branch. However, it's too bad thing for web develpers. Because some web app developers may change their apps for Fx3.0 on Mac, however, we will re-break the behavior.

Comment 76

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #73)
> (In reply to comment #69)
> Is this bug really fixed for Windows and GTK2, but not Mac? It would be bad to
> ship with that kind of inconsistency.

Masayuki confirms that this is the case. Given that, and the tab switching and help shortcuts this would break in chrome (not to mention shortcuts potentially used by webapps), I think we should block on this.

Comment 77

[Mike Beltzner [:beltzner, not reading bugmail]]

Do we have any ETA on the Cocoa part? Can Josh help?

Comment 78

[Mike Schroepfer]

Given comment 72 we probably need to fix this for final.  Josh/Masayuki/Karlt do we have a plan to get the minimal patch needed?

Comment 79

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

(In reply to comment #78)
> Given comment 72 we probably need to fix this for final.  Josh/Masayuki/Karlt
> do we have a plan to get the minimal patch needed?

I think the comment 72 issue was already fixed (it's another bug). This bug is regression from bug 359638. So, b5 must not have this bug.

And the most part of cocoa part will be landed by bug 432388. Then, bug 432389 needs really small patch. And this bug needs a low risky patch. So, after bug 432388, we don't need long time for the two bugs, I think.

Comment 80

[Karl Tomlinson (:karlt)]

(In reply to comment #79)
> I think the comment 72 issue was already fixed (it's another bug).

Yes the changes for bug 359638 look like the would have fixed that issue,
and John successfully tested a very similar situation.

Comment 81

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Let's spin off another bug for the remaining work here. That new bug should have clear "steps to reproduce" so we can see exactly what still needs to be fixed. Then we should close this bug since it's too long and complicated.

IMHO the new bug should not block Firefox 3.

Comment 82

[Mike Schroepfer]

Given comment 81 I'm going to take this bug off the blocker list

Comment 83

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

o.k. I filed bug 432951 and bug 432953. I recommend that bug 432951 should be fixed at Gecko1.9. but bug 432953 is really minor issue.

-> FIXED (comment 81)

Comment 84

[Eyal Rozenberg]

Ctrl+Shift+X in textboxes still broken if you're in non-latin keyboard layout (trunk nightly 2008-05-07, Windows XP). Is that not part of this bug's scope?

Comment 85

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

(In reply to comment #84)
> Ctrl+Shift+X in textboxes still broken if you're in non-latin keyboard layout
> (trunk nightly 2008-05-07, Windows XP). Is that not part of this bug's scope?

What's that? I cannot reproduce such bug. Please file a new bug. And you should report your keyboard layout in a new bug. And please test with clean profile too.