405151 - IME handling is killed at opening a popup which has |ignorekeys="true"|

Status: RESOLVED FIXED

(Core :: XUL, defect, P1)

Description

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

this is a regression of bug 400893.

Seamonkey's popup has |ignorekeys="true"| now. By this changing, autocomplete popup kills IME handling. We should not kill the IME handling when the popups are not menus.

Comment 1

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

Attachment: Patch rv1.0

quick fix.

We don't need to kill the IME handling when popup isn't menu. Even if the non-menu popup has menu-like accesskeys.

Comment 2

[Neil Deakin]

(In reply to comment #1)
> Created an attachment (id=289962) [details]
> Patch rv1.0
> 
> quick fix.
> 
> We don't need to kill the IME handling when popup isn't menu. Even if the
> non-menu popup has menu-like accesskeys.
> 

I'm not sure sure I understand this patch.

When a non-menu panel is opened, we need to call nsContentUtils::NotifyInstalledMenuKeyboardListener(PR_FALSE) even though it was never called with a value of PR_TRUE?

What exactly does that method do?

Comment 3

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

If (In reply to comment #2)
> When a non-menu panel is opened, we need to call
> nsContentUtils::NotifyInstalledMenuKeyboardListener(PR_FALSE) even though it
> was never called with a value of PR_TRUE?

There are no reason. We don't need to call it at that time. But the implementation of the notification was optimized, so you don't need to worry about it.

> What exactly does that method do?

Actually, IME is disabled/enabled by the notification. But the semantics of the method should be only the notification for the menus being opened/closed.

Comment 4

[Neil Deakin]

This is the IME related behaviour for non-menu popups currently:

Opened:
  nsContentUtils::NotifyInstalledMenuKeyboardListener(PR_TRUE);
Closed:
  nsContentUtils::NotifyInstalledMenuKeyboardListener(PR_FALSE);

This is the new behaviour after this patch:

Panel is opened:
  nsContentUtils::NotifyInstalledMenuKeyboardListener(PR_FALSE);
Panel is closed:
  nsContentUtils::NotifyInstalledMenuKeyboardListener(PR_FALSE);

I'm not saying it's wrong, I just want to make sure I understand why.

Comment 5

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

(In reply to comment #4)
> This is the IME related behaviour for non-menu popups currently:
> 
> Opened:
>   nsContentUtils::NotifyInstalledMenuKeyboardListener(PR_TRUE);
> Closed:
>   nsContentUtils::NotifyInstalledMenuKeyboardListener(PR_FALSE);
> 
> This is the new behaviour after this patch:
> 
> Panel is opened:
>   nsContentUtils::NotifyInstalledMenuKeyboardListener(PR_FALSE);
> Panel is closed:
>   nsContentUtils::NotifyInstalledMenuKeyboardListener(PR_FALSE);
> 
> I'm not saying it's wrong, I just want to make sure I understand why.

When menus are opened but the IME handling is not killed, we cannot use menu mnemonics, because the key typing is hooked by IME. However, other popups doesn't have mnemonics and it might really need to handle the IMEs. I.e., we should be handling IME even if the autocomplete is popped up. Therefore, we don't kill the IME handling when non-menu pupup is opened. nsIMEStateManager is changing the IME handling state when nsContentUtils::NotifyInstalledMenuKeyboardListener is called. So, we need to notify the timing.

Comment 6

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

checked-in.