Closed Bug 429118 Opened 17 years ago Closed 16 years ago

Spellcheck gives up after too few misspelled words in HTML editor

Categories

(Thunderbird :: Message Compose Window, defect, P1)

Product:
Thunderbird
Component:
Message Compose Window
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 3.0b1

People

(Reporter: scanorama346, Unassigned)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

Patch, version 0.9
2.86 KB, patch
Details | Diff | Splinter Review
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008041406 Minefield/3.0pre Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008041403 Thunderbird/3.0a1pre ID:2008041403 When composing messages in Thunderbird the inline spell checker only checks the first word of the message but ignores the rest. So for example, if my message is: wnorg worng gnorw Spell checker should highlight all 3 words but instead only the first word is highlighted. Reproducible: Always Steps to Reproduce: 1.Open Thunderbird 2.Create a new message 3.Enter message content 4.Let the inline spell checker does its job Actual Results: Only the first word is highlighted (if it is spelt wrong) Expected Results: All spelling errors should be highlighted.
Version: unspecified → Trunk
WFM on linux.
What build are you on? It just stopped working about two weeks ago for me. The 2008041403 build is still broken. FF 3.0pre nightly also had a broken inline spell check but that got fixed last week sometime. Not sure if there is any shared code that may affect both projects.
Tested on version 3.0a1pre (2008041503).
Ok I just booted up a clean XP install in a virtual machine and then installed the same build and the inline spell check worked fine. So then I went back to my Vista box and created a new clean profile in TB and added one email account and tested the inline spell check and it worked. So something is wrong with my profile I guess. I just created this profile not to long ago. I am going to create a clean profile and slowly make all the changes I normally make until I find the one that breaks the spell check. I will report back what I find.
version 3.0a1pre (2008042203) Vista This was broken for me also, I have just removed the funambol add-on and restarted and it seems to work again now.
Spoke to soon, had nothing to do with the add-on, spell check seems to work briefly after a restart on the first compose window (doesn't work at all if I reply) and not just on the first word but after a paragraph or some random time/length/something just stops working..
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking-thunderbird3.0a1?
Keywords: regression
This probably isn't so sever
Keywords: qawanted
This isn't so severe that it would block an alpha. It might well block Thunderbird3 depending on how common the problem is. My initial test with tonight's nightly doesn't immediately hit this on Mac. Added the qawanted keyword in the hopes that one or more folks will figure out precisely how to reproduce it, as well as do a binary search to figure out exactly when the regression happened and what checkins are in that range. Correlating with whatever Firefox bugs are relevant here is also likely to be helpful.
Flags: blocking-thunderbird3.0a1? → blocking-thunderbird3.0a1-
The inline spell check in FF broke about the same time TB broke. But they broke a little different. In FF you could misspell a word and it wouldn't underline it but if you right clicked the misspelled word it would give you suggestions. It was doing the spell check but not underlining the bad words. TB will underline the first bad word but none of the others. If you right click on one of the other bad words you don't even get suggestions like you would in FF. Not sure if this makes sense or is at all helpful in debugging.
Confirming in today's debug Mac 10.5 Trunk build, type a whole phrase (can be quite long) of misspelled words and only the first few will be underlined. Nominating blocking-thunderbird3? to keep it on the radar.
Flags: blocking-thunderbird3?
Keywords: qawanted
Nominating wanted-thunderbird3a2? Still broken as at Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008050203 Thunderbird/3.0a1pre ID:2008050203
Flags: wanted-thunderbird3.0a2?
This should block Thunderbird 3; getting it sooner would be nice, though.
Flags: wanted-thunderbird3.0a2?
Flags: wanted-thunderbird3.0a2+
Flags: blocking-thunderbird3?
Flags: blocking-thunderbird3+
Confirming broken in Shredder Alpha 1 version 3.0a1 (2008050714). (PPC 10.4 Mac)
So I've just looked at reproducing this. On Shredder Alpha 1 version 3.0a1 (2008050714) running on intel mac. What I've found is that inline spell check stops working after the first correctly spelled word.
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008051603 Thunderbird/3.0a2pre ID:2008051603 Lack of inline spell check has been plaguing my nightly installs for a few weeks now. I had reported it as bug 430489. It started working again, I marked it "worksforme" - then promptly broke once more.
Inline spell checker seems to be working when editing a sent message. Step to reproduce: Right click on any sent message in the sent folder Select 'edit as new' Start typing in the middle of the message and purposely spell some words wrong Spell checker picks up all the wrong words However it still does not work when composing a new message. Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008051903 Thunderbird/3.0a2pre ID:2008051903
(In reply to comment #17) > Inline spell checker seems to be working when editing a sent message. Strange, I just tried your steps and mine isn't even checking on messages that I "Edit as New". I am running version 3.0a1pre (2008050703)
Priority: -- → P1
Also broken for me. I am running version 3.0a1 (2008050715) This seems *consistently* reproducable: ======================================= 1. Close TB 2. Open TB 3. Click 'Write' and start composing. Type a garbage word. It is properly underlined in red. 4. Close that email window, clicking "Don't Save" 5. Immediately click 'Write' again and start typing garbage. Now the in-line spell checker no longer functions.
I can't reproduce on linux. Do anyone still see this on latest trunk - http://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/latest-trunk/
Upgraded to version 3.0a2pre (2008063003) Following my previous steps in Comment #19, which seemed to consistently cause the bug to appear, I no longer reproduce this problem. Seems to be fixed? Can others confirm? But my question is, how would this be fixed with no one knowing about it?
I just downloaded the latest build and still have the bug.
Spell check seems to WFM in the latest trunk build, can others confirm? Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.0.2pre) Gecko/2008071403 Thunderbird/3.0a2pre ID:2008071403
(In reply to comment #24) > Spell check seems to WFM in the latest trunk build, can others confirm? I have seen it working or only intermittently working throughout a message recently. I probably need to send some more emails, I'm not 100% convinced it is fully fixed though.
Still not working. Example. Start new email, leave the recipients blank and the subject blank then in the body type: wdiwgd iduewgdwieug idugwqediuwg Only the first word is underlined in red. Hit F7 and it shows that all three words are misspelled. I will create a new profile and see if that helps. This is a TON of work though. BTW, I am using: version 3.0a2pre (2008071403)
Start a new email, leave the recipients blank and the subject blank and then in the body I typed: jmklj lkj lkj lkj;jkl ;ljk ;lkj ;lkj ;lkj ;lkj ;lkj ;lkj ;lkj ;lkj ;lkj ;lkj ;lkj ;lkj ;lkj ;lkj k;lj Spell checker only picked up the first 14 words, the rest weren't highlighted. I did a similar test 2 days ago and the first 11 words were highlighted. Putting a correctly spelt word such as 'Melbourne' at the beginning does not affect the results. I agree this is not 100% fixed. Though it has improved since this bug was reported. List of extensions I have installed: English (Australian) Dictionary 2.1.1 Nightly Tester Tools 2.0.2 Update Channel Selector 1.0.3 Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.0.2pre) Gecko/2008071503 Thunderbird/3.0a2pre ID:2008071503
I just created a new profile and only configured one email account. I didn't change any other settings. No plugins. Just a bare bones profile with one account. Then I created a new message, left To and Subject blank and in the body typed: weiuweg fg weweug weoiugwe doweugd weodigwe odigwe douegd owedugw eodiugw eodiugw eodiuw geod uwgeodwu egodiwue gdow goig oig oiug oig It only marked the first 13 words. But it isn't consistent. I can close the message and start a new one and it will mark even less words. Interesting though, I can type as many misspelled words as I want in the subject field and it will mark ever single one of them every time. I was happy to be able to create a bare bones profile and still recreate the bug.
Target Milestone: --- → Thunderbird 3.0b1
I downloaded the source code because I wanted to help fix this and made a lot of progress. However, I don't understand virtually any of the design of thunderbird so I need someone else to put the change in. I think I *mostly* fixed this bug by removing the "!" from an if statement, but there are still other bugs. File: extensions/spellcheck/src/mozInlineSpellChecker.cpp The problem centered on the program not keeping the count of "mNumWordsInSpellSelection" correctly. This variable tracks the number of misspelled words in the current email you are typing. There is a maximum number of misspellings in "mMaxNumWordsInSpellSelection", and this number was being reached very quickly. Every keystroke you enter in a misspelled word was counting towards another misspelled word and the mNumWordsInSpellSelection variable wasn't being decremented. If you put debug statements everywhere mNumWordsInSpellSelection is changed, you will observe this behavior. In the function mozInlineSpellChecker::DoSpellCheck() there is some code that is meant to remove any spellcheck range that already contains the current word. If I interpret this code correctly, the if statement logic was reserved and so the word was never being removed from the spell check range. /* if (! inCreatedRange) { */ // <- Incorrect line? - please verify if (inCreatedRange) { // <---my change Someone can verify my understanding. When you recompile, the spellchecker works very well overall (and fixes the problems in this thread). *However* I was still able to easily mess up the mNumWordsInSpellSelection count. For example, pasting and cutting selections seemed to cause this. If you have start an email with 3 misspelled words in it like this: I am speling somthing wrng hre. mNumWordsInSpellSelection will be 4. If you hit enter at the end of the line, these 4 are re-counted and mNumWordsInSpellSelection is set to 8. Now, put the caret back at the end of the line and press enter again. mNumWordsInSpellSelection will then be 12. This happens each time you do this. So there are still bugs in the code related to this count of words, but the fix above seems to fix this to 90%, and seems to make the spell checker quite usable. Please let me know if you confirm and verify my logic and when this fix will be in the nightly. (It is removing one character of code--- hopefully that can be done quickly ;) Thanks. - Kent
Can anyone point me to any development documentation (designs, classes description, etc?) I might be interested in helping fix other issues that annoy my wife ;)
Whiteboard: [needs owner]
brettw: thoughts (starting in the neighborhood of comment 27)? Note that it's not Thunderbird-only, http://www.mozilla.org/editor/midasdemo/ in Firefox does the same thing, but I haven't moved the bug so we can hang onto our blocking flag since "only marks a dozen misspelled words in rich editors" is a blocker for us (even our "plain text compose" editor is rich) but isn't for Firefox and thus isn't for Gecko.
Summary: Thunderbird inline spell check broken → Spellcheck gives up after too few misspelled words in HTML editor
Magnus, Yes, that is the place I made the change. I don't know how fast I can read up on creating a patch, so if anyone else wants to take ownership of this bug, *please* do. Also, like I noted in comment 29, the counter still has other bugs in it, but I think this single change makes it much much better.
Whiteboard: [needs owner] → [dmose to find owner]
No longer depends on: 432225
Blocks: 389933
Depends on: 432225
(In reply to comment #32) > brettw: thoughts (starting in the neighborhood of comment 27)? Sorry, I don't remember how the counter is supposed to work at all.
Whiteboard: [dmose to find owner] → [dmose to re-ping damon]
Whiteboard: [dmose to re-ping damon] → [dmose to re-ping damon; high risk]
Whiteboard: [dmose to re-ping damon; high risk] → [damons looking for owner]
(In reply to comment #29) > /* if (! inCreatedRange) { */ // <- Incorrect line? - please verify > if (inCreatedRange) { // <---my change I looked some more and now believe that my proposed fix in comment #29 is wrong. It does make the symptoms in this thread disappear, but I think it creates other problems. My gut/studying is telling me that we should be generally hitting the continue statement here, but are not (while typing new words): 1391 if (noCheckRange) { 1392 PRBool inExclusion = PR_FALSE; 1393 noCheckRange->IsPointInRange(beginNode, beginOffset, &inExclusion); 1394 if (inExclusion) 1395 continue; 1396 } I have no proof for this yet, but since I am getting at this at an extremely slow pace, if someone claims ownership of this bug, this may be a good place to start.
Whiteboard: [damons looking for owner] → [dmose looking for owner]
I fixed the reported problem in this thread by *unconditionally* checking for spellcheck intersections in the area I described in comment #29. However, this "fix" creates a worse problem: typing many misspelled words eventually leads to a situation where we timeout before finishing the spell check, but instead of starting where we left off, each time it starts over from the beginning and re-times out, so it just eats CPU processing the same misspelled words over and over.
I was doing some mild sleuthing, and here's the largest problem. I typed "asd asd" into the editor with a printf on every misspelled word, as well as the number of unspelled words. The output: Misspelled asd (0) Misspelled asd (1) Misspelled asd (2) Misspelled asd (3) Misspelled asd (4) In short, the first asd accounts for 4 of the misspelled words; we're rechecking the same range over and over again and finding the same misspelled word.
These are all of the cases I can find were the spell counts desynchronize: 1. All words in a line are checked for being misspelled without decrementing. 2. Select text + delete does not seem to work in some cases. The culprit seems to be FinishInitOnEvent.
From my understanding we have two problems here. 1. The if statement of comment #29 is inverted. 2. We're checking too many things: we want to be checking the word and not the line.
Other problems abound, this much I see as the solution for problem 2 of comment #39: We're getting too large of a range for the change (the line) by the call on line 798 of mozInlineSpellChecker.cpp. For anyone else working on this, the following code is bloody useful in debugging: it tells us what we're trying to spell check and how many bad words we have. nsString rangeContents; aStatus->mRange->ToString(rangeContents); printf("%s %d\n", NS_ConvertUTF16toUTF8(rangeContents).get(), mNumWordsInSpellSelection); And now my body has its wakefulness wall. And there definitely is a bug on deleting a lot of text at once. It's wrong sometimes, I just can't figure out when it's wrong.
Joshua, All words in a line are rechecked... they are also all in mCreatedRange. I do not think the original authors intended this. Instead of *inverting* the statement in comment #29, if you *remove* it (so it *always* checks for removal of intersecting ranges), you get very good results ... it seems like the bug is fixed. Try that. But that is when you realize comment #36. I think it needs to be worked out so that we do not recheck the whole line each time we spell check. Also, it seems wrong that the entire line is in mCreatedRange -- even if you are just typing (and not pasting text).
Attached patch Patch, version 0.8 (obsolete) — — Splinter Review
This is my shot at solving this. I did what I think what was originally intended: make mCheckedRange be the inserted text and only the inserted text. Just this single change seems to have fixed the issue, as I don't count to infinity rapidly. I still need to test more, though.
Assignee: nobody → Pidgeot18
Status: NEW → ASSIGNED
Whiteboard: [dmose looking for owner] → [patch in progress]
Joshua, That fix seems good, except that it produces the same problem described in comment #36. If you start typing lots of garbage (this could happen if someone tries to email code or other text that isn't English) - don't hit enter, so it is all one long line - eventually you get to the point where the the spell checker can't finish spell checking all the words in the line before it times-out. But when it "resumes", it *seems* (not 100% sure) it isn't really resuming at all but rather starting over, and timing out again. And again. And again. The CPU goes nuts for a really long time. I feel we need to work to the point where the spell checker doesn't re-check words it has already checked...
This patch ameliorates the problems to what I think is an acceptable level: if you mash the keyboard quickly enough, yes, the spell check lags, but the CPU usage will immediately tank after you stop typing. I also verified that the spell check correctly in the case of typing quickly and rapidly delete, and it appears to work properly for copy-paste content, but I haven't checked thoroughly enough...
Attachment #340127 - Attachment is obsolete: true
(In reply to comment #45) > How does it work? What it does (it isn't the best commented in the world) is that it (ResumeCheck) will only run a spell check request (pushed by ScheduleSpellCheck) if the request is the last one pushed or the last one pushed has special range calculations.
+ mLastStatus = &resume->mStatus; A bit more clear if you use aStatus You need to clear mLastStatus somewhere. Right now it just hangs around as a dangling pointer. If a new status object happened to be allocated at the same address, things would go wrong. But it seems to me that with this patch, if two pastes happen without the spellcheck event firing in between, the first paste simply won't be spellchecked. Is that true?
(In reply to comment #47) > + mLastStatus = &resume->mStatus; > > A bit more clear if you use aStatus Clearer but also incorrect, as mozInlineSpellResume::mStatus is copy-constructed. > You need to clear mLastStatus somewhere. Right now it just hangs around as a > dangling pointer. If a new status object happened to be allocated at the same > address, things would go wrong. Good point. > But it seems to me that with this patch, if two pastes happen without the > spellcheck event firing in between, the first paste simply won't be > spellchecked. Is that true? That appears to be the case, although it took me three tries to get the events firing. I guess it's back to the drawing board again...
I just found a way to work around this problem. Start the email with a single bad word followed by enter. Then on the next line you start typing your email and every misspelled work will be checked perfectly. The key is that you have one misspelled word on the first line all by it's self.
Never mind. I spoke to soon. It still will not check all the words. It checks a little better but still messed up.
Unfortunately due to the extra work required to fix this bug, it isn't going to make Alpha 3, moving out to beta 1.
Target Milestone: Thunderbird 3.0a3 → Thunderbird 3.0b1
(In reply to comment #1) > WFM on linux. Same here.
Fixed by the bug 432225 backout of bug 389933
Assignee: Pidgeot18 → nobody
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
OS: Windows Vista → All
Hardware: PC → All
Resolution: --- → FIXED
Whiteboard: [patch in progress]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: