Open Bug 960836 (e10s-ime-tsf) Opened 6 years ago Updated 4 years ago

[TSF] TSF support on e10s

Categories

(Core :: Widget: Win32, defect)

x86
Windows 8.1
defect
Not set

Tracking

()

Tracking Status
e10s + ---

People

(Reporter: m_kato, Assigned: m_kato)

References

(Depends on 2 open bugs, Blocks 1 open bug)

Details

(Keywords: dogfood, inputmethod, meta)

Even if landed bug 960493, each character seems to be always committed, not keeping composition state.  This is TSF+e10s only issue.
Depends on: 960493
If OnTextChangeMsg is called, text is committed, not keep composition state.

Non-e10s, OnTextChangeInternal is called from composition/text event. So when OnTextChangeInternal is called, document is always locked since FlushPendingActions is called when locked.

OnTextChangeInternal is useless now on non-e10s...
(In reply to Makoto Kato (:m_kato) from comment #1)
> If OnTextChangeMsg is called, text is committed, not keep composition state.

If it's caused by our editor, it might be fixed in the future. Otherwise, it may depend on TIP.

> Non-e10s, OnTextChangeInternal is called from composition/text event. So
> when OnTextChangeInternal is called, document is always locked since
> FlushPendingActions is called when locked.

Text change notifications have to be ignored if the change is caused by TIP's composition. Therefore, while document is locked, OnTextChangeInternal() does nothing. However, if it's called by a change of editor content caused by JS or without composition (e.g., direct input from keyboard), we need to notify TSF of text change.
Depends on: 960877
Blocks: e10s
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #2)
> (In reply to Makoto Kato (:m_kato) from comment #1)
> > If OnTextChangeMsg is called, text is committed, not keep composition state.
> 
> If it's caused by our editor, it might be fixed in the future. Otherwise, it
> may depend on TIP.

When having composition, if we calls mSink->OnTextChange, it seems to be composition information into TIP will be broken.  Then, TIP doesn't work after that.  

- Microsoft IME commits composition string by abandon state.
- Google IME and ATOK becomes TIP doesn't work well.

> > Non-e10s, OnTextChangeInternal is called from composition/text event. So
> > when OnTextChangeInternal is called, document is always locked since
> > FlushPendingActions is called when locked.
> 
> Text change notifications have to be ignored if the change is caused by
> TIP's composition. Therefore, while document is locked,
> OnTextChangeInternal() does nothing. However, if it's called by a change of
> editor content caused by JS or without composition (e.g., direct input from
> keyboard), we need to notify TSF of text change.

On e10s, DispatchEvent is asynchronized, so OnTextChangedInternal is called by unlocked document and having composition.  Then, it always hits debug assertion.

Maybe, we should check composition state on WinIMEHandler.cpp to call OnTextChangedInternal.

This situation is same as OnSelectionChange.
(In reply to Makoto Kato (:m_kato) from comment #3)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #2)
> > (In reply to Makoto Kato (:m_kato) from comment #1)
> > > If OnTextChangeMsg is called, text is committed, not keep composition state.
> > 
> > If it's caused by our editor, it might be fixed in the future. Otherwise, it
> > may depend on TIP.
> 
> When having composition, if we calls mSink->OnTextChange, it seems to be
> composition information into TIP will be broken.  Then, TIP doesn't work
> after that.  
> 
> - Microsoft IME commits composition string by abandon state.
> - Google IME and ATOK becomes TIP doesn't work well.

> This situation is same as OnSelectionChange.

I'm thinking that, anyway, we need to create a wall for preventing the critical state.

> > > Non-e10s, OnTextChangeInternal is called from composition/text event. So
> > > when OnTextChangeInternal is called, document is always locked since
> > > FlushPendingActions is called when locked.
> > 
> > Text change notifications have to be ignored if the change is caused by
> > TIP's composition. Therefore, while document is locked,
> > OnTextChangeInternal() does nothing. However, if it's called by a change of
> > editor content caused by JS or without composition (e.g., direct input from
> > keyboard), we need to notify TSF of text change.
> 
> On e10s, DispatchEvent is asynchronized, so OnTextChangedInternal is called
> by unlocked document and having composition.  Then, it always hits debug
> assertion.

I've concerned about it and hoped I'm wrong, though...

If we add a new flag to nsIMEUpdatePreference.mWantUpdates, e.g., IGNORE_CHANGES_BY_COMPOSITION and nsTextStateManager can ignore any changes during dispatching composition/text events. I think that this improves running performance during composition.

I think that we should do this, first. Should I do it in another bug?

However, even while there is composition, text change and selection change may occur by JS. We need to care this case.

> Maybe, we should check composition state on WinIMEHandler.cpp to call
> OnTextChangedInternal.

No. It should be done in nsTextStore because MetroWidget directly communicates with nsTextStore.
Keywords: inputmethod
Summary: TSF support on e10s → [TSF] TSF support on e10s
Depends on: 966157
Depends on: 975260
FYI: I'd like to enable TSF mode in default settings only on Nightly in next cycle (34). Check bug 1037328 for that.
Blocks: e10s-ime
Alias: e10s-ime-tsf
removing this from m2, we are blocking on individual bugs.
No longer blocks: old-e10s-m2
Keywords: dogfood
Depends on: 1136903
Depends on: 1147722
Keywords: meta
You need to log in before you can comment on or make changes to this bug.