Closed Bug 1049768 Opened 8 years ago Closed 8 years ago

[TSF] Glitches in Chinese IME with intl.tsf.enable=true

Categories

(Core :: Widget: Win32, defect)

x86
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: wmchan4, Assigned: masayuki)

References

Details

(Keywords: inputmethod)

Attachments

(3 files)

Attached image FF34-freecj.jpg
User Agent: Mozilla/5.0 (Windows NT 6.1; rv:34.0) Gecko/20100101 Firefox/34.0 (Beta/Release)
Build ID: 20140805170718

Steps to reproduce:

* You need a Traditional Chinese version of Windows 7 to install FreeCJ.
(1) Set intl.tsf.enable=true in FF Nightly;
(2) Download and install FreeCJ from http://input.foruto.com/freecj/. 
Direct download link: http://input.foruto.com/cgi-bin/Counter/download3.cgi;
(3) Open FF, go to a text area, e.g. www.pastebin.com;
(4) Type some Chinese texts. In my case I just entered "m", Space, "m", Space, "m", Space.... (without double quotes)


Actual results:

See Box (1) in the attachment.
You can see glitches (extra spaces), and the first Chinese character is not entered
at all. Also the character selection menu is not seen completely.


Expected results:

See Box (2) in the attachement. 
No extra spaces, the characters are entered responsively.
You also see the character menu with possible candidates for certain inputs.

FF31 isn't affected. 
In Nightly the glitch can be worked around by setting intl.tsf.enable=false.

Related discussion in http://forums.mozillazine.org/viewtopic.php?f=23&t=2858387

Question: Will TSF be enforced permanently in the foreseeable future? 
It will be a disaster for me.
Attached image FF34-ecj.jpg
With EasyChangJei (http://www.ecj.hk/index.html) it also shows another type of glitches.
In this case I type 

"m", Comma, Space, "m", Space

Extra double-byte Underscore appeared when it shouldn't happen.

Setting intl.tsf.enable to false also works around it.
Blocks: 1037328
Summary: Glitches in Chinese IME with intl.tsf.enable=true → [TSF] Glitches in Chinese IME with intl.tsf.enable=true
Thank you for the report. I'll take a look today.

(In reply to wmchan4 from comment #0)
> Question: Will TSF be enforced permanently in the foreseeable future? 
> It will be a disaster for me.

Of cause, yes but not soon.
Assignee: nobody → masayuki
Do you know how many users to use the FreeCJ? And also EasyChangJie?
Hmm, not showing candidate list window may be a bug of FreeCJ.

It tries to retrieve text position with GetTextExt() after inserting " ". However, it always fails since we've not calculate the layout including the new text yet. At this time, we return TS_E_NOLAYOUT for conforming TSF spec. However, due to a bug of TSF, TIP (FreeCJ) will get E_FAIL. For avoiding this bug, TIP should check if ITfContextView::GetRangeFromPoint() returns TS_E_NOLAYOUT when ITextStore::GetTextExt() returns E_FAIL. If it returns TS_E_NOLAYOUT, TIP should wait a call of ITextStoreACPSink::OnLayoutChange(). We need to contact them if this is true.

I'm still investigating other issues.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: inputmethod
I'll fix only the unexpected commit string in this bug.
Blocks: 1049488
No longer blocks: 1037328
Status: NEW → ASSIGNED
The cause of this bug is, these TIPs don't call OnUpdateComposition() with proper range.

If OnUpdateComposition() is never called with new range, we should forcibly record the composition update action before flushing the pending actions.
Attachment #8468975 - Flags: review?(VYV03354)
Comment on attachment 8468975 [details] [diff] [review]
If OnCompositionUpdate() never called with new range, we should call RecordCompositionUpdateAction() forcibly before flushing pending actions

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

::: widget/windows/nsTextStore.cpp
@@ +2935,5 @@
>    }
>  
>    // pRangeNew is null when the update is not complete
>    if (!pRangeNew) {
> +    PendingAction* action = GetPendingCompositionUpdate();

It was not obvious that this function will never return null. It should have been named as something like LastOrNewPendingCompositionUpdate().
Attachment #8468975 - Flags: review?(VYV03354) → review+
(In reply to Masatoshi Kimura [:emk] from comment #7)
> Comment on attachment 8468975 [details] [diff] [review]
> > +    PendingAction* action = GetPendingCompositionUpdate();
> 
> It was not obvious that this function will never return null. It should have
> been named as something like LastOrNewPendingCompositionUpdate().

Indeed. Thanks!
https://hg.mozilla.org/mozilla-central/rev/1f01cae60e26
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.