Bug 1224994 part.9 TSFTextStore shouldn't set selection when there is unknown pending text changes r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 30 Jun 2016 17:55:01 +0900
changeset 383075 12ed8368b813000a05bca0d15302f697abf84d33
parent 383074 0f6ece6cf96421722890e62e9bc7c4a5f8c5bc2d
child 383076 91677e5cbece4580e5663181461abcb68922f6ad
push id21915
push usermasayuki@d-toybox.com
push dateFri, 01 Jul 2016 05:16:51 +0000
reviewersm_kato
bugs1224994
milestone50.0a1
Bug 1224994 part.9 TSFTextStore shouldn't set selection when there is unknown pending text changes r?m_kato I'm still not sure what we should do in this case, though. If mContentForTSF is initialized and there are some unknown changes in actual contents, i.e., not caused by composition of the active TIP itself, we cannot set selection range properly in some cases. For example, if TSF tires to set non-empty selection range but the range has been removed by web apps. For now, let's try to return E_FAIL in such case because that should occur at reconversion or something immediately after previous content change not caused by previous composition. If TIP does nothing in this case, user can retry with same operation after all pending text changes are notified to TSF. MozReview-Commit-ID: 9unrNVeC1tW
widget/windows/TSFTextStore.cpp
widget/windows/TSFTextStore.h
--- a/widget/windows/TSFTextStore.cpp
+++ b/widget/windows/TSFTextStore.cpp
@@ -2665,16 +2665,27 @@ TSFTextStore::SetSelectionInternal(const
   Selection& currentSel = CurrentSelection();
   if (currentSel.IsDirty()) {
     MOZ_LOG(sTextStoreLog, LogLevel::Error,
        ("TSF: 0x%p   TSFTextStore::SetSelectionInternal() FAILED due to "
         "CurrentSelection() failure", this));
     return E_FAIL;
   }
 
+  // If actually the range is not changing, we should do nothing.
+  // Perhaps, we can ignore the difference change because it must not be
+  // important for following edit.
+  if (currentSel.EqualsExceptDirection(*pSelection)) {
+    MOZ_LOG(sTextStoreLog, LogLevel::Error,
+       ("TSF: 0x%p   TSFTextStore::SetSelectionInternal() Succeeded but "
+        "did nothing because the selection range isn't changing", this));
+    currentSel.SetSelection(*pSelection);
+    return S_OK;
+  }
+
   if (mComposition.IsComposing()) {
     if (aDispatchCompositionChangeEvent) {
       HRESULT hr = RestartCompositionIfNecessary();
       if (FAILED(hr)) {
         MOZ_LOG(sTextStoreLog, LogLevel::Error,
            ("TSF: 0x%p   TSFTextStore::SetSelectionInternal() FAILED due to "
             "RestartCompositionIfNecessary() failure", this));
         return hr;
@@ -2696,23 +2707,60 @@ TSFTextStore::SetSelectionInternal(const
            ("TSF: 0x%p   TSFTextStore::SetSelectionInternal() FAILED due to "
             "RecordCompositionUpdateAction() failure", this));
         return hr;
       }
     }
     return S_OK;
   }
 
+  TS_SELECTION_ACP selectionInContent(*pSelection);
+
+  // If mContentForTSF caches old contents which is now different from
+  // actual contents, we need some complicated hack here...
+  // Note that this hack assumes that this is used for reconversion.
+  if (mContentForTSF.IsInitialized() &&
+      mPendingTextChangeData.IsValid() &&
+      !mPendingTextChangeData.mCausedOnlyByComposition) {
+    uint32_t startOffset = static_cast<uint32_t>(selectionInContent.acpStart);
+    uint32_t endOffset = static_cast<uint32_t>(selectionInContent.acpEnd);
+    if (mPendingTextChangeData.mStartOffset >= endOffset) {
+      // Setting selection before any changed ranges is fine.
+    } else if (mPendingTextChangeData.mRemovedEndOffset <= startOffset) {
+      // Setting selection after removed range is fine with following
+      // adjustment.
+      selectionInContent.acpStart += mPendingTextChangeData.Difference();
+      selectionInContent.acpEnd += mPendingTextChangeData.Difference();
+    } else if (startOffset == endOffset) {
+      // Moving caret position may be fine in most cases even if the insertion
+      // point has already gone but in this case, composition will be inserted
+      // to unexpected position, though.
+      // It seems that moving caret into middle of the new text is odd.
+      // Perhaps, end of it is expected by users in most cases.
+      selectionInContent.acpStart = mPendingTextChangeData.mAddedEndOffset;
+      selectionInContent.acpEnd = selectionInContent.acpStart;
+    } else {
+      // Otherwise, i.e., setting range has already gone, we cannot set
+      // selection properly.
+      MOZ_LOG(sTextStoreLog, LogLevel::Error,
+         ("TSF: 0x%p   TSFTextStore::SetSelectionInternal() FAILED due to "
+          "there is unknown content change", this));
+      return E_FAIL;
+    }
+  }
+
   CompleteLastActionIfStillIncomplete();
   PendingAction* action = mPendingActions.AppendElement();
   action->mType = PendingAction::SET_SELECTION;
-  action->mSelectionStart = pSelection->acpStart;
-  action->mSelectionLength = pSelection->acpEnd - pSelection->acpStart;
-  action->mSelectionReversed = (pSelection->style.ase == TS_AE_START);
-
+  action->mSelectionStart = selectionInContent.acpStart;
+  action->mSelectionLength =
+    selectionInContent.acpEnd - selectionInContent.acpStart;
+  action->mSelectionReversed = (selectionInContent.style.ase == TS_AE_START);
+
+  // Use TSF specified selection for updating mSelection.
   currentSel.SetSelection(*pSelection);
 
   return S_OK;
 }
 
 STDMETHODIMP
 TSFTextStore::SetSelection(ULONG ulCount,
                            const TS_SELECTION_ACP* pSelection)
--- a/widget/windows/TSFTextStore.h
+++ b/widget/windows/TSFTextStore.h
@@ -549,16 +549,26 @@ protected:
     }
 
     WritingMode GetWritingMode() const
     {
       MOZ_ASSERT(!mDirty);
       return mWritingMode;
     }
 
+    bool EqualsExceptDirection(const TS_SELECTION_ACP& aACP) const
+    {
+      if (mACP.style.ase == aACP.style.ase) {
+        return mACP.acpStart == aACP.acpStart &&
+               mACP.acpEnd == aACP.acpEnd;
+      }
+      return mACP.acpStart == aACP.acpEnd &&
+             mACP.acpEnd == aACP.acpStart;
+    }
+
   private:
     TS_SELECTION_ACP mACP;
     WritingMode mWritingMode;
     bool mDirty;
   };
   // Don't access mSelection directly except at calling MarkDirty().
   // Use CurrentSelection() instead.  This is marked as dirty when the
   // selection or content is changed without document lock.