Last Comment Bug 433192 - Ctrl+Shift+X doesn't work in text boxes with Arabic/Hebrew/Persian keyboard to switch between RTL/LTR layouts
: Ctrl+Shift+X doesn't work in text boxes with Arabic/Hebrew/Persian keyboard t...
Status: RESOLVED FIXED
[key hell]
: intl, regression, rtl
Product: Core
Classification: Components
Component: Widget (show other bugs)
: Trunk
: x86 Windows XP
: -- normal (vote)
: mozilla1.9
Assigned To: Masayuki Nakano [:masayuki] (Mozilla Japan)
:
Mentors:
Depends on:
Blocks: fx3-l10n-he 429510
  Show dependency treegraph
 
Reported: 2008-05-10 13:49 PDT by Eyal Rozenberg
Modified: 2008-05-15 14:13 PDT (History)
15 users (show)
mtschrep: blocking1.9+
jwalden+bmo: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1.0 (2.51 KB, patch)
2008-05-10 22:09 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
Define handlers for cmd_switchTextDirection on editable elements (4.20 KB, patch)
2008-05-11 08:25 PDT, Karl Tomlinson (ni?:karlt)
neil: review+
Details | Diff | Review
Patch v2.0 (3.90 KB, patch)
2008-05-11 08:54 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
Patch v2.1 (2.85 KB, patch)
2008-05-11 08:56 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
masayuki: review-
Details | Diff | Review
Patch v2.2 (2.83 KB, patch)
2008-05-11 09:30 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
Patch v2.2.1 (v2.2 + tests) (5.91 KB, patch)
2008-05-11 10:15 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
Patch v2.2.2 (v2.2 + tests) (6.69 KB, patch)
2008-05-11 11:03 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
masayuki: review-
Details | Diff | Review
Patch v2.3 (6.72 KB, patch)
2008-05-11 11:50 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
karlt: review+
roc: superreview+
Details | Diff | Review
fix without HasSameCharCaseInsensitive and with further simplification (2.36 KB, patch)
2008-05-11 15:27 PDT, Karl Tomlinson (ni?:karlt)
no flags Details | Diff | Review
tests [checked in] (3.86 KB, patch)
2008-05-11 15:50 PDT, Karl Tomlinson (ni?:karlt)
no flags Details | Diff | Review
patch v3 without HasSameCharCaseInsensitive [checked in] (2.63 KB, patch)
2008-05-11 17:09 PDT, Karl Tomlinson (ni?:karlt)
gavin.sharp: review+
roc: superreview+
mtschrep: approval1.9+
Details | Diff | Review
additional tests (6.08 KB, patch)
2008-05-12 08:27 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
roc: review+
roc: superreview+
Details | Diff | Review

Description Eyal Rozenberg 2008-05-10 13:49:43 PDT
I use the US English and Hebrew keyboard layouts on Windows. Ctrl+Shift+X, which switches the direction of a textbox between RTL and LTR, now only works in US English, not in Hebrew. Tried a new profile, same thing. To reproduce, just use one of the textboxes in a bugzilla bug page...
Comment 1 Karl Tomlinson (ni?:karlt) 2008-05-10 17:47:27 PDT
Any idea when this last worked?

US or Hebrew localized build?
Comment 2 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-10 22:09:17 PDT
Created attachment 320409 [details] [diff] [review]
Patch v1.0

Interesting.

We have two event handlers:
1. nsXBLEventHandler
2. nsXBLWindowKeyHandler

And the only 2 has Ctrl+Shift+X. But 1 has Ctrl+X for "Cut". On the Hebrew keyboard layout's Latin alphabets are not paired on a key. Therefore, we append shift ignorable shortcut candidate in nsContentUtils::GetAccelKeyCandidates.

So, we need to check whether the alternative character is case changeable character and it is same as original charCode.
Comment 3 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-10 22:10:29 PDT
We must fix this bug for Hebrew users.
Comment 4 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-10 22:13:16 PDT
(In reply to comment #2)
> We have two event handlers:
> 1. nsXBLEventHandler
> 2. nsXBLWindowKeyHandler
> 
> And the only 2 has Ctrl+Shift+X. But 1 has Ctrl+X for "Cut". On the Hebrew
> keyboard layout's Latin alphabets are not paired on a key. Therefore, we append
> shift ignorable shortcut candidate in nsContentUtils::GetAccelKeyCandidates.

I meant that the event is consumed as Ctrl+X in nsXBLEventHandler. Therefore, text direction switcher is never called from nsXBLWindowKeyHandler.
Comment 5 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-05-10 22:47:08 PDT
Maybe we should do the first "exact match" pass in both event handlers and then do the second pass in both event handlers?
Comment 6 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-10 22:50:18 PDT
(In reply to comment #5)
> Maybe we should do the first "exact match" pass in both event handlers and then
> do the second pass in both event handlers?

It is true. However, such conflict case should not be there.
Comment 7 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-10 22:54:56 PDT
And also such strictly fix is pretty risky, now. We should do it after 1.9.
Comment 8 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-10 23:35:25 PDT
If we need to fix this issue ideally, we need to change |nsEventTargetChainItem::HandleEventTargetChain| at keypress event. This loop should run *each* shortcut candidates. But that means keypress event is fired many times. So, I don't have idea for the ideally fix.
Comment 9 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-11 00:48:48 PDT
The cut command may be defined here:
http://mxr.mozilla.org/mozilla/source/content/xbl/builtin/browser-base.inc

and this is included by:
http://mxr.mozilla.org/mozilla/source/content/xbl/builtin/win/platformHTMLBindings.xml

and this binds to input and textarea:
http://mxr.mozilla.org/mozilla/source/layout/style/forms.css#101

I think that the non-exact match candidate should be also consumed by the inner element. So, I think that we don't need to change the current our strategy. The key conflict might be occurred, but that is too rare case. And then, the inner element's binding should beat the outer element's handlers.
Comment 10 Karl Tomlinson (ni?:karlt) 2008-05-11 07:51:05 PDT
I don't know enough about bidi editing to say whether or not this should be considered for blocking, so I'm asking in bug 415606 for input.

This command is available in the edit menu when the system locale is Hebrew
and also when the bidi.browser.ui pref is true.

When the command is available in the menu, "Alt+ע כ" and "<Menu> כ" shortcuts
are available for Hebrew localizations but unfortunately these only activate
the command when the keyboard is in Hebrew layout, so the shortcut would be
different depending on whether Hebrew or US layout is currently selected.
("Alt+e w" or "<Menu> w" activates the command from any layout for en builds.)
Comment 11 Tomer Cohen :tomer 2008-05-11 08:10:16 PDT
Please note that this bug is more 
Comment 12 Karl Tomlinson (ni?:karlt) 2008-05-11 08:18:19 PDT
Comment on attachment 320409 [details] [diff] [review]
Patch v1.0

         if (ch == unshiftCh ||
             (IS_IN_BMP(ch) && IS_IN_BMP(unshiftCh) &&
              ToLowerCase(PRUnichar(ch)) == ToLowerCase(PRUnichar(unshiftCh))))
           continue;
 
+        if (lowerCharCode != upperCharCode &&
+            (lowerCharCode == ch || upperCharCode == ch))
+          continue;

Is there a reason why you only compare with charCode, rather than checking
whether each alternative character is a bicameral letter?

If a test for case variability were performed on every character, then the
logic in the previous conditional could also be simplified.

We could fix this in nsContentUtils::GetAccelKeyCandidates (as you have
suggested) but I have another suggestion.
Comment 13 Karl Tomlinson (ni?:karlt) 2008-05-11 08:20:43 PDT
This is my understanding of the problem:

There is a conflict between the bindings for cmd_switchTextDirection
(Ctrl+Shift+X) and cmd_cut (Ctrl+X) in a keyboard layout (Hebrew) where X is
only available with Shift.  (We advertise Ctrl+X as the shortcut for Cut in
the menu, and the only way to type an X while keeping the keyboard in a Hebrew
layout is to hold down shift.  It would therefore not be surprising if some
users tried to Cut by using Ctrl+Shift+X.  However, this particular shortcut
is well enough known that most [almost all] users would not hold down shift
when trying to Cut.)

This conflict would be resolved in favour of cmd_switchTextDirection (as the
handler is defined with Shift in "modifiers") if the handlers for these
commands were defined on the same element.

IMO a big part of the problem is that the cmd_switchTextDirection handler is
only defined on the window.  cmd_cut is defined on editable elements, and
takes priority due to being on the innermost element.

It seems that a cmd_switchTextDirection handler should be defined on editable
elements (as that is where the shortcut has effect) and doing so would resolve
this issue.

(This bug does not exist on Linux because on that platform XBL handlers are
not defined for cut on editable elements, but instead nsINativeKeyBindings is
used.  This bug does not exist on Mac because the Hebrew layout on that
platform is different.)
Comment 14 Karl Tomlinson (ni?:karlt) 2008-05-11 08:25:13 PDT
Created attachment 320433 [details] [diff] [review]
Define handlers for cmd_switchTextDirection on editable elements

This defines handlers for cmd_switchTextDirection on editable elements in the
same way as cmd_cut handlers are defined on such elements.  I haven't thoroughly tested, sorry, but builds should be available here in about an hour:

https://build.mozilla.org/tryserver-builds/2008-05-11_07:37-ktomlinson@mozilla.com-switch-text-direction/

I'm assuming that &bidiSwitchTextDirectionItem.commandkey; can't be used in
platformHTMLBindings.xml because no other bindings there are localized.  If
this binding is ever localized then the localized binding would still exist on
the browser window.

editorOverlay defines a (empty) key_cut key but I don't know enough about this
to know whether it should also define a key_switchTextDirection:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/editor/ui/composer/content/editorOverlay.xul&rev=1.257&mark=71#57
Comment 15 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-11 08:54:05 PDT
Created attachment 320435 [details] [diff] [review]
Patch v2.0

Karl:

Even if we use your approach, this bug is really a bug. Because we should not ignore the shift state at that time. Ctrl+Shift+X should not execute cmd_cut on Hebrew layout on Windows.
Comment 16 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-11 08:56:25 PDT
Created attachment 320436 [details] [diff] [review]
Patch v2.1

oops, sorry for the spam.
Comment 17 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-11 08:59:16 PDT
Comment on attachment 320436 [details] [diff] [review]
Patch v2.1

er, this may have a bug. sorry.
Comment 18 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-11 09:30:17 PDT
Created attachment 320438 [details] [diff] [review]
Patch v2.2
Comment 19 Mike Schroepfer 2008-05-11 09:37:47 PDT
Is there any kind of user workaround (e.g. content menu, etc).  How often is RTL-LTR switching done for edit boxes?
Comment 20 Mike Schroepfer 2008-05-11 09:40:59 PDT
Also is this something we can add to the test suite?
Comment 21 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-11 09:42:14 PDT
(In reply to comment #20)
> Also is this something we can add to the test suite?

now, I'm checking the test, please wait a moment.
Comment 22 Mike Beltzner [:beltzner, not reading bugmail] 2008-05-11 09:46:34 PDT
(In reply to comment #1)
> Any idea when this last worked?
> US or Hebrew localized build?

Has anyone answered these questions? Drivers will need to make a decision based on impact, and I haven't seen a clear statement about if this is:

 - only Hebrew keyboard layouts on non-localized builds,
 - affecting more keyboard layouts on non-localized builds,
 - something that can be worked around,
 - something that can be fixed on branch.
Comment 23 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-11 10:04:10 PDT
(In reply to comment #22)
>  - only Hebrew keyboard layouts on non-localized builds,

This is only on Hebrew kayboard layout, but I'm not sure whether the Hebrew localized build localizes the key.

>  - affecting more keyboard layouts on non-localized builds,

My patch affects to all keyboard layout. If that has a bug.

>  - something that can be worked around,

I think that this is an important usability issue for the keyboard shortcut users. Even if there is another way for accessing to cmd_switchTextDirection, it is uncomfortable for the shortcut key users.

>  - something that can be fixed on branch.

Yes, we can.

(In reply to comment #19)
> How often is RTL-LTR switching done for edit boxes?

Smontagu should know the answer.
Comment 24 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-11 10:15:28 PDT
Created attachment 320441 [details] [diff] [review]
Patch v2.2.1 (v2.2 + tests)
Comment 25 Eyal Rozenberg 2008-05-11 10:24:22 PDT
> How often is RTL-LTR switching done for edit boxes?

Very. Let's say 30% as often as cutting and pasting.

> (In reply to comment #1)
> Any idea when this last worked?

Up until 429510 started manifesting.

> US or Hebrew localized build?

US build. 

(In reply to comment #22)

This is pretty major bustage for Hebrew, Arabic, Farsi layout users (who use the shortcut rather than the menus or context menus, which I suspect is the majority).
Comment 26 Mike Schroepfer 2008-05-11 10:39:33 PDT
(In reply to comment #24)
> Created an attachment (id=320441) [details]
> Patch v2.2.1 (v2.2 + tests)
> 

Masayuki/Karlt what is the regression risk of this patch?
Comment 27 :Ehsan Akhgari (out sick) 2008-05-11 10:47:31 PDT
(In reply to comment #25)
> This is pretty major bustage for Hebrew, Arabic, Farsi layout users (who use
> the shortcut rather than the menus or context menus, which I suspect is the
> majority).

Confirming this for Persian users.  Ctrl+Shift+X is a very useful shortcut which Persian users use, and in fact is quite important, because some of the most often used text boxes (such as those of web mail services) need to change from LTR to RTL when for example I want to type in a Persian email.  I think 1.9 should have this fixed for the three RTL locales (especially that users expect this to work even in non-ar/fa/he builds as well.)
Comment 28 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-11 11:03:11 PDT
Created attachment 320448 [details] [diff] [review]
Patch v2.2.2 (v2.2 + tests)

adding some tests.
Comment 29 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-11 11:09:37 PDT
(In reply to comment #26)
> (In reply to comment #24)
> > Created an attachment (id=320441) [details] [details]
> > Patch v2.2.1 (v2.2 + tests)
> > 
> 
> Masayuki/Karlt what is the regression risk of this patch?

I think the risk of the patch is low. Because that only appends one limitation for appending a shortcut candidate key combination that ignores the Shift key state.

If non-alphabet key is pressed and that cannot be inputted without shift key, we need to ignore the shift key. E.g., Ctrl+'+' is Ctrl+Shift+';' on Japanese keyboard layout. Then, by ignoring the Shift key state, we can execute the Ctrl+'+' command. If there is a regression, this is broken. But I added the test on the latest testcases.

I don't have the other ideas for the possibility of the regressions, now.
Comment 30 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-11 11:13:14 PDT
(In reply to comment #27)
> (In reply to comment #25)
> > This is pretty major bustage for Hebrew, Arabic, Farsi layout users (who use
> > the shortcut rather than the menus or context menus, which I suspect is the
> > majority).
> 
> Confirming this for Persian users.  Ctrl+Shift+X is a very useful shortcut
> which Persian users use, and in fact is quite important, because some of the
> most often used text boxes (such as those of web mail services) need to change
> from LTR to RTL when for example I want to type in a Persian email.  I think
> 1.9 should have this fixed for the three RTL locales (especially that users
> expect this to work even in non-ar/fa/he builds as well.)

Really? Can you reproduce this bug with Persian/Arabic layout?? This bug should be reproduced with Persian and Arabic keyboard layout...
Comment 31 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-11 11:14:25 PDT
(In reply to comment #30)
>  This bug should be reproduced with Persian and Arabic keyboard layout...

oops, I meant:

this bug should *not* be reproduced with Persian and Arabic keyboard layouts...
Comment 32 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-11 11:35:19 PDT
Comment on attachment 320448 [details] [diff] [review]
Patch v2.2.2 (v2.2 + tests)

I found a mistake.
Comment 33 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-11 11:50:50 PDT
Created attachment 320454 [details] [diff] [review]
Patch v2.3

fix a mistake.
Comment 34 Karl Tomlinson (ni?:karlt) 2008-05-11 14:54:53 PDT
Comment on attachment 320454 [details] [diff] [review]
Patch v2.3

For the sake of Hebrew/Farsi users, I think we should try
to jam this into rc1 if we can.  It sounds like the current
behavior is not expected and inconvenient, and Masayuki's tests
cover the area for which we would be most concerned about regressions. (Thank you!)

(I haven't reproduced on Arabic layout, but there are several
different Arabic layouts and I've only tried Saudi.)

With this patch, we don't need attachment 320433 [details] [diff] [review] for rc1 or 1.9.

Anyone able to sr soon?
Comment 35 Karl Tomlinson (ni?:karlt) 2008-05-11 15:05:20 PDT
Comment on attachment 320454 [details] [diff] [review]
Patch v2.3

+        if (IsCaseChangeableChar(ch) &&
+            HasSameCharCaseInsensitive(ch, aCandidates))

The HasSameCharCaseInsensitive looks unnecessary.  Isn't this always true
because we've already done

      nsShortcutCandidate key(nativeKeyEvent->charCode, PR_FALSE);

and when ch != nativeKeyEvent->charCode

          nsShortcutCandidate key(ch, PR_FALSE);
Comment 36 Mike Schroepfer 2008-05-11 15:11:23 PDT
(In reply to comment #34)
> (From update of attachment 320454 [details] [diff] [review])
> For the sake of Hebrew/Farsi users, I think we should try
> to jam this into rc1 if we can.  It sounds like the current
> behavior is not expected and inconvenient, and Masayuki's tests
> cover the area for which we would be most concerned about regressions. (Thank
> you!)
> 
> (I haven't reproduced on Arabic layout, but there are several
> different Arabic layouts and I've only tried Saudi.)
> 
> With this patch, we don't need attachment 320433 [details] [diff] [review] for rc1 or 1.9.
> 
> Anyone able to sr soon?
> 

You didn't target the SR - who did you want to SR?

Can we get an updated version of the patch?

Comment 37 Karl Tomlinson (ni?:karlt) 2008-05-11 15:27:45 PDT
Created attachment 320470 [details] [diff] [review]
fix without HasSameCharCaseInsensitive and with further simplification

Changes based on comment 35.

I think this is all we need but attachment 320454 [details] [diff] [review] is safe (just with some unnecessary code).  If we can't find anyone to review this then we could go with attachment 320454 [details] [diff] [review].  I'm assuming Masayuki is asleep.  Gavin do you feel up to reviewing this?  roc are you around?
Comment 38 neil@parkwaycc.co.uk 2008-05-11 15:39:37 PDT
Comment on attachment 320433 [details] [diff] [review]
Define handlers for cmd_switchTextDirection on editable elements

>Index: content/xbl/builtin/browser-base.inc
>===================================================================
>RCS file: /cvsroot/mozilla/content/xbl/builtin/browser-base.inc,v
>retrieving revision 1.3
>diff -u -p -U 8 -r1.3 browser-base.inc
>--- content/xbl/builtin/browser-base.inc	2 Sep 2005 15:54:18 -0000	1.3
>+++ content/xbl/builtin/browser-base.inc	11 May 2008 14:34:11 -0000
>@@ -24,10 +24,12 @@
> 
>       <handler event="keypress" key="x" command="cmd_cut" modifiers="accel"/>
>       <handler event="keypress" key="c" command="cmd_copy" modifiers="accel"/>
>       <handler event="keypress" key="v" command="cmd_paste" modifiers="accel"/>
>       <handler event="keypress" key="z" command="cmd_undo" modifiers="accel"/>
>       <handler event="keypress" key="z" command="cmd_redo" modifiers="accel,shift" />
>       <handler event="keypress" key="a" command="cmd_selectAll" modifiers="accel"/>
> 
>+      <handler event="keypress" key="x" modifiers="accel,shift" command="cmd_switchTextDirection"/>
>+
JFTR I don't think this one was necessary. r=me in the unlikely event that this patch is preferred by drivers even though it hardcodes the accel+shift+x key.
Comment 39 Karl Tomlinson (ni?:karlt) 2008-05-11 15:50:49 PDT
Created attachment 320473 [details] [diff] [review]
tests [checked in]

The tests (only) from attachment 320454 [details] [diff] [review].
Comment 40 Karl Tomlinson (ni?:karlt) 2008-05-11 17:09:32 PDT
Created attachment 320477 [details] [diff] [review]
patch v3 without HasSameCharCaseInsensitive [checked in]

Removing HasSameCharCaseInsensitive without the extra simplification because I couldn't convince people that

  "If there are _not_ distinct upper and lower case forms of ch, then
   ToLowerCase(ch) has no effect.  Otherwise ToLowerCase and ToUpperCase would
   need to both change ch and both change ch to the same character."
Comment 41 Karl Tomlinson (ni?:karlt) 2008-05-11 17:24:13 PDT
Comment on attachment 320477 [details] [diff] [review]
patch v3 without HasSameCharCaseInsensitive [checked in]

Request a1.9 conditional on gavin's review (which sounded likely on #developers) and landing with test cases in attachment 320473 [details] [diff] [review].
Comment 42 Mike Schroepfer 2008-05-11 17:29:11 PDT
Comment on attachment 320477 [details] [diff] [review]
patch v3 without HasSameCharCaseInsensitive [checked in]

a+ schrep please land immediately + a clobber.

If the builds go clean we'll kickoff RC1.
Comment 44 Tony Mechelynck [:tonymec] 2008-05-11 19:53:29 PDT
(In reply to comment #29)
[...]
> I think the risk of the patch is low. Because that only appends one limitation
> for appending a shortcut candidate key combination that ignores the Shift key
> state.
> 
> If non-alphabet key is pressed and that cannot be inputted without shift key,
> we need to ignore the shift key. E.g., Ctrl+'+' is Ctrl+Shift+';' on Japanese
> keyboard layout. Then, by ignoring the Shift key state, we can execute the
> Ctrl+'+' command. If there is a regression, this is broken. But I added the
> test on the latest testcases.
> 
> I don't have the other ideas for the possibility of the regressions, now.
> 

Wouldn't this regress the recently fixed bug for fr-FR keyboards, where 6 is Shift+- so that distinguishing between Ctrl+- (decrease font size) and Ctrl+6 (Firefox: go to tab 6, SeaMonkey: go to Chatzilla window) requires, on that layout, not ignoring the Shift status even when Ctrl is depressed?
Comment 45 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-11 21:44:09 PDT
(In reply to comment #35)
> (From update of attachment 320454 [details] [diff] [review])
> +        if (IsCaseChangeableChar(ch) &&
> +            HasSameCharCaseInsensitive(ch, aCandidates))
> 
> The HasSameCharCaseInsensitive looks unnecessary.  Isn't this always true
> because we've already done

Oh, right! Thank you!

(In reply to comment #44)
> Wouldn't this regress the recently fixed bug for fr-FR keyboards, where 6 is
> Shift+- so that distinguishing between Ctrl+- (decrease font size) and Ctrl+6
> (Firefox: go to tab 6, SeaMonkey: go to Chatzilla window) requires, on that
> layout, not ignoring the Shift status even when Ctrl is depressed?

The behavior is a special case for AZERTY layout at *unshifted* key pressing. Don't worry the regression for that. I confirmed that is not regressed.

Ctrl+Shift+'-': switching the active tab to 6th.
Ctrl+'-': zoom-out.
Comment 46 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-05-11 21:48:34 PDT
Can we have an automated test for comment #44?
Comment 47 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-12 07:01:06 PDT
(In reply to comment #46)
> Can we have an automated test for comment #44?

ok, I'll post the tests.
Comment 48 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-12 08:27:04 PDT
Created attachment 320548 [details] [diff] [review]
additional tests
Comment 49 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-05-12 09:52:17 PDT
Comment on attachment 320548 [details] [diff] [review]
additional tests

excellent. I guess we won't be able to land these until the mozilla-central tree opens.
Comment 50 :Gavin Sharp [email: gavin@gavinsharp.com] 2008-05-12 10:33:49 PDT
I don't see why you couldn't just land them in CVS now.
Comment 51 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-05-12 10:36:46 PDT
There is a code change.
Comment 52 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-05-15 10:35:49 PDT
Roc or someone:

looks like the tree is opened now. but I cannot commit it to tree now, would you do it?

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