342511 - correcting another word (with spell checker) doesn't cause current word to be spell-checked

Status: RESOLVED FIXED

(Core :: Spelling checker, defect)

Description

[tommyjb]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1a3) Gecko/20060622 BonEcho/2.0a3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1a3) Gecko/20060622 BonEcho/2.0a3

1. Type the following in a textarea: wone twoo
2. Right-click "wone" and select "one".

After this, the insertion point moves to the right of "one".  Given that the insertion point is now away from "twoo", "twoo" should be underlined as incorrect, but it isn't.

Reproducible: Always

Comment 1

[Ria Klaassen (not reading all bugmail)]

All underlines disappear after correcting the first word with one of the examples in the context-menu. But when you click after the next word all dotted underlines re-appear.

Comment 2

[Ria Klaassen (not reading all bugmail)]

Regression between 1.9a1_2006062014 and 1.9a1_2006062022:
http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=2006-06-20+13%3A00&maxdate=2006-06-20+23%3A00

I don't see immediately a culprit or it might be Bug 312930.

Comment 3

[tommyjb]

Just to be clear, there should be no space after the "twoo": "wone twoo".

Comment 4

[Martijn Wargers (dead)]

This is a regression from bug 312778, backing out the "remove the redundant DeleteSelection call" patch fixes it.

Comment 5

[Martijn Wargers (dead)]

I have no idea why this would make a difference, though.

Comment 6

[Martijn Wargers (dead)]

This is odd, the "remove the redundant DeleteSelection call" patch wasn't checked in on the 1.8.1 branch, so this shouldn't be a problem on the 1.8.1 branch, then.
Reporter, you're sure you're seeing this on the 1.8.1 branch, right?

Comment 7

[Brett Wilson]

The reporter used the trunk, but I though the code was the same. We should get that other patch on the branch, though, right? So it will be a problem?

Comment 8

[Martijn Wargers (dead)]

Well, that other patch was about code that was supposed to be unnecessary. It now seems it is not.

Comment 9

[tommyjb]

I'm the reporter.  I'm seeing this on the Firefox branch nightly (I'm not entirely sure which core version it amounts to):

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1a3) Gecko/20060622 BonEcho/2.0a3

Comment 10

[Ria Klaassen (not reading all bugmail)]

WFM with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1a3) Gecko/20060623 BonEcho/2.0a3 so that's pretty weird. 

Comment 11

[Martijn Wargers (dead)]

Argh! We have a mix-up of two bugs here.
 
I think I now know what Tommy means.
- Spellchecking already needs to be enabled on the text control.
- Type (do not copy paste) in a text control "wone twoo"
- Right click on "wone" and choose "one" as spelling suggestion.
The word has been replaced, but no underline has appeared under "twoo", even though the caret has moved out of the word.

I filed bug 342536 for the trunk regression. This bug should remain about what Tommy has reported.

Comment 12

[Ria Klaassen (not reading all bugmail)]

I tested it on the branch exactly as described here (did not copy & paste).

Comment 13

[Ria Klaassen (not reading all bugmail)]

OK, I see. Personally I would expect that if you don't type a space or a dot (or comma) after a word but put your cursor somewhere else temporarily, the checker would not consider the word as finished and thus not mark it as wrong.

Comment 14

[Martijn Wargers (dead)]

(In reply to comment #13)
> OK, I see. Personally I would expect that if you don't type a space or a dot
> (or comma) after a word but put your cursor somewhere else temporarily, the
> checker would not consider the word as finished and thus not mark it as wrong.

Hmm, I think you might be right. That would mean this bug is invalid. I see some similarities with bug 339478 here.

Comment 15

[Brett Wilson]

> OK, I see. Personally I would expect that if you don't type a space or a dot
> (or comma) after a word but put your cursor somewhere else temporarily, the
> checker would not consider the word as finished and thus not mark it as wrong.

This isn't possible. What about the last word in the textbox? Many times, people don't end imput with periods.

Comment 16

[Ria Klaassen (not reading all bugmail)]

(In reply to comment #15)
>
Yeah, but on the other hand it would provide a lot of irritation when that red is served to you when you know you were only not yet finished with the last word.

Comment 17

[Brett Wilson]

Can you honestly say you think this is the right behavior for most users? What is more annoying, having a misspelled word in the document that you're not done typing marked as misspelled, or having it not? The answers seem clear to me.

Comment 18

[tommyjb]

(In reply to comment #16)
> Yeah, but on the other hand it would provide a lot of irritation when that red
> is served to you when you know you were only not yet finished with the last
> word.

Not really.  Simply completing the word resolves it.

IMO, requiring punctuation would do little more than to cause inconvenience and confusion.  "Why isn't it spell-checking that word?  Is it turned off?  Oh, wait, I need to place a period after it."  That's assuming that the user even considers that the word might be wrong.

Imagine sending the following to BugZilla: "that bug is seperate".  According to your proposal, "seperate" wouldn't even get flagged.

Note that Word spell-checks words when there's no punctuation at the end.

(In reply to comment #14)
> Hmm, I think you might be right. That would mean this bug is invalid. I see
> some similarities with bug 339478 here.

That bug WFM on "Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1a3) Gecko/20060622 BonEcho/2.0a3".

Comment 19

[tommyjb]

(In reply to comment #18)
> Imagine sending the following to BugZilla: "that bug is seperate".  According
> to your proposal, "seperate" wouldn't even get flagged.

Sorry, that's irrelevant.  It wouldn't flag it in either case.

Comment 20

[tommyjb]

…assuming the user sent immediately after typing, anyway.

Excuse the bug spam; I'm running on too little sleep.  :)

Comment 21

[Brett Wilson]

Not going to get fixed for 2.0

Comment 22

[Martijn Wargers (dead)]

Attachment: patch

This fixes it by always checking the current word when the mouseclick is not a left mouse button.

Comment 23

[Martijn Wargers (dead)]

Also happening with current trunk.

Comment 24

[Martijn Wargers (dead)]

Checking in mozInlineSpellChecker.cpp;
/cvsroot/mozilla/extensions/spellcheck/src/mozInlineSpellChecker.cpp,v  <--  moz
InlineSpellChecker.cpp
new revision: 1.41; previous revision: 1.40
done

Checked into trunk.

Comment 25

[Scott MacGregor]

Martijn, in addition to Bug 345428, this bug also seems like a pretty useful fix for the spell checker for Fx and Tb, should we consider it for the branch? 

Comment 26

[Daniel Veditz [:dveditz]]

Can't see this as "blocking", but going to leave the nomination request as a reminder that we do want it. Please add an approval request to a branch-appropriate patch.

Comment 27

[Daniel Veditz [:dveditz]]

Martijn: does this patch apply to the branch?

Comment 28

[Martijn Wargers (dead)]

Yes, sure, with some fuzz, it applies. I'm just wary about fixing non-critical bugs on branch.

Comment 29

[Daniel Veditz [:dveditz]]

Comment on attachment 280587 [details] [diff] [review]
patch

approved for 1.8.1.12, a=dveditz for release-drivers

code-freeze Jan 25 EOD

Comment 30

[Martijn Wargers (dead)]

It seems as if someone already checked it in on the 1.8 tree, but I can't see the bonsai log, so I'm a little confused now.
But if it isn't checked in yet, feel free to check it in (although I'm still kinda wary about checking in this kind of trivial bug on a 1.8 tree).

Comment 31

[Martijn Wargers (dead)]

Checking in mozInlineSpellChecker.cpp;
/cvsroot/mozilla/extensions/spellcheck/src/mozInlineSpellChecker.cpp,v  <--  moz
InlineSpellChecker.cpp
new revision: 1.6.4.21; previous revision: 1.6.4.20
done

Checked in on the 1.8 branch.

Comment 32

[juan becerra [:juanb]]

I was unable to reproduce the problem on 2.0.0.7 (Vista), nor 2.0.0.10 (Mac), nor 2.0.0.12 (Mac), nor trunk on Mac.  

Comment 33

[Martijn Wargers (dead)]

Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.12pre) Gecko/20080130 BonEcho/2.0.0.12pre

I was able to reproduce the problem with a 1.8 branch build of 20080123.