789706 - [TSF] Candidate window of ATOK isn't located at composition string

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

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

ATOK 2012's composition window isn't located at composition string. Instead, it's located bottom of our window.

I'm not sure this is a bug of us, we need to investigate it.

Comment 1

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

I cannot reproduce this bug with ATOK 2013 preview:
http://www.justsystems.com/jp/services/atok/preview/

And also, I cannot reproduce this bug with ATOK 2011.

So that I believe this is a bug of ATOK 2012. However, if we will enable TSF mode in our default settings, we need to add this to the release note written in Japanese.

Comment 2

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

I found a way to fix this.

If we create native caret over our caret, we can avoid this issue.

Comment 3

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

Attachment: part.1 Create native caret over our caret when ITextStoreACP::GetTextExt() is called during composition

I think that we should not call CreateNativeCaret() when active TIP isn't ATOK.

I'll try to do so with next patch.

Comment 4

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

Attachment: part.1 Create native caret over our caret when ITextStoreACP::GetTextExt() is called during composition

ATOK refers native caret position of desktop application at deciding its candidate window position. Let's create native caret over our caret. This hack is similar to the hack for Chinese IME in nsIMM32Handler.

Comment 5

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

Attachment: part.2 nsTextStore::CreateNativeCaret() should be called only when active TIP is ATOK

This patch makes nsTextStore store active keyboard TIP name. And the hack of the previous patch should run only when the active keyboard TIP is ATOK. Because the other TIP users shouldn't waste the running cost.

Additionally, this patch has a pref for disabling the hack for ATOK developers.

Comment 6

[Masatoshi Kimura [:emk]]

Is this hack needed for ATOK versions other than 2012? (Comment #1 indicates it isn't.)

Comment 7

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

(In reply to Masatoshi Kimura [:emk] from comment #6)
> Is this hack needed for ATOK versions other than 2012? (Comment #1 indicates
> it isn't.)

Yes, I confirmed with 2011, 2012 and 2013. And perhaps, 2014 too (AFAIK).
# 2011 is the first version of TIP-ATOK.

Comment 8

[Masatoshi Kimura [:emk]]

I can't reproduce this bug with Windows 8.1 desktop + ATOK 2011/2012/2013 + Nightly 29.0a1 + intl.enable_tsf_support=true. The composition window was located at the composition string.

Comment 9

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

The pref was changed to intl.tsf.enable. (If you enabled TSF, the pref should be migrated automatically)

Comment 10

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

FYI: I can reproduce this bug on normal window. E.g., web contents or search bar. However, I cannot reproduce this bug on modal dialog such as "Option". I don't know the reason. I asked JustSystems unofficially, but I've not received the reply from them :-(

Comment 11

[Masatoshi Kimura [:emk]]

Wow, my Nightly was pretty old (2013-12-23).
The pref was surely migrated when I updated the Nightly, but the bug is still not reproduced...

I tested this on the search bar. And I have tested it on the web content now. Still not reproduced.

Comment 12

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

Hmm, it's odd... On my all environments with ATOK (except WinXP), I can reproduce this bug... The candidate window is always displayed at left-bottom corner of the window. Can you test it on pure environment on VM? I can reproduce it on Win7 x64 ja-JP on VM.

According to the log, ATOK checks the composition string's rect with GetTextExt() and nsTextStore returns correct value. However, ATOK ignores it...

Comment 13

[Masatoshi Kimura [:emk]]

I installed a new Win7 x64 VM and tested. I was able to reproduce the bug on the VM.

Comment 14

[Masatoshi Kimura [:emk]]

But could not reproduce on Win8.1 x64 VM.

Comment 15

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

(In reply to Masatoshi Kimura [:emk] from comment #14)
> But could not reproduce on Win8.1 x64 VM.

Hmm, odd... If we can know the difference, we can advise JustSystems. However, anyway, we should fix this bug since we cannot ignore this bug now that we know the workaround. If you have no idea of the difference, please review the patches.

Comment 16

[Masatoshi Kimura [:emk]]

Comment on attachment 8367068 [details] [diff] [review]
part.1 Create native caret over our caret when ITextStoreACP::GetTextExt() is called during composition

OK, at least this has no ill effect.

Comment 17

[Masatoshi Kimura [:emk]]

Comment on attachment 8367072 [details] [diff] [review]
part.2 nsTextStore::CreateNativeCaret() should be called only when active TIP is ATOK

Review of attachment 8367072 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/windows/nsTextStore.cpp
@@ +154,5 @@
>  
>  #define TEXTSTORE_DEFAULT_VIEW (1)
>  
> +static nsCString
> +GetCLSIDNameStr(REFCLSID aCLSID)

Why did you move this function?

@@ +169,5 @@
> +  return result;
> +}
> +
> +static nsCString
> +GetGUIDNameStr(REFGUID aGUID)

ditto

Comment 18

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

(In reply to Masatoshi Kimura [:emk] from comment #17)
> Comment on attachment 8367072 [details] [diff] [review]
> part.2 nsTextStore::CreateNativeCaret() should be called only when active
> TIP is ATOK
> 
> Review of attachment 8367072 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: widget/windows/nsTextStore.cpp
> @@ +154,5 @@
> >  
> >  #define TEXTSTORE_DEFAULT_VIEW (1)
> >  
> > +static nsCString
> > +GetCLSIDNameStr(REFCLSID aCLSID)
> 
> Why did you move this function?
> 
> @@ +169,5 @@
> > +  return result;
> > +}
> > +
> > +static nsCString
> > +GetGUIDNameStr(REFGUID aGUID)
> 
> ditto

Ah, it is for my first approach which reads registry. However, I found the API to get the description of TIP. So, I can remove the change now. Thanks!

Comment 19

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/ff3c388e5410
https://hg.mozilla.org/integration/mozilla-inbound/rev/e6fe72a30359

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ff3c388e5410
https://hg.mozilla.org/mozilla-central/rev/e6fe72a30359