Bug 1543585 - Clean up finding words for spellchecker. r=masayuki
authorMakoto Kato <m_kato@ga2.so-net.ne.jp>
Fri, 12 Apr 2019 03:54:18 +0000
changeset 469260 a34e5dcf0fe8277176fb505348fdf85a1f22e1fb
parent 469259 21ee37c5b888931751bc0a2429eb15ecdf74f57f
child 469261 e8717a523b34291683440914deda399199dc6247
push id112776
push usershindli@mozilla.com
push dateFri, 12 Apr 2019 16:20:17 +0000
treeherdermozilla-inbound@b4501ced5619 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmasayuki
bugs1543585
milestone68.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 1543585 - Clean up finding words for spellchecker. r=masayuki `mozSpellChecker::Replace` and `mozSpellChecker::NextMisspelledWord` have a loop to find word from text content. But `mozEnglishWordUtils::FindNextWord` always returns `NS_OK` and some code doesn't check error even if `nsresult` local variable is used. So I would like to clean up this loop. - `mozEnglishWordUtils::FindNextWord` should return true if word is found - We should use reference type for some `TextServicesDocument`'s methods. - Add more check for error Differential Revision: https://phabricator.services.mozilla.com/D27037
editor/spellchecker/EditorSpellCheck.cpp
editor/spellchecker/TextServicesDocument.cpp
editor/spellchecker/TextServicesDocument.h
extensions/spellcheck/src/mozEnglishWordUtils.cpp
extensions/spellcheck/src/mozEnglishWordUtils.h
extensions/spellcheck/src/mozSpellChecker.cpp
extensions/spellcheck/src/mozSpellChecker.h
--- a/editor/spellchecker/EditorSpellCheck.cpp
+++ b/editor/spellchecker/EditorSpellCheck.cpp
@@ -404,17 +404,17 @@ NS_IMETHODIMP
 EditorSpellCheck::GetNextMisspelledWord(nsAString& aNextMisspelledWord) {
   NS_ENSURE_TRUE(mSpellChecker, NS_ERROR_NOT_INITIALIZED);
 
   DeleteSuggestedWordList();
   // Beware! This may flush notifications via synchronous
   // ScrollSelectionIntoView.
   RefPtr<mozSpellChecker> spellChecker(mSpellChecker);
   return spellChecker->NextMisspelledWord(aNextMisspelledWord,
-                                          &mSuggestedWordList);
+                                          mSuggestedWordList);
 }
 
 NS_IMETHODIMP
 EditorSpellCheck::GetSuggestedWord(nsAString& aSuggestedWord) {
   // XXX This is buggy if mSuggestedWordList.Length() is over INT32_MAX.
   if (mSuggestedWordIndex < static_cast<int32_t>(mSuggestedWordList.Length())) {
     aSuggestedWord = mSuggestedWordList[mSuggestedWordIndex];
     mSuggestedWordIndex++;
--- a/editor/spellchecker/TextServicesDocument.cpp
+++ b/editor/spellchecker/TextServicesDocument.cpp
@@ -318,25 +318,23 @@ nsresult TextServicesDocument::ExpandRan
 }
 
 nsresult TextServicesDocument::SetFilterType(uint32_t aFilterType) {
   mTxtSvcFilterType = aFilterType;
 
   return NS_OK;
 }
 
-nsresult TextServicesDocument::GetCurrentTextBlock(nsString* aStr) {
-  NS_ENSURE_TRUE(aStr, NS_ERROR_NULL_POINTER);
-
-  aStr->Truncate();
+nsresult TextServicesDocument::GetCurrentTextBlock(nsAString& aStr) {
+  aStr.Truncate();
 
   NS_ENSURE_TRUE(mFilteredIter, NS_ERROR_FAILURE);
 
   nsresult rv = CreateOffsetTable(&mOffsetTable, mFilteredIter,
-                                  &mIteratorStatus, mExtent, aStr);
+                                  &mIteratorStatus, mExtent, &aStr);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
   return NS_OK;
 }
 
 nsresult TextServicesDocument::FirstBlock() {
   NS_ENSURE_TRUE(mFilteredIter, NS_ERROR_FAILURE);
@@ -1059,21 +1057,17 @@ nsresult TextServicesDocument::DeleteSel
   // printf("Sel: (%2d, %4d) (%2d, %4d)\n", mSelStartIndex,
   //        mSelStartOffset, mSelEndIndex, mSelEndOffset);
   // PrintOffsetTable();
   //**** KDEBUG ****
 
   return rv;
 }
 
-nsresult TextServicesDocument::InsertText(const nsString* aText) {
-  if (NS_WARN_IF(!aText)) {
-    return NS_ERROR_INVALID_ARG;
-  }
-
+nsresult TextServicesDocument::InsertText(const nsAString& aText) {
   if (NS_WARN_IF(!mTextEditor) || NS_WARN_IF(!SelectionIsValid())) {
     return NS_ERROR_FAILURE;
   }
 
   // If the selection is not collapsed, we need to save
   // off the selection offsets so we can restore the
   // selection and delete the selected content after we've
   // inserted the new text. This is necessary to try and
@@ -1092,22 +1086,22 @@ nsresult TextServicesDocument::InsertTex
 
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   // AutoTransactionBatchExternal grabs mTextEditor, so, we don't need to grab
   // the instance with local variable here.
   AutoTransactionBatchExternal treatAsOneTransaction(*mTextEditor);
 
-  nsresult rv = mTextEditor->InsertTextAsAction(*aText);
+  nsresult rv = mTextEditor->InsertTextAsAction(aText);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
-  int32_t strLength = aText->Length();
+  int32_t strLength = aText.Length();
 
   OffsetEntry* itEntry;
   OffsetEntry* entry = mOffsetTable[mSelStartIndex];
   void* node = entry->mNode;
 
   NS_ASSERTION((entry->mIsValid), "Invalid insertion point!");
 
   if (entry->mStrOffset == mSelStartOffset) {
@@ -2474,17 +2468,17 @@ nsresult TextServicesDocument::GetFirstT
 
   // Restore the iterator:
   return mFilteredIter->PositionAt(node);
 }
 
 nsresult TextServicesDocument::CreateOffsetTable(
     nsTArray<OffsetEntry*>* aOffsetTable,
     FilteredContentIterator* aFilteredIter, IteratorStatus* aIteratorStatus,
-    nsRange* aIterRange, nsString* aStr) {
+    nsRange* aIterRange, nsAString* aStr) {
   nsCOMPtr<nsIContent> first;
   nsCOMPtr<nsIContent> prev;
 
   NS_ENSURE_TRUE(aFilteredIter, NS_ERROR_NULL_POINTER);
 
   ClearOffsetTable(aOffsetTable);
 
   if (aStr) {
--- a/editor/spellchecker/TextServicesDocument.h
+++ b/editor/spellchecker/TextServicesDocument.h
@@ -112,17 +112,17 @@ class TextServicesDocument final : publi
    */
   nsresult SetFilterType(uint32_t aFilterType);
 
   /**
    * Returns the text in the current text block.
    *
    * @param aStr                [OUT] This will contain the text.
    */
-  nsresult GetCurrentTextBlock(nsString* aStr);
+  nsresult GetCurrentTextBlock(nsAString& aStr);
 
   /**
    * Tells the document to point to the first text block in the document.  This
    * method does not adjust the current cursor position or selection.
    */
   nsresult FirstBlock();
 
   enum class BlockSelectionStatus {
@@ -206,17 +206,17 @@ class TextServicesDocument final : publi
   MOZ_CAN_RUN_SCRIPT
   nsresult DeleteSelection();
 
   /**
    * Inserts the given text at the current cursor position.  If there is a
    * selection, it will be deleted before the text is inserted.
    */
   MOZ_CAN_RUN_SCRIPT
-  nsresult InsertText(const nsString* aText);
+  nsresult InsertText(const nsAString& aText);
 
   /**
    * nsIEditActionListener method implementations.
    */
   NS_DECL_NSIEDITACTIONLISTENER
 
   /**
    * Actual edit action listeners.  When you add new method here for listening
@@ -284,17 +284,17 @@ class TextServicesDocument final : publi
                                    int32_t* aSelOffset, int32_t* aSelLength);
 
   bool SelectionIsCollapsed();
   bool SelectionIsValid();
 
   static nsresult CreateOffsetTable(nsTArray<OffsetEntry*>* aOffsetTable,
                                     FilteredContentIterator* aFilteredIter,
                                     IteratorStatus* aIteratorStatus,
-                                    nsRange* aIterRange, nsString* aStr);
+                                    nsRange* aIterRange, nsAString* aStr);
   static nsresult ClearOffsetTable(nsTArray<OffsetEntry*>* aOffsetTable);
 
   static nsresult NodeHasOffsetEntry(nsTArray<OffsetEntry*>* aOffsetTable,
                                      nsINode* aNode, bool* aHasEntry,
                                      int32_t* aEntryIndex);
 
   nsresult RemoveInvalidOffsetEntries();
   nsresult SplitOffsetEntry(int32_t aTableIndex, int32_t aOffsetIntoEntry);
--- a/extensions/spellcheck/src/mozEnglishWordUtils.cpp
+++ b/extensions/spellcheck/src/mozEnglishWordUtils.cpp
@@ -26,76 +26,81 @@ mozEnglishWordUtils::~mozEnglishWordUtil
 // This needs vast improvement
 
 // static
 bool mozEnglishWordUtils::ucIsAlpha(char16_t aChar) {
   // XXX we have to fix callers to handle the full Unicode range
   return nsUGenCategory::kLetter == mozilla::unicode::GetGenCategory(aChar);
 }
 
-nsresult mozEnglishWordUtils::FindNextWord(const char16_t *word,
-                                           uint32_t length, uint32_t offset,
-                                           int32_t *begin, int32_t *end) {
+bool mozEnglishWordUtils::FindNextWord(const nsAString &aWord, uint32_t offset,
+                                       int32_t *begin, int32_t *end) {
+  if (offset >= aWord.Length()) {
+    *begin = -1;
+    *end = -1;
+    return false;
+  }
+
+  const char16_t *word = aWord.BeginReading();
+  uint32_t length = aWord.Length();
   const char16_t *p = word + offset;
   const char16_t *endbuf = word + length;
   const char16_t *startWord = p;
-  if (p < endbuf) {
-    // XXX These loops should be modified to handle non-BMP characters.
-    // if previous character is a word character, need to advance out of the
-    // word
-    if (offset > 0 && ucIsAlpha(*(p - 1))) {
-      while (p < endbuf && ucIsAlpha(*p)) p++;
-    }
-    while ((p < endbuf) && (!ucIsAlpha(*p))) {
-      p++;
-    }
-    startWord = p;
-    while ((p < endbuf) && ((ucIsAlpha(*p)) || (*p == '\''))) {
+
+  // XXX These loops should be modified to handle non-BMP characters.
+  // if previous character is a word character, need to advance out of the
+  // word
+  if (offset > 0 && ucIsAlpha(*(p - 1))) {
+    while (p < endbuf && ucIsAlpha(*p)) {
       p++;
     }
-
-    // we could be trying to break down a url, we don't want to break a url into
-    // parts, instead we want to find out if it really is a url and if so, skip
-    // it, advancing startWord to a point after the url.
+  }
+  while ((p < endbuf) && (!ucIsAlpha(*p))) {
+    p++;
+  }
+  startWord = p;
+  while ((p < endbuf) && ((ucIsAlpha(*p)) || (*p == '\''))) {
+    p++;
+  }
 
-    // before we spend more time looking to see if the word is a url, look for a
-    // url identifer and make sure that identifer isn't the last character in
-    // the word fragment.
-    if ((p < endbuf - 1) && (*p == ':' || *p == '@' || *p == '.')) {
-      // ok, we have a possible url...do more research to find out if we really
-      // have one and determine the length of the url so we can skip over it.
+  // we could be trying to break down a url, we don't want to break a url into
+  // parts, instead we want to find out if it really is a url and if so, skip
+  // it, advancing startWord to a point after the url.
 
-      if (mURLDetector) {
-        int32_t startPos = -1;
-        int32_t endPos = -1;
+  // before we spend more time looking to see if the word is a url, look for a
+  // url identifer and make sure that identifer isn't the last character in
+  // the word fragment.
+  if ((p < endbuf - 1) && (*p == ':' || *p == '@' || *p == '.')) {
+    // ok, we have a possible url...do more research to find out if we really
+    // have one and determine the length of the url so we can skip over it.
 
-        mURLDetector->FindURLInPlaintext(startWord, endbuf - startWord,
-                                         p - startWord, &startPos, &endPos);
+    if (mURLDetector) {
+      int32_t startPos = -1;
+      int32_t endPos = -1;
 
-        // ok, if we got a url, adjust the array bounds, skip the current url
-        // text and find the next word again
-        if (startPos != -1 && endPos != -1) {
-          startWord = p + endPos + 1;  // skip over the url
-          p = startWord;               // reset p
+      mURLDetector->FindURLInPlaintext(startWord, endbuf - startWord,
+                                       p - startWord, &startPos, &endPos);
 
-          // now recursively call FindNextWord to search for the next word now
-          // that we have skipped the url
-          return FindNextWord(word, length, startWord - word, begin, end);
-        }
+      // ok, if we got a url, adjust the array bounds, skip the current url
+      // text and find the next word again
+      if (startPos != -1 && endPos != -1) {
+        startWord = p + endPos + 1;  // skip over the url
+
+        // now recursively call FindNextWord to search for the next word now
+        // that we have skipped the url
+        return FindNextWord(aWord, startWord - word, begin, end);
       }
     }
+  }
 
-    while ((p > startWord) &&
-           (*(p - 1) == '\'')) {  // trim trailing apostrophes
-      p--;
-    }
-  } else {
-    startWord = endbuf;
+  while ((p > startWord) && (*(p - 1) == '\'')) {  // trim trailing apostrophes
+    p--;
   }
+
   if (startWord == endbuf) {
     *begin = -1;
     *end = -1;
-  } else {
-    *begin = startWord - word;
-    *end = p - word;
+    return false;
   }
-  return NS_OK;
+  *begin = startWord - word;
+  *end = p - word;
+  return true;
 }
--- a/extensions/spellcheck/src/mozEnglishWordUtils.h
+++ b/extensions/spellcheck/src/mozEnglishWordUtils.h
@@ -16,21 +16,22 @@ class mozEnglishWordUtils final {
  public:
   NS_INLINE_DECL_CYCLE_COLLECTING_NATIVE_REFCOUNTING(mozEnglishWordUtils)
   NS_DECL_CYCLE_COLLECTION_NATIVE_CLASS(mozEnglishWordUtils)
 
   mozEnglishWordUtils();
 
   /**
    * Given a unicode string and an offset, find the beginning and end of the
-   * next word. begin and end are -1 if there are no words remaining in the
-   * string. This should really be folded into the Line/WordBreaker.
+   * next word. Return false, begin and end are -1 if there are no words
+   * remaining in the string. This should really be folded into the
+   * Line/WordBreaker.
    */
-  nsresult FindNextWord(const char16_t* word, uint32_t length, uint32_t offset,
-                        int32_t* begin, int32_t* end);
+  bool FindNextWord(const nsAString& aWord, uint32_t offset, int32_t* begin,
+                    int32_t* end);
 
  protected:
   virtual ~mozEnglishWordUtils();
 
   static bool ucIsAlpha(char16_t aChar);
 
   nsCOMPtr<mozITXTToHTMLConv>
       mURLDetector;  // used to detect urls so the spell checker can skip them.
--- a/extensions/spellcheck/src/mozSpellChecker.cpp
+++ b/extensions/spellcheck/src/mozSpellChecker.cpp
@@ -70,52 +70,49 @@ TextServicesDocument *mozSpellChecker::G
 nsresult mozSpellChecker::SetDocument(
     TextServicesDocument *aTextServicesDocument, bool aFromStartofDoc) {
   mTextServicesDocument = aTextServicesDocument;
   mFromStart = aFromStartofDoc;
   return NS_OK;
 }
 
 nsresult mozSpellChecker::NextMisspelledWord(nsAString &aWord,
-                                             nsTArray<nsString> *aSuggestions) {
-  if (!aSuggestions || !mConverter) return NS_ERROR_NULL_POINTER;
+                                             nsTArray<nsString> &aSuggestions) {
+  if (NS_WARN_IF(!mConverter)) {
+    return NS_ERROR_NOT_INITIALIZED;
+  }
 
   int32_t selOffset;
-  int32_t begin, end;
   nsresult result;
   result = SetupDoc(&selOffset);
-  bool isMisspelled, done;
   if (NS_FAILED(result)) return result;
 
+  bool done;
   while (NS_SUCCEEDED(mTextServicesDocument->IsDone(&done)) && !done) {
-    nsString str;
-    result = mTextServicesDocument->GetCurrentTextBlock(&str);
-
-    if (NS_FAILED(result)) {
-      return result;
-    }
-    do {
-      result = mConverter->FindNextWord(str.get(), str.Length(), selOffset,
-                                        &begin, &end);
-      if (NS_SUCCEEDED(result) && begin != -1) {
-        const nsAString &currWord = Substring(str, begin, end - begin);
-        result = CheckWord(currWord, &isMisspelled, aSuggestions);
-        if (isMisspelled) {
-          aWord = currWord;
-          MOZ_KnownLive(mTextServicesDocument)
-              ->SetSelection(begin, end - begin);
-          // After ScrollSelectionIntoView(), the pending notifications might
-          // be flushed and PresShell/PresContext/Frames may be dead.
-          // See bug 418470.
-          mTextServicesDocument->ScrollSelectionIntoView();
-          return NS_OK;
-        }
+    int32_t begin, end;
+    nsAutoString str;
+    mTextServicesDocument->GetCurrentTextBlock(str);
+    while (mConverter->FindNextWord(str, selOffset, &begin, &end)) {
+      const nsDependentSubstring currWord(str, begin, end - begin);
+      bool isMisspelled;
+      result = CheckWord(currWord, &isMisspelled, &aSuggestions);
+      if (NS_WARN_IF(NS_FAILED(result))) {
+        return result;
+      }
+      if (isMisspelled) {
+        aWord = currWord;
+        MOZ_KnownLive(mTextServicesDocument)->SetSelection(begin, end - begin);
+        // After ScrollSelectionIntoView(), the pending notifications might
+        // be flushed and PresShell/PresContext/Frames may be dead.
+        // See bug 418470.
+        mTextServicesDocument->ScrollSelectionIntoView();
+        return NS_OK;
       }
       selOffset = end;
-    } while (end != -1);
+    }
     mTextServicesDocument->NextBlock();
     selOffset = 0;
   }
   return NS_OK;
 }
 
 RefPtr<CheckWordPromise> mozSpellChecker::CheckWords(
     const nsTArray<nsString> &aWords) {
@@ -182,105 +179,107 @@ nsresult mozSpellChecker::CheckWord(cons
     *aIsMisspelled = true;
   }
   return NS_OK;
 }
 
 nsresult mozSpellChecker::Replace(const nsAString &aOldWord,
                                   const nsAString &aNewWord,
                                   bool aAllOccurrences) {
-  if (!mConverter) return NS_ERROR_NULL_POINTER;
-
-  nsAutoString newWord(aNewWord);  // sigh
+  if (NS_WARN_IF(!mConverter)) {
+    return NS_ERROR_NOT_INITIALIZED;
+  }
 
-  if (aAllOccurrences) {
-    int32_t selOffset;
-    int32_t startBlock, currentBlock, currOffset;
-    int32_t begin, end;
-    bool done;
-    nsresult result;
-    nsAutoString str;
+  if (!aAllOccurrences) {
+    MOZ_KnownLive(mTextServicesDocument)->InsertText(aNewWord);
+    return NS_OK;
+  }
 
-    // find out where we are
-    result = SetupDoc(&selOffset);
-    if (NS_FAILED(result)) return result;
-    result = GetCurrentBlockIndex(mTextServicesDocument, &startBlock);
-    if (NS_FAILED(result)) return result;
+  int32_t selOffset;
+  int32_t startBlock;
+  int32_t begin, end;
+  bool done;
+  nsresult result;
 
-    // start at the beginning
-    result = mTextServicesDocument->FirstBlock();
-    currOffset = 0;
-    currentBlock = 0;
-    while (NS_SUCCEEDED(mTextServicesDocument->IsDone(&done)) && !done) {
-      result = mTextServicesDocument->GetCurrentTextBlock(&str);
-      do {
-        result = mConverter->FindNextWord(str.get(), str.Length(), currOffset,
-                                          &begin, &end);
-        if (NS_SUCCEEDED(result) && (begin != -1)) {
-          if (aOldWord.Equals(Substring(str, begin, end - begin))) {
-            // if we are before the current selection point but in the same
-            // block move the selection point forwards
-            if (currentBlock == startBlock && begin < selOffset) {
-              selOffset +=
-                  int32_t(aNewWord.Length()) - int32_t(aOldWord.Length());
-              if (selOffset < begin) {
-                selOffset = begin;
-              }
-            }
-            MOZ_KnownLive(mTextServicesDocument)
-                ->SetSelection(begin, end - begin);
-            MOZ_KnownLive(mTextServicesDocument)->InsertText(&newWord);
-            mTextServicesDocument->GetCurrentTextBlock(&str);
-            end += (aNewWord.Length() -
-                    aOldWord.Length());  // recursion was cute in GEB, not here.
+  // find out where we are
+  result = SetupDoc(&selOffset);
+  if (NS_WARN_IF(NS_FAILED(result))) {
+    return result;
+  }
+  result = GetCurrentBlockIndex(mTextServicesDocument, &startBlock);
+  if (NS_WARN_IF(NS_FAILED(result))) {
+    return result;
+  }
+
+  // start at the beginning
+  result = mTextServicesDocument->FirstBlock();
+  if (NS_WARN_IF(NS_FAILED(result))) {
+    return result;
+  }
+  int32_t currOffset = 0;
+  int32_t currentBlock = 0;
+  while (NS_SUCCEEDED(mTextServicesDocument->IsDone(&done)) && !done) {
+    nsAutoString str;
+    mTextServicesDocument->GetCurrentTextBlock(str);
+    while (mConverter->FindNextWord(str, currOffset, &begin, &end)) {
+      if (aOldWord.Equals(Substring(str, begin, end - begin))) {
+        // if we are before the current selection point but in the same
+        // block move the selection point forwards
+        if (currentBlock == startBlock && begin < selOffset) {
+          selOffset += int32_t(aNewWord.Length()) - int32_t(aOldWord.Length());
+          if (selOffset < begin) {
+            selOffset = begin;
           }
         }
-        currOffset = end;
-      } while (currOffset != -1);
-      mTextServicesDocument->NextBlock();
-      currentBlock++;
-      currOffset = 0;
+        MOZ_KnownLive(mTextServicesDocument)->SetSelection(begin, end - begin);
+        MOZ_KnownLive(mTextServicesDocument)->InsertText(aNewWord);
+        mTextServicesDocument->GetCurrentTextBlock(str);
+        end += (aNewWord.Length() -
+                aOldWord.Length());  // recursion was cute in GEB, not here.
+      }
+      currOffset = end;
     }
-
-    // We are done replacing.  Put the selection point back where we found  it
-    // (or equivalent);
-    result = mTextServicesDocument->FirstBlock();
-    currentBlock = 0;
-    while (NS_SUCCEEDED(mTextServicesDocument->IsDone(&done)) && !done &&
-           currentBlock < startBlock) {
-      mTextServicesDocument->NextBlock();
-    }
+    mTextServicesDocument->NextBlock();
+    currentBlock++;
+    currOffset = 0;
+  }
 
-    // After we have moved to the block where the first occurrence of replace
-    // was done, put the selection to the next word following it. In case there
-    // is no word following it i.e if it happens to be the last word in that
-    // block, then move to the next block and put the selection to the first
-    // word in that block, otherwise when the Setupdoc() is called, it queries
-    // the LastSelectedBlock() and the selection offset of the last occurrence
-    // of the replaced word is taken instead of the first occurrence and things
-    // get messed up as reported in the bug 244969
+  // We are done replacing.  Put the selection point back where we found  it
+  // (or equivalent);
+  result = mTextServicesDocument->FirstBlock();
+  if (NS_WARN_IF(NS_FAILED(result))) {
+    return result;
+  }
+  currentBlock = 0;
+  while (NS_SUCCEEDED(mTextServicesDocument->IsDone(&done)) && !done &&
+         currentBlock < startBlock) {
+    mTextServicesDocument->NextBlock();
+  }
 
-    if (NS_SUCCEEDED(mTextServicesDocument->IsDone(&done)) && !done) {
-      nsString str;
-      result = mTextServicesDocument->GetCurrentTextBlock(&str);
-      result = mConverter->FindNextWord(str.get(), str.Length(), selOffset,
-                                        &begin, &end);
-      if (end == -1) {
-        mTextServicesDocument->NextBlock();
-        selOffset = 0;
-        result = mTextServicesDocument->GetCurrentTextBlock(&str);
-        result = mConverter->FindNextWord(str.get(), str.Length(), selOffset,
-                                          &begin, &end);
-        MOZ_KnownLive(mTextServicesDocument)->SetSelection(begin, 0);
-      } else {
-        MOZ_KnownLive(mTextServicesDocument)->SetSelection(begin, 0);
-      }
+  // After we have moved to the block where the first occurrence of replace
+  // was done, put the selection to the next word following it. In case there
+  // is no word following it i.e if it happens to be the last word in that
+  // block, then move to the next block and put the selection to the first
+  // word in that block, otherwise when the Setupdoc() is called, it queries
+  // the LastSelectedBlock() and the selection offset of the last occurrence
+  // of the replaced word is taken instead of the first occurrence and things
+  // get messed up as reported in the bug 244969
+
+  if (NS_SUCCEEDED(mTextServicesDocument->IsDone(&done)) && !done) {
+    nsAutoString str;
+    mTextServicesDocument->GetCurrentTextBlock(str);
+    if (mConverter->FindNextWord(str, selOffset, &begin, &end)) {
+      MOZ_KnownLive(mTextServicesDocument)->SetSelection(begin, 0);
+      return NS_OK;
     }
-  } else {
-    MOZ_KnownLive(mTextServicesDocument)->InsertText(&newWord);
+    mTextServicesDocument->NextBlock();
+    mTextServicesDocument->GetCurrentTextBlock(str);
+    if (mConverter->FindNextWord(str, 0, &begin, &end)) {
+      MOZ_KnownLive(mTextServicesDocument)->SetSelection(begin, 0);
+    }
   }
   return NS_OK;
 }
 
 nsresult mozSpellChecker::IgnoreAll(const nsAString &aWord) {
   if (mPersonalDictionary) {
     mPersonalDictionary->IgnoreWord(aWord);
   }
--- a/extensions/spellcheck/src/mozSpellChecker.h
+++ b/extensions/spellcheck/src/mozSpellChecker.h
@@ -48,17 +48,17 @@ class mozSpellChecker final {
   /**
    * Selects (hilites) the next misspelled word in the document.
    * @param aWord will contain the misspelled word.
    * @param aSuggestions is an array of nsStrings, that represent the
    * suggested replacements for the misspelled word.
    */
   MOZ_CAN_RUN_SCRIPT
   nsresult NextMisspelledWord(nsAString& aWord,
-                              nsTArray<nsString>* aSuggestions);
+                              nsTArray<nsString>& aSuggestions);
 
   /**
    * Checks if a word is misspelled. No document is required to use this method.
    * @param aWord is the word to check.
    * @param aIsMisspelled will be set to true if the word is misspelled.
    * @param aSuggestions is an array of nsStrings which represent the
    * suggested replacements for the misspelled word. The array will be empty
    * in chrome process if there aren't any suggestions. If suggestions is