Sould not use obsoleted API(WINNLSEnableIME)

RESOLVED FIXED in mozilla1.9alpha1

Status

()

RESOLVED FIXED
13 years ago
13 years ago

People

(Reporter: masayuki, Assigned: emk)

Tracking

({intl})

Trunk
mozilla1.9alpha1
x86
Windows 2000
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

We are using obsoleted API for IME disabling by bug 276727.
Masatoshi Kimura-san said, we can use ImmAssociateContext instead of WINNLSEnableIME.
I confirmed that. It's good way. We should replace this API.
Created attachment 203209 [details] [diff] [review]
Patch rv1.0(created by Kimura-san)
Attachment #203209 - Flags: review+
(Reporter)

Updated

13 years ago
Status: NEW → ASSIGNED
Comment on attachment 203209 [details] [diff] [review]
Patch rv1.0(created by Kimura-san)

timeless:

Looks good for me.
Please check it too.
Attachment #203209 - Flags: review?(timeless)

Comment 3

13 years ago
Comment on attachment 203209 [details] [diff] [review]
Patch rv1.0(created by Kimura-san)

>RCS file: /cvsroot/mozilla/widget/src/windows/nsWindow.cpp,v
>retrieving revision 3.594
>@@ -7353,37 +7351,30 @@ NS_IMETHODIMP nsWindow::GetIMEOpenState(
> NS_IMETHODIMP nsWindow::SetIMEEnabled(PRBool aState)
>+  if (aState && !mOldIMC)
>+    return NS_OK; // already enabled
>+  if (!aState && mOldIMC)
>+    return NS_OK; // already disabled

you could use == or ^ instead of these paired conditions:

if (!aState == mOldIMC)
  return NS_OK;
Attachment #203209 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #203209 - Flags: review?(timeless)
Attachment #203209 - Flags: review+
(In reply to comment #3)
> you could use == or ^ instead of these paired conditions:
> 
> if (!aState == mOldIMC)
>   return NS_OK;
> 

Is this right? I think:

if (!aState == !!mOldIMC)
  return NS_OK;
(Assignee)

Comment 5

13 years ago
IMO that is bit tricky and unreadable. Actually, timeless made a mistake :-)

Comment 6

13 years ago
Comment on attachment 203209 [details] [diff] [review]
Patch rv1.0(created by Kimura-san)

>+  if (aState && !mOldIMC)
>+    return NS_OK; // already enabled
>+  if (!aState && mOldIMC)
>+    return NS_OK; // already disabled
While I don't understand why enabling IME would give you an old IMC my vote for the test goes to if (!aState != !mOldIMC)
Attachment #203209 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
Kimura-san:

Could you create new patch for check-in?
(Assignee)

Comment 8

13 years ago
Created attachment 203391 [details] [diff] [review]
Patch rv1.1

Incorporated neil's suggestion
Attachment #203209 - Attachment is obsolete: true
checked-in. thank you for your work!
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.