187839 - Caret disappears when turning on IME(JA)

Status: VERIFIED FIXED

(Core :: Internationalization, defect)

Description

[Koike Kazuhiko]

Caret disappears when you turn on IME in input field.

Reproduced with 2003010508/WinXP.

Original report in Bugzilla-jp:
http://bugzilla.mozilla.gr.jp/show_bug.cgi?id=2904

Comment 1

[Yuying Long]

Confirm on 01-03 trunk build on both WinME-JA and WinXP-SC when switch to
Japanese IME on text field, but not with Chinese IME/WinXP-SC.

This is a regression - not reproducible on N7.01(1.02branch).

Comment 2

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

I can reproduce when I downed alt key.

Messages      wParam     lParam
WM_IME_NOTIFY 0x0000000E 0x00000001
WM_SYSKEYDOWN 0x00000012 0x21380001

Comment 3

[Ryuichi KUBUKI]

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

Comment 4

[u69748]

I am annoyed by this bug every days!
This is not a minor bug for Japanese.

If 1.3b is released befor this bug is fixed,
I can't use it.

Comment 5

[Yuying Long]

This is a bad regression -> nsbeta1.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Can someone identify between which builds this bug started appearing?

Does this affect all Japanese IME users, or just some of them?  (What about
Chinese IME or others?)  I'm marking as blocking1.3b+ on the assumption that
it's all, although if it's only a small proportion I wouldn't maintain that.

Comment 7

[u69748]

2003010108-trunk OK
2003010408-trunk NG
Comment #1 says 01-03 trunk is NG

I think this is Windows specific and more than half 
of Japanese Windows users are affected by this bug.

I am binding right-ALT key to IME-key on WindowsXP.
When I type right-ALT key, then caret disappeared.
I can recall caret by clicking outside the textarea and 
clicking again inside the textarea.

When I type left-ALT key, which is not bound to any function,
then caret move to menu bar. And typing ESC or left-ALT or 
other keys, caret back to the textarea.

Comment 8

[Asa Dotzler [:asa]]

Who can help us here? is ftang still working on Mozilla?

Comment 9

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

I think, this problem is regression of Bug 132489?
this bug was checked in 01/02/2003 17:14

We have report, this bug is not reproduced with 2003010108
and this bug is reproduced with 2003010408.
http://bugzilla.mozilla.gr.jp/show_bug.cgi?id=2904#c0

see.
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/layout/xul/base/src&command=DIFF_FRAMESET&root=/cvsroot&file=nsMenuBarFrame.cpp&rev1=1.104&rev2=1.105

Comment 10

[Asa Dotzler [:asa]]

Aaron, it's been suggested that your fix could be responsible for this problem.
It's blocking 1.3beta. Can you take a look at see if you can help us out here?
Thanks. 

Comment 11

[Aaron Leventhal]

Yes, that's my code. 
The caret disappears when the alt key is pressed, because the alt key activates
the main menu.

So we're behaving like Internet Explorer and other programs. When the menubar is
focused, the caret disappears. Does this IME caret problem not occur with other
applications?

Comment 12

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

When I tested with native windows applications...
alt key down ....... No function
alt key up   ....... Menu has focus, and hide caret
esc key down ....... before focused control has focus, and show caret

but, Mozilla is...
alt key down ....... Menu has focus, and hide caret
alt key up   ....... No function
esc key down ....... before focused control has focus(I can't show caret)

Comment 13

[u69748]

I find 2 kinds of behavior in Windows Apps (IE, notepad, other text editors, etc.).

When ALT key is pressed, menu bar activated and:
1. the caret disappears.
2. the caret stop to blink.
When menubar is deactivated, the caret appears and starts to blink again.

I don't find this problem in these applications.

Comment 14

[Aaron Leventhal]

> When I tested with native windows applications...
> alt key down ....... No function
> alt key up   ....... Menu has focus, and hide caret

That's strange, this is the behavior I am getting on win2k mozilla.

What is the key combo used to turn on Japanese IME?
Is it Alt+Shift, or just Alt by itself?

Comment 15

[Aaron Leventhal]

Also, is this bug happening in win2k? If it is, I'd like to try and repro the
bug on my win2k system -- but where can I download Japanese IME for win2k?

Comment 16

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

We can reproduce with Win98,Win2k,WinMe and WinXP.
We trun on IME Alt + Hankaku/Zenkaku(Kanji) with 106 keyboard.

I don't know the case of that with 101 keyboard.

Adding Cc: momoi-san.

Comment 17

[Aaron Leventhal]

> What is the key combo used to turn on Japanese IME?
> Is it Alt+Shift, or just Alt by itself?
I'll answer my own question :)  I now see you metioned it is configured to use
right alt.

So ... I wonder why Mozilla's menu bar is activating when your right alt key
goes down rather than when it come up. On my win2k machine it waits for alt to
come back up.

Comment 18

[Shotaro Kamio]

Attachment: patch v1

I found Mozilla's menu bar calls |SetActive()| from somewhere
 even if the key combination is not assigned to menu bar.
(only in WIndows?)
This patch checks if the activity of menu bar is changed or not.

In this patch, direct assignment of |mIsActive| was replaced to |SetActive()|
call.
And the |SetActive()| has |FireDOMEvent()| call.
Does this cause some side-effects to DOMEvent?

Comment 19

[timeless]

aaronl: start>settings>control panel>keyboard>input locales>add>japanese, japanese.

yes i'm running w/ the japanese ime (i'm trying to cut ime bloat out of
nsWindow). My tree is currently frozen from pre darin and i'm working on other
things. if this isn't fixed before i merge to tip i'll play with it.

Comment 20

[timeless]

oh, you'll want to run one of these:
conime.exe
phime.exe
cjime.exe
imejpmgr.exe <- this one
pyime.exe
from your system32 directory.

Comment 21

[Aaron Leventhal]

Comment on attachment 113188 [details] [diff] [review]
patch v1

I think this fix might work -- you tested it right? It might be a pretty big
change for this stage. Another possible fix might be to not activate the menu
bar when we receive a WM_IME_NOTIFY with the alt key.


>+  PRBool isChangedActivity = (mIsActive != aActiveFlag) ? PR_TRUE : PR_FALSE;
+  PRBool isChangedActivity = (mIsActive != aActiveFlag);

>   mIsActive = aActiveFlag;
>   if (mIsActive) {
>     InstallKeyboardNavigator();
>@@ -239,14 +240,16 @@
>     if (!selCon)
>       break;
> 
>-    if (mIsActive) {// store whether caret was visible so that we can restore that state when menu is closed
>+    if (mIsActive && isChangedActivity) {// store whether caret was visible so that we can restore that state when menu is closed
>       PRBool isCaretVisible;
>       selCon->GetCaretEnabled(&isCaretVisible);
>       mCaretWasVisible |= isCaretVisible;
>     }
>-    selCon->SetCaretEnabled(!mIsActive && mCaretWasVisible);
>-    if (!mIsActive) {
>-      mCaretWasVisible = PR_FALSE;
>+    if (isChangedActivity) {
>+      selCon->SetCaretEnabled(!mIsActive && mCaretWasVisible);
>+      if (!mIsActive) {
>+        mCaretWasVisible = PR_FALSE;
>+      }
You don't need to test isChangedActivity twice. You can wrap the 2 if blocks
that use it in if (isChangedActivity) so that it has 1 less test.

>     }
>   } while (0);
> 
>@@ -379,7 +382,7 @@
>   if (result) {
>     // We got one!
>     aHandledFlag = PR_TRUE;
>-    mIsActive = PR_TRUE;
>+    SetActive(PR_TRUE);
>     SetCurrentMenuItem(result);
>     result->OpenMenu(PR_TRUE);
>     result->SelectFirstItem();
>@@ -627,14 +630,10 @@
>     return NS_OK;
>   }
> 
>-  // It's us. Just set our active flag to false.
>-  mIsActive = PR_FALSE;
>-
>   // Clear our current menu item if we've got one.
>   SetCurrentMenuItem(nsnull);
> 
>-  // Remove our keyboard navigator
>-  RemoveKeyboardNavigator();
>+  SetActive(PR_FALSE);
> 
>   // Clear out our dismissal listener
>   if (nsMenuFrame::sDismissalListener)

Firing the DOM event only has side effects when accessibility APIs are being
used -- the DOM event is used to tell assistive technology that the content
area now does or doesn't have focus. So this side effect is desirable whenever
the menubar active state changes. So, that part of the change should be
non-risky, and good for accessibility.

Should we even be activating the menu bar when the alt key is being used for
IME?

Comment 22

[Shotaro Kamio]

Attachment: patch v2

If we remove  |SetActive(PR_FALSE)| from |nsMenuBarFrame::DismissChain()|,
we can suppress original reporter's bug.
But there is another bug.
Clicking a few menu items sequentially, the caret disappears.
This is because |SetActive()| is triggered multiple time in same activity.

It seems to me that checking changes of the activity is reasonable.

Comment 23

[Aaron Leventhal]

Looks reasonable to me. One question:

Do we really need to set the menus to active state in ShortcutNavigation()?
Wouldn't they already be active? What happens if we just remove |mActive =
PR_TRUE;| -- it is it unneccessary?

@@ -379,7 +383,7 @@
   if (result) {
     // We got one!
     aHandledFlag = PR_TRUE;
-    mIsActive = PR_TRUE;
+    SetActive(PR_TRUE);
     SetCurrentMenuItem(result);
     result->OpenMenu(PR_TRUE);
     result->SelectFirstItem();

This patch seems like the way to go, although I still don't see why the menubar
should activate at all when IME is being activated, or why the menubar is
getting activated when alt is onkeydown.

I'd like to see what bryner or hyatt think of the patch.

Comment 24

[Aaron Leventhal]

Comment on attachment 113295 [details] [diff] [review]
patch v2

I'll answer my own question about needing to set the menubar to the active
state in ShortcutNavigation(). We do need that, because the menu bar is not yet
active when the user type Alt+letter from content, to access the menubar.

This patch seems like the right fix to me.

Comment 25

[Aaron Leventhal]

checked in -- thanks Shotaro!

Comment 26

[Yuying Long]

Verified fixed in 02-04 trunk build / WinME and WinXP.

Comment 27

[John Morrison]

please to be removing the 'printf("\nIs active?...' that was accidentally checked 
in and now shows up every time you open a menu in opt. release builds.

Comment 28

[Aaron Leventhal]

Sorry about the printf!

Fixed again, without the printf this time :-)

Comment 29

[John Morrison]

back to verif.

Comment 30

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 113188 [details] [diff] [review]
patch v1

(clearing review request from obsolete patch)