Bug 1475153 - 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
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.
MozReview-Commit-ID: 1RyuacxEbky
--- a/widget/windows/TSFTextStore.cpp
+++ b/widget/windows/TSFTextStore.cpp
@@ -5036,27 +5036,29 @@ TSFTextStore::InsertTextAtSelectionInter
compositionStart->mSelectionStart = oldSelection.acpStart;
compositionStart->mSelectionLength =
oldSelection.acpEnd - oldSelection.acpStart;
compositionStart->mAdjustSelection = false;
PendingAction* compositionEnd = mPendingActions.AppendElement();
compositionEnd->mType = PendingAction::Type::eCompositionEnd;
compositionEnd->mData = aInsertStr;
+ compositionEnd->mSelectionStart = compositionStart->mSelectionStart;
MOZ_LOG(sTextStoreLog, LogLevel::Debug,
("0x%p TSFTextStore::InsertTextAtSelectionInternal() "
"appending pending compositionstart and compositionend... "
"PendingCompositionStart={ mSelectionStart=%d, "
"mSelectionLength=%d }, PendingCompositionEnd={ mData=\"%s\" "
- "(Length()=%u) }",
+ "(Length()=%u), mSelectionStart=%d }",
this, compositionStart->mSelectionStart,
compositionStart->mSelectionLength,
GetEscapedUTF8String(compositionEnd->mData).get(),
- compositionEnd->mData.Length()));
+ compositionEnd->mData.Length(),
+ compositionEnd->mSelectionStart));
}
contentForTSF.ReplaceSelectedTextWith(aInsertStr);
if (aTextChange) {
aTextChange->acpStart = oldSelection.acpStart;
aTextChange->acpOldEnd = oldSelection.acpEnd;
aTextChange->acpNewEnd = contentForTSF.Selection().EndOffset();
@@ -5141,28 +5143,32 @@ TSFTextStore::RecordCompositionStartActi
("0x%p TSFTextStore::RecordCompositionStartAction() FAILED due to "
"destroyed during dispatching a keyboard event", this));
return false;
}
CompleteLastActionIfStillIncomplete();
// TIP may have inserted text at selection before calling
- // OnStartComposition(). In this case, we've already created a pair of
- // pending compositionstart and pending compositionend. If the pending
- // compositionstart occurred same range as this composition, it was the
- // start of this composition. In such case, we should cancel the pending
- // compositionend and start composition normally.
+ // OnStartComposition(). In this case, we've already created a pending
+ // compositionend. If new composition replaces all commit string of the
+ // pending compositionend, we should cancel the pending compositionend and
+ // keep the previous composition normally.
+ // On Windows 7, MS-IME for Korean, MS-IME 2010 for Korean and MS Old Hangul
+ // may start composition with calling InsertTextAtSelection() and
+ // OnStartComposition() with this order (bug 1208043).
+ // On Windows 10, MS Pinyin, MS Wubi, MS ChangJie and MS Quick commits
+ // last character and replace it with empty string with new composition
+ // when user removes last character of composition string with Backspace
+ // key (bug 1462257).
if (!aPreserveSelection &&
- WasTextInsertedWithoutCompositionAt(aStart, aLength)) {
+ IsLastPendingActionCompositionEndAt(aStart, aLength)) {
const PendingAction& pendingCompositionEnd = mPendingActions.LastElement();
- const PendingAction& pendingCompositionStart =
- mPendingActions[mPendingActions.Length() - 2];
- contentForTSF.RestoreCommittedComposition(
- aComposition, pendingCompositionStart, pendingCompositionEnd);
+ contentForTSF.RestoreCommittedComposition(aComposition,
+ pendingCompositionEnd);
mPendingActions.RemoveLastElement();
MOZ_LOG(sTextStoreLog, LogLevel::Info,
("0x%p TSFTextStore::RecordCompositionStartAction() "
"succeeded: restoring the committed string as composing string, "
"mComposition={ mStart=%ld, mString.Length()=%ld, "
"mSelectionForTSF={ acpStart=%ld, acpEnd=%ld, style.ase=%s, "
"style.fInterimChar=%s } }",
this, mComposition.mStart, mComposition.mString.Length(),
@@ -5234,16 +5240,17 @@ TSFTextStore::RecordCompositionEndAction
// composition update, we can forget them since composition end will send
// the latest composition string and it overwrites the composition string
// even if we dispatch eCompositionChange event before that. So, let's
// forget all composition updates now.
RemoveLastCompositionUpdateActions();
PendingAction* action = mPendingActions.AppendElement();
action->mType = PendingAction::Type::eCompositionEnd;
action->mData = mComposition.mString;
+ action->mSelectionStart = mComposition.mStart;
Content& contentForTSF = ContentForTSFRef();
if (!contentForTSF.IsInitialized()) {
MOZ_LOG(sTextStoreLog, LogLevel::Error,
("0x%p TSFTextStore::RecordCompositionEndAction() FAILED due "
"to ContentForTSFRef() failure", this));
return E_FAIL;
}
@@ -7145,34 +7152,31 @@ TSFTextStore::Content::StartComposition(
mSelection.SetSelection(mComposition.mStart, mComposition.mString.Length(),
false, mSelection.GetWritingMode());
}
}
void
TSFTextStore::Content::RestoreCommittedComposition(
ITfCompositionView* aCompositionView,
- const PendingAction& aPendingCompositionStart,
const PendingAction& aCanceledCompositionEnd)
{
MOZ_ASSERT(mInitialized);
MOZ_ASSERT(aCompositionView);
MOZ_ASSERT(!mComposition.mView);
- MOZ_ASSERT(aPendingCompositionStart.mType ==
- PendingAction::Type::eCompositionStart);
MOZ_ASSERT(aCanceledCompositionEnd.mType ==
PendingAction::Type::eCompositionEnd);
MOZ_ASSERT(GetSubstring(
- static_cast<uint32_t>(aPendingCompositionStart.mSelectionStart),
+ static_cast<uint32_t>(aCanceledCompositionEnd.mSelectionStart),
static_cast<uint32_t>(aCanceledCompositionEnd.mData.Length())) ==
aCanceledCompositionEnd.mData);
// Restore the committed string as composing string.
mComposition.Start(aCompositionView,
- aPendingCompositionStart.mSelectionStart,
+ aCanceledCompositionEnd.mSelectionStart,
aCanceledCompositionEnd.mData);
mLatestCompositionStartOffset = mComposition.mStart;
mLatestCompositionEndOffset = mComposition.EndOffset();
}
void
TSFTextStore::Content::EndComposition(const PendingAction& aCompEnd)
{
--- a/widget/windows/TSFTextStore.h
+++ b/widget/windows/TSFTextStore.h
@@ -695,18 +695,19 @@ protected:
{
eCompositionStart,
eCompositionUpdate,
eCompositionEnd,
eSetSelection,
eKeyboardEvent,
};
Type mType;
+ // For eCompositionStart, eCompositionEnd and eSetSelection
+ LONG mSelectionStart;
// For eCompositionStart and eSetSelection
- LONG mSelectionStart;
LONG mSelectionLength;
// For eCompositionStart, eCompositionUpdate and eCompositionEnd
nsString mData;
// For eCompositionUpdate
RefPtr<TextRangeArray> mRanges;
// For eKeyboardEvent
const MSG* mKeyMsg;
// For eSetSelection
@@ -733,40 +734,37 @@ protected:
PendingAction* newAction = mPendingActions.AppendElement();
newAction->mType = PendingAction::Type::eCompositionUpdate;
newAction->mRanges = new TextRangeArray();
newAction->mIncomplete = true;
return newAction;
}
/**
- * WasTextInsertedWithoutCompositionAt() checks if text was inserted without
- * composition immediately before (e.g., see InsertTextAtSelectionInternal()).
+ * IsLastPendingActionCompositionEndAt() checks whether the previous pending
+ * action is committing composition whose range starts from aStart and its
+ * length is aLength. In other words, this checks whether new composition
+ * which will replace same range as previous pending commit can be merged
+ * with the previous composition.
*
* @param aStart The inserted offset you expected.
* @param aLength The inserted text length you expected.
- * @return true if the last pending actions are
- * eCompositionStart and eCompositionEnd and
- * aStart and aLength match their information.
+ * @return true if the last pending action is
+ * eCompositionEnd and it inserted the text
+ * between aStart and aStart + aLength.
*/
- bool WasTextInsertedWithoutCompositionAt(LONG aStart, LONG aLength) const
+ bool IsLastPendingActionCompositionEndAt(LONG aStart, LONG aLength) const
{
- if (mPendingActions.Length() < 2) {
+ if (mPendingActions.IsEmpty()) {
return false;
}
const PendingAction& pendingLastAction = mPendingActions.LastElement();
- if (pendingLastAction.mType != PendingAction::Type::eCompositionEnd ||
- pendingLastAction.mData.Length() != ULONG(aLength)) {
- return false;
- }
- const PendingAction& pendingPreLastAction =
- mPendingActions[mPendingActions.Length() - 2];
- return pendingPreLastAction.mType ==
- PendingAction::Type::eCompositionStart &&
- pendingPreLastAction.mSelectionStart == aStart;
+ return pendingLastAction.mType == PendingAction::Type::eCompositionEnd &&
+ pendingLastAction.mSelectionStart == aStart &&
+ pendingLastAction.mData.Length() == static_cast<ULONG>(aLength);
}
bool IsPendingCompositionUpdateIncomplete() const
{
if (mPendingActions.IsEmpty()) {
return false;
}
const PendingAction& lastAction = mPendingActions.LastElement();
@@ -887,29 +885,27 @@ protected:
void ReplaceTextWith(LONG aStart, LONG aLength, const nsAString& aString);
void StartComposition(ITfCompositionView* aCompositionView,
const PendingAction& aCompStart,
bool aPreserveSelection);
/**
* RestoreCommittedComposition() restores the committed string as
* composing string. If InsertTextAtSelection() or something is called
- * before a call of OnStartComposition(), there is a pending
- * compositionstart and a pending compositionend. In this case, we
- * need to cancel the pending compositionend and continue the composition.
+ * before a call of OnStartComposition() or previous composition is
+ * committed and new composition is restarted to clean up the commited
+ * string, there is a pending compositionend. In this case, we need to
+ * cancel the pending compositionend and continue the composition.
*
* @param aCompositionView The composition view.
- * @param aPendingCompositionStart The pending compositionstart which
- * started the committed composition.
* @param aCanceledCompositionEnd The pending compositionend which is
* canceled for restarting the composition.
*/
void RestoreCommittedComposition(
ITfCompositionView* aCompositionView,
- const PendingAction& aPendingCompositionStart,
const PendingAction& aCanceledCompositionEnd);
void EndComposition(const PendingAction& aCompEnd);
const nsString& Text() const
{
MOZ_ASSERT(mInitialized);
return mText;
}