Last Comment Bug 342511 - correcting another word (with spell checker) doesn't cause current word to be spell-checked
: correcting another word (with spell checker) doesn't cause current word to be...
Status: RESOLVED FIXED
: verified1.8.1.12
Product: Core
Classification: Components
Component: Spelling checker (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla1.8.1beta2
Assigned To: Martijn Wargers [:mwargers] (not working for Mozilla)
:
: Jet Villegas (:jet)
Mentors:
one twoo
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-06-23 02:59 PDT by tommyjb
Modified: 2008-01-30 17:58 PST (History)
8 users (show)
dveditz: blocking1.8.1.12+
dveditz: wanted1.8.1.x+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (1.74 KB, patch)
2007-09-12 04:24 PDT, Martijn Wargers [:mwargers] (not working for Mozilla)
brettw: review+
dveditz: approval1.8.1.12+
dsicore: approval1.9+
Details | Diff | Splinter Review

Description tommyjb 2006-06-23 02:59:29 PDT
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) 2006-06-23 03:58:18 PDT
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) 2006-06-23 04:11:42 PDT
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 2006-06-23 04:16:32 PDT
Just to be clear, there should be no space after the "twoo": "wone twoo".
Comment 4 Martijn Wargers [:mwargers] (not working for Mozilla) 2006-06-23 06:36:32 PDT
This is a regression from bug 312778, backing out the "remove the redundant DeleteSelection call" patch fixes it.
Comment 5 Martijn Wargers [:mwargers] (not working for Mozilla) 2006-06-23 06:46:37 PDT
I have no idea why this would make a difference, though.
Comment 6 Martijn Wargers [:mwargers] (not working for Mozilla) 2006-06-23 07:52:15 PDT
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 2006-06-23 08:18:17 PDT
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 [:mwargers] (not working for Mozilla) 2006-06-23 09:04:26 PDT
Well, that other patch was about code that was supposed to be unnecessary. It now seems it is not.
Comment 9 tommyjb 2006-06-23 09:13:22 PDT
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) 2006-06-23 09:25:19 PDT
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 [:mwargers] (not working for Mozilla) 2006-06-23 09:36:01 PDT
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) 2006-06-23 09:47:17 PDT
I tested it on the branch exactly as described here (did not copy & paste).
Comment 13 Ria Klaassen (not reading all bugmail) 2006-06-23 10:10:04 PDT
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 [:mwargers] (not working for Mozilla) 2006-06-23 10:26:52 PDT
(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 2006-06-23 10:31:08 PDT
> 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) 2006-06-23 10:40:58 PDT
(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 2006-06-23 10:56:47 PDT
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 2006-06-23 23:18:32 PDT
(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 2006-06-23 23:33:03 PDT
(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 2006-06-23 23:35:20 PDT
…assuming the user sent immediately after typing, anyway.

Excuse the bug spam; I'm running on too little sleep.  :)
Comment 21 Brett Wilson 2006-07-28 15:19:46 PDT
Not going to get fixed for 2.0
Comment 22 Martijn Wargers [:mwargers] (not working for Mozilla) 2007-09-12 04:24:48 PDT
Created attachment 280587 [details] [diff] [review]
patch

This fixes it by always checking the current word when the mouseclick is not a left mouse button.
Comment 23 Martijn Wargers [:mwargers] (not working for Mozilla) 2007-09-12 04:25:33 PDT
Also happening with current trunk.
Comment 24 Martijn Wargers [:mwargers] (not working for Mozilla) 2007-09-20 09:03:31 PDT
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 2007-10-05 16:28:48 PDT
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] 2007-12-19 11:55:08 PST
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] 2008-01-08 10:49:28 PST
Martijn: does this patch apply to the branch?
Comment 28 Martijn Wargers [:mwargers] (not working for Mozilla) 2008-01-08 13:16:53 PST
Yes, sure, with some fuzz, it applies. I'm just wary about fixing non-critical bugs on branch.
Comment 29 Daniel Veditz [:dveditz] 2008-01-24 13:50:02 PST
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 [:mwargers] (not working for Mozilla) 2008-01-26 15:16:43 PST
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 [:mwargers] (not working for Mozilla) 2008-01-26 15:55:46 PST
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] 2008-01-30 17:47:29 PST
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 [:mwargers] (not working for Mozilla) 2008-01-30 17:58:42 PST
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.

Note You need to log in before you can comment on or make changes to this bug.