Bug 1347433 part.1 Separate TextRange offset and length adjustment to AdjustRange() r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 15 Mar 2017 18:51:32 +0900
changeset 347907 a77b4a61225ac5f799126fa92f658de3fc768421
parent 347906 75c776f77bf7e863794ed867796037b7c941dc24
child 347908 c27841632c8ad1ba886667f63c002dabbcbbb50b
push id31507
push usercbook@mozilla.com
push dateThu, 16 Mar 2017 14:35:12 +0000
treeherdermozilla-central@3ea221e3ebbb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1347433
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1347433 part.1 Separate TextRange offset and length adjustment to AdjustRange() r=m_kato First of all, replacing native line breakers with XP line breakers needs to adjust offset and length of each TextRange. Therefore, we cannot just duplicate the code into TextEventDispatcher::PendingComposition::Flush(). For creating a new method to replace the native line breakers in PendingComposition::mString, we should separate range adjustment code to a static method, AdjustRange(), because we cannot use for loop simply because we need to adjust both mClauses and mCaret. MozReview-Commit-ID: 5ycsN8EAs45
widget/TextEventDispatcher.cpp
widget/TextEventDispatcher.h
--- a/widget/TextEventDispatcher.cpp
+++ b/widget/TextEventDispatcher.cpp
@@ -675,47 +675,59 @@ TextEventDispatcher::PendingComposition:
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
     }
     return NS_OK;
   }
 
   // Adjust offsets in the ranges for XP linefeed character (only \n).
-  // XXX Following code is the safest approach.  However, it wastes performance.
-  //     For ensuring the clauses do not overlap each other, we should redesign
-  //     TextRange later.
   for (uint32_t i = 0; i < aRanges->Length(); ++i) {
     TextRange range = aRanges->ElementAt(i);
-    TextRange nativeRange = range;
-    if (nativeRange.mStartOffset > 0) {
-      nsAutoString preText(Substring(aString, 0, nativeRange.mStartOffset));
-      preText.ReplaceSubstring(NS_LITERAL_STRING("\r\n"),
-                               NS_LITERAL_STRING("\n"));
-      range.mStartOffset = preText.Length();
-    }
-    if (nativeRange.Length() == 0) {
-      range.mEndOffset = range.mStartOffset;
-    } else {
-      nsAutoString clause(
-        Substring(aString, nativeRange.mStartOffset, nativeRange.Length()));
-      clause.ReplaceSubstring(NS_LITERAL_STRING("\r\n"),
-                              NS_LITERAL_STRING("\n"));
-      range.mEndOffset = range.mStartOffset + clause.Length();
-    }
+    AdjustRange(range, aString);
     if (range.mRangeType == TextRangeType::eCaret) {
       mCaret = range;
     } else {
       EnsureClauseArray();
       mClauses->AppendElement(range);
     }
   }
   return NS_OK;
 }
 
+// static
+void
+TextEventDispatcher::PendingComposition::AdjustRange(
+                                           TextRange& aRange,
+                                           const nsAString& aNativeString)
+{
+  TextRange nativeRange = aRange;
+  // XXX Following code wastes runtime cost because this causes computing
+  //     mStartOffset for each clause from the start of composition string.
+  //     If we'd make TextRange have only its length, we don't need to do
+  //     this.  However, this must not be so serious problem because
+  //     composition string is usually short and separated as a few clauses.
+  if (nativeRange.mStartOffset > 0) {
+    nsAutoString preText(
+      Substring(aNativeString, 0, nativeRange.mStartOffset));
+    preText.ReplaceSubstring(NS_LITERAL_STRING("\r\n"),
+                             NS_LITERAL_STRING("\n"));
+    aRange.mStartOffset = preText.Length();
+  }
+  if (nativeRange.Length() == 0) {
+    aRange.mEndOffset = aRange.mStartOffset;
+  } else {
+    nsAutoString clause(
+      Substring(aNativeString, nativeRange.mStartOffset, nativeRange.Length()));
+    clause.ReplaceSubstring(NS_LITERAL_STRING("\r\n"),
+                            NS_LITERAL_STRING("\n"));
+    aRange.mEndOffset = aRange.mStartOffset + clause.Length();
+  }
+}
+
 nsresult
 TextEventDispatcher::PendingComposition::Flush(
                                            TextEventDispatcher* aDispatcher,
                                            nsEventStatus& aStatus,
                                            const WidgetEventTime* aEventTime)
 {
   aStatus = nsEventStatus_eIgnore;
 
--- a/widget/TextEventDispatcher.h
+++ b/widget/TextEventDispatcher.h
@@ -327,16 +327,26 @@ private:
     void Clear();
 
   private:
     nsString mString;
     RefPtr<TextRangeArray> mClauses;
     TextRange mCaret;
 
     void EnsureClauseArray();
+
+    /**
+     * AdjustRange() adjusts aRange as in the string with XP line breakers.
+     *
+     * @param aRange            The reference to a range in aNativeString.
+     *                          This will be modified.
+     * @param aNativeString     The string with native line breakers.
+     *                          This may include "\r\n" and/or "\r".
+     */
+    static void AdjustRange(TextRange& aRange, const nsAString& aNativeString);
   };
   PendingComposition mPendingComposition;
 
   // While dispatching an event, this is incremented.
   uint16_t mDispatchingEvent;
 
   enum InputTransactionType : uint8_t
   {