Closed Bug 491438 Opened 15 years ago Closed 15 years ago

[IMM32] nsIMM32Handler should not be created by some WM_IME_* messages

Categories

(Core :: Widget: Win32, defect)

x86
Windows Vista
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

(Keywords: inputmethod)

Attachments

(1 file, 3 obsolete files)

Attached patch Patch v1.0 (obsolete) — Splinter Review
if the system was installed one or more IMEs, WM_IME_SETCONTEXT and WM_IME_NOTIFY is called even if the current keyboard layout doesn't use IME. At this time, we should not create the instance of nsIMM32Handler for footprint.

I think that WM_IME_CHAR, WM_IME_NOTIFY, WM_IME_REQUEST, WM_IME_SELECT and WM_IME_SETCONTEXT can be processed without nsIMM32Handler instance. So, their handlers can be static methods.

However, for WM_IME_REQUEST, I need more works. Temporary, we should keep their handlers being non-static.
Attachment #375759 - Flags: review?(VYV03354)
> +  DWORD IMEProperty =
> +    gIMM32Handler ? gIMM32Handler->mIMEProperty :
> +                    ::ImmGetProperty(::GetKeyboardLayout(0), IGP_PROPERTY);
Looks ugly. Could you make InitKeyboardLayout and mIMEProperty static?
Attached patch Patch v2.0 (obsolete) — Splinter Review
Attachment #375759 - Attachment is obsolete: true
Attachment #375770 - Flags: review?(VYV03354)
Attachment #375759 - Flags: review?(VYV03354)
Comment on attachment 375770 [details] [diff] [review]
Patch v2.0

ah, I should fix some nits, sorry.
Attachment #375770 - Flags: review?(VYV03354)
Attached patch Patch v2.1 (obsolete) — Splinter Review
Attachment #375770 - Attachment is obsolete: true
Attachment #375771 - Flags: review?(VYV03354)
Comment on attachment 375771 [details] [diff] [review]
Patch v2.1

> +  ::GetLocaleInfoA(MAKELCID(langID, SORT_DEFAULT),
> +                   LOCALE_IDEFAULTANSICODEPAGE | LOCALE_RETURN_NUMBER,
> +                   (PSTR)&sCodePage, sizeof(sCodePage));
Let's replace GetLocaleInfoA with GetLocaleInfoW. I should have pointed out when I saw this in bug 424663.
Don't forget changing 4th parameter to |sizeof(sCodePage) / sizeof(WCHAR)|.
Otherwise looks good.
Attachment #375771 - Flags: review?(VYV03354) → review+
Attached patch Patch v2.2Splinter Review
Thank you, Kimura-san!
Attachment #375771 - Attachment is obsolete: true
Attachment #375818 - Flags: superreview?(roc)
Attachment #375818 - Flags: superreview?(roc) → superreview+
http://hg.mozilla.org/mozilla-central/rev/c56c2ad1eab0

# Note: fixed a typo in comment: menbers -> members
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: