244969 - Spell checker fails to catch misspelled words after a "Replace all"

Status: RESOLVED FIXED

(Core :: Spelling checker, defect)

Description

[Ken Roller]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6) Gecko/20040113 MITLL
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6) Gecko/20040113 MITLL

This text describes the bug and also serves as an example to
illustrate the problem. 

Place this text in a new mail message and run the spell checker on it.

This line contains the mispelled word that will trigger the bug.  When 
the spell checker detects this word, and suggests the correct version,
click on replace all.  The spell checker will then continue with the 
line marked continue rather than catching the errors in the next lines.
More specifically, it will continue with the word in that line following
the second instance of the incorrectly spelled word.

Put sopme vry badly speled stuff here in the middel of the text that 
should be caught by the spell chedker, but is not.

Continue, this line contains the same mispelled word as above.

Then put at least one additional word after the second instance of
the misspelled word to show that the spell checker has jumped to
it.

This line contains another msispelled word.



Reproducible: Always
Steps to Reproduce:
1.See details section.  Cut and paste into a new mail message and spell check.
2.
3.

Actual Results:  
See the details section.  The misspelled words should have been caught by the 
spell checker.

Expected Results:  
Continued the spell check from the first instance of the incorrectly spelled 
word rather than from the last instance of the incorrectly spelled word.

Comment 1

[R.K.Aa.]

*** Bug 250571 has been marked as a duplicate of this bug. ***

Comment 2

[Mike Cowperthwaite]

Ken Roller, I am unable to reproduce this symptom with TB 0.8.  Are you still 
seeing it?

Comment 3

[Ken Roller]

I don't have ThunderBird on any of my systems yet, but I am still seeing it in
Mozilla 1.7.3.

Comment 4

[Wayne Mery (:wsmwk)]

fails in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050316
so CONFIRMING

and, as Ken says above, this works in TB. my version (20050410)

Comment 5

[KIRAN SHIVARAMA SUNDAR]

Here are the steps for recreating the above reported problem. 

1) Enter the following 2 lines of text given:

problem has this intereskting
fsdhfjsdhjkjkh problem sah intereskting the wehere the

2) Run the spell check utility.

3) Choose 'Replace all' for the suggested mispelling i.e "intereskting" as
"interesting".

Actual Result: After Replacing all occurances for "intereskting" to
"interesting", the spell checker moves on to highlight the next misspelled word
as "wehere".

Expected result: The spell checker instead should have moved on to the next
mispelled word "fsdhfjsdhjkjkh" instead as it is the one following after the
first occurance of the misspelled word "intereskting"

Comment 6

[KIRAN SHIVARAMA SUNDAR]

Also, the above problem is seen only when the mispelling happens to be the last
word in the line or in mozilla jargon "Block" :-)

Comment 7

[KIRAN SHIVARAMA SUNDAR]

The below fix seems to solve the problem for me. Want somebody also to try the same.

File: seamonkey/source/extensions/spellcheck/src/mozSpellChecker.cpp

152 NS_IMETHODIMP 
153 mozSpellChecker::Replace(const nsAString &aOldWord, const nsAString  
    &aNewWord, PRBool aAllOccurrences)
154 {
155   if(!mConverter)
156     return NS_ERROR_NULL_POINTER;

206     // We are done replacing.  Put the selection point back where we found  
        //it (or equivalent);
207     result = mTsDoc->FirstBlock();
208     currentBlock = 0;
209     while(( NS_SUCCEEDED(mTsDoc->IsDone(&done)) && !done ) &&(currentBlock <
startBlock)){
210       mTsDoc->NextBlock();
211     }

/* Comment out or remove the below block of code */

/* The below code puts the selection to the next word following the first
occurance of the misspelled word(i.e first misspelled word in the 'Replace all') 
so that the spellcheck can start of from that point onwards and selOffset is
populated with the offset of this location after doing a call to SetupDoc() */

212   if( NS_SUCCEEDED(mTsDoc->IsDone(&done)) && !done ){
213        nsString str;                   // regenerate the offset table
214      result = mTsDoc->GetCurrentTextBlock(&str);  // this seems necessary.   
                                                      //I'm not 100% sure why
215      mTsDoc->SetSelection(selOffset,0);
216    }


/* The problem with the above code is, when the NextMisspelledWord() function
queries for the selOffset in the setupdoc() and if first occurance of the
replaced word happens to be the last word in the block then, the above code
fails to update the LastSelectedBlock() back and hence results in the above
reported bug */

// Replace the below code with the code above */

/* The Logic goes like this:
If the mispelled word happens to be the last word in the block, then move to the
next block and update the selection to point to the first word in the next block 
else put the selection back to position next to the first occurance of the
misspelled word */


212    if( NS_SUCCEEDED(mTsDoc->IsDone(&done)) && !done ){
         nsString str;
      result = mTsDoc->GetCurrentTextBlock(&str);
      result =
mConverter->FindNextWord(str.get(),str.Length(),selOffset,&begin,&end);
            if(end == -1)
             {
                mTsDoc->NextBlock();
                selOffset=0;
                result = mTsDoc->GetCurrentTextBlock(&str);
                result =
mConverter->FindNextWord(str.get(),str.Length(),selOffset,&begin,&end);
                mTsDoc->SetSelection(begin, 0);
             }
         else
                mTsDoc->SetSelection(begin, 0);
    } 

Doing the above change solves the problem. Request someone tries this fix too.
Also, I would like to know whether this code change causes any regressions or
not. As far as I have verified, So far I havent seen any regression with this fix.

Comment 8

[timeless]

[e-mail]: would you please attach your changes as a cvs diff?

Comment 9

[KIRAN SHIVARAMA SUNDAR]

Attachment: Patch Fixing the Problem

I have uploaded the patch which is fixing the problem reported. Kindly review
it.

Comment 10

[Mike Cowperthwaite]

Comment on attachment 180680 [details] [diff] [review]
Patch Fixing the Problem

(In reply to comment #9)
> Created an attachment (id=180680) [edit]
> Patch Fixing the Problem
> 
> I have uploaded the patch which is fixing the problem reported. Kindly review
> it.

To request a review, you need to set a flag on the attachment and specify a
reviewer.  mscott is the "default owner" of the Spelling Checker component.

Comment 11

[Scott MacGregor]

Comment on attachment 180680 [details] [diff] [review]
Patch Fixing the Problem

I've been testing with this patch for a while now haven't seen any regressions
yet.

Thanks for the detailed test case and the fix!

Comment 12

[Asa Dotzler [:asa]]

Comment on attachment 180680 [details] [diff] [review]
Patch Fixing the Problem

a=asa

Comment 13

[Scott MacGregor]

fixed

Comment 14

[Stephen Donner [:stephend] Not actively reading bugmail]

BTW, it's spelled 'occurrence'.

Comment 15

[[e-mail]]

It wouldn't be a true spellchecker patch without a misspelling ;-)