1475153 - TSFTextStore should merge new composition to old composition if it's possible

Status: RESOLVED FIXED

(Core :: Widget: Win32, enhancement)

Description

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

This is a part of bug 1462257.

When user removes all composition string, MS Pinyin removes last character with following odd steps:

1. commits the last character.
2. select the committed character.
3. replaces the selection with another temporary composition which commits with empty string.

Even after fix of bug 1462257, user may see selected last character when removing the last character.  Additionally, undo transaction is split.  First undo restores the last character.  Then, second undo removes the last character.

When this occurs, TSFTextStore's pending actions are:

> D/nsTextStoreWidgets 0x000002940B80D190   TSFTextStore::FlushPendingActions() flushing Type::eCompositionEnd={ mData="n" }
> D/nsTextStoreWidgets 0x000002940B80D190   TSFTextStore::FlushPendingActions(), dispatching compositioncommit event...
> D/nsTextStoreWidgets 0x000002940B80D190   TSFTextStore::FlushPendingActions() flushing Type::eCompositionStart={ mSelectionStart=0, mSelectionLength=1 }, mDestroyed=false
> D/nsTextStoreWidgets 0x000002940B80D190   TSFTextStore::FlushPendingActions() dispatching compositionstart event...
> D/nsTextStoreWidgets 0x000002940B80D190   TSFTextStore::FlushPendingActions() flushing Type::eCompositionEnd={ mData="" }
> D/nsTextStoreWidgets 0x000002940B80D190   TSFTextStore::FlushPendingActions(), dispatching compositioncommit event...

So, if composition is restarted immediately after composition end and the range is exactly same as previous commit string, we can merge those compositions and making the behavior in web apps simpler.

Comment 1

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=cdea38604bd2dacd901cf34ecaed410970a2f2c4

Comment 2

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

Attachment: Bug 1475153 - Make TSFTextStore::RecordCompositionStartAction() merge new composition with previous composition if IME commits composition and restart composition to replace the previous commit string

When user removes all composition string of MS Pinyin, MS Wubi, MS ChangJie and
MS Quick with Backspace key, IME commits last character temporarily and
restart composition to replace the last character with empty string when
user tries to remove last one character.

This causes flicking composition string because the additional composition
selects the character and it may be painted immediately before removed, and
also editor will have unnecessary undo transaction.

Therefore, as same as bug 1208043, TSFTextStore::RecordCompositionStartAction()
should restart last composition in such case.  Fortunately, we implemented
similar code for bug 1208043, however, unfortunately, we don't have preceding
pending compositionstart in this case.  Therefore, this patch makes
pending compositionend store start offset of composition.  Then, we can
restart composition only with information stored by pending compositionend
action.  Additionally, this patch renames the method checking pending
actions for self-describing the new meaning.

Review commit: https://reviewboard.mozilla.org/r/256582/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/256582/

Comment 3

[Makoto Kato [:m_kato]]

Comment on attachment 8991662 [details]
Bug 1475153 - Make TSFTextStore::RecordCompositionStartAction() merge new composition with previous composition if IME commits composition and restart composition to replace the previous commit string

https://reviewboard.mozilla.org/r/256582/#review263628

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/618089e03bd6
Make TSFTextStore::RecordCompositionStartAction() merge new composition with previous composition if IME commits composition and restart composition to replace the previous commit string r=m_kato

Comment 5

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/618089e03bd6