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

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Makoto Kato 2019-04-12 03:54:18 +00:00
parent 959b87ac8f
commit e6c2e7bc87
7 changed files with 179 additions and 180 deletions

View File

@ -409,7 +409,7 @@ EditorSpellCheck::GetNextMisspelledWord(nsAString& aNextMisspelledWord) {
// ScrollSelectionIntoView.
RefPtr<mozSpellChecker> spellChecker(mSpellChecker);
return spellChecker->NextMisspelledWord(aNextMisspelledWord,
&mSuggestedWordList);
mSuggestedWordList);
}
NS_IMETHODIMP

View File

@ -323,15 +323,13 @@ nsresult TextServicesDocument::SetFilterType(uint32_t 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;
}
@ -1064,11 +1062,7 @@ nsresult TextServicesDocument::DeleteSelection() {
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;
}
@ -1097,12 +1091,12 @@ nsresult TextServicesDocument::InsertText(const nsString* aText) {
// 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];
@ -2479,7 +2473,7 @@ nsresult TextServicesDocument::GetFirstTextNodeInNextBlock(
nsresult TextServicesDocument::CreateOffsetTable(
nsTArray<OffsetEntry*>* aOffsetTable,
FilteredContentIterator* aFilteredIter, IteratorStatus* aIteratorStatus,
nsRange* aIterRange, nsString* aStr) {
nsRange* aIterRange, nsAString* aStr) {
nsCOMPtr<nsIContent> first;
nsCOMPtr<nsIContent> prev;

View File

@ -117,7 +117,7 @@ class TextServicesDocument final : public nsIEditActionListener {
*
* @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
@ -211,7 +211,7 @@ class TextServicesDocument final : public nsIEditActionListener {
* 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.
@ -289,7 +289,7 @@ class TextServicesDocument final : public nsIEditActionListener {
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,

View File

@ -31,71 +31,76 @@ bool mozEnglishWordUtils::ucIsAlpha(char16_t aChar) {
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++;
}
}
while ((p < endbuf) && (!ucIsAlpha(*p))) {
p++;
}
startWord = p;
while ((p < endbuf) && ((ucIsAlpha(*p)) || (*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.
// 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.
// 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.
// 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.
if (mURLDetector) {
int32_t startPos = -1;
int32_t endPos = -1;
if (mURLDetector) {
int32_t startPos = -1;
int32_t endPos = -1;
mURLDetector->FindURLInPlaintext(startWord, endbuf - startWord,
p - startWord, &startPos, &endPos);
mURLDetector->FindURLInPlaintext(startWord, endbuf - startWord,
p - startWord, &startPos, &endPos);
// 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
// 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(word, length, startWord - word, begin, end);
}
// 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;
}

View File

@ -21,11 +21,12 @@ class mozEnglishWordUtils final {
/**
* 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();

View File

@ -75,42 +75,39 @@ nsresult mozSpellChecker::SetDocument(
}
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;
}
@ -187,95 +184,97 @@ nsresult mozSpellChecker::CheckWord(const nsAString &aWord, bool *aIsMisspelled,
nsresult mozSpellChecker::Replace(const nsAString &aOldWord,
const nsAString &aNewWord,
bool aAllOccurrences) {
if (!mConverter) return NS_ERROR_NULL_POINTER;
if (NS_WARN_IF(!mConverter)) {
return NS_ERROR_NOT_INITIALIZED;
}
nsAutoString newWord(aNewWord); // sigh
if (!aAllOccurrences) {
MOZ_KnownLive(mTextServicesDocument)->InsertText(aNewWord);
return NS_OK;
}
if (aAllOccurrences) {
int32_t selOffset;
int32_t startBlock, currentBlock, currOffset;
int32_t begin, end;
bool done;
nsresult result;
int32_t selOffset;
int32_t startBlock;
int32_t begin, end;
bool done;
nsresult result;
// 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;
// find out where we are
result = SetupDoc(&selOffset);
if (NS_FAILED(result)) return result;
result = GetCurrentBlockIndex(mTextServicesDocument, &startBlock);
if (NS_FAILED(result)) return 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.
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;
}
// 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();
}
// 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) {
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);
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;
}
mTextServicesDocument->NextBlock();
currentBlock++;
currOffset = 0;
}
// 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();
}
// 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;
}
mTextServicesDocument->NextBlock();
mTextServicesDocument->GetCurrentTextBlock(str);
if (mConverter->FindNextWord(str, 0, &begin, &end)) {
MOZ_KnownLive(mTextServicesDocument)->SetSelection(begin, 0);
}
} else {
MOZ_KnownLive(mTextServicesDocument)->InsertText(&newWord);
}
return NS_OK;
}

View File

@ -53,7 +53,7 @@ class mozSpellChecker final {
*/
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.