Last Comment Bug 340634 - spell checker doesn't suggest "alot" correction
: spell checker doesn't suggest "alot" correction
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Spelling checker (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla9
Assigned To: :Ehsan Akhgari (busy, don't ask for review please)
:
Mentors:
Depends on: 1168802
Blocks:
  Show dependency treegraph
 
Reported: 2006-06-06 23:19 PDT by skierpage
Modified: 2015-05-27 05:12 PDT (History)
13 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (v1) (684 bytes, patch)
2008-01-05 23:16 PST, :Ehsan Akhgari (busy, don't ask for review please)
ryanvm: review-
Details | Diff | Review
Patch (v2) (1.54 KB, patch)
2008-01-09 09:23 PST, :Ehsan Akhgari (busy, don't ask for review please)
ryanvm: review+
mscott: superreview+
mtschrep: approval1.9+
Details | Diff | Review
Fix the crash (851 bytes, patch)
2008-01-11 01:29 PST, :Ehsan Akhgari (busy, don't ask for review please)
no flags Details | Diff | Review
Patch (v3) (for check-in) (1.87 KB, patch)
2008-01-11 21:39 PST, :Ehsan Akhgari (busy, don't ask for review please)
ryanvm: review+
mscott: superreview+
mtschrep: approval1.9+
Details | Diff | Review
same fix to en-US.aff as before (547 bytes, patch)
2011-08-28 15:33 PDT, skierpage
no flags Details | Diff | Review

Description skierpage 2006-06-06 23:19:22 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060606 Minefield/3.0a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060601 Minefield/3.0a1

The spell checker in Minefield (and Thunderbird) is great, but it doesn't suggest "a lot" for "alot".  Yet it will suggest "a little" for "alittle".

Reproducible: Always

Steps to Reproduce:
1.  In a text field, enter "Test alot alittle "
2.  Wait for the red squiggles
3.  Right-click on the two words.

Actual Results:  
Spell checker correctly suggests "a little" for "alittle", but not "a lot" for "alot".

Expected Results:  
Suggest "a lot" for the most common spelling misteak in the English language.

Spell checking in the Thunderbird 1.5.0.4 composition window has this bug also, not sure if that should be a separate bug.

My current Minefield build doesn't show any suggestions but I saw this a few days ago.
Comment 1 Tad Whiteside 2006-09-21 08:27:19 PDT
Thunderbird 1.5.0.5 (20060725) Suse 9.3

"alot" is detected as mispelled, but the original comment is correct, "a lot" is not suggested
Comment 2 guanxi 2006-10-26 07:35:08 PDT
test alot 

*** This bug has been marked as a duplicate of 335813 ***
Comment 3 skierpage 2008-01-05 17:29:30 PST
I'm reopening, this is still broken in
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2008010508 Minefield/3.0b3pre ID:2008010508

Comment #14 in bug 335813 by Németh László suggests something special needs to be added to get "a lot" as a suggestion.  My en-US.dic and en-US.aif are dated 2008-01-04 and don't have "a lot" or "REP a_lot".
Comment 4 :Ehsan Akhgari (busy, don't ask for review please) 2008-01-05 23:16:17 PST
Created attachment 295597 [details] [diff] [review]
Patch (v1)

Trivial patch.  With this applied, the trunk suggests "a lot" as the first suggestion for "alot".
Comment 5 Scott MacGregor 2008-01-07 22:53:21 PST
Comment on attachment 295597 [details] [diff] [review]
Patch (v1)

weren't we applying a mozilla patch to the dictionaries as opposed to modifying the dictionaries by hand where they might get stomped on the next time we get a new dictionary drop?
Comment 6 Ryan VanderMeulen [:RyanVM] 2008-01-08 15:24:08 PST
Comment on attachment 295597 [details] [diff] [review]
Patch (v1)

Correct, mozilla_words.diff will also need to be updated with this change so that it doesn't get lost the next time the dictionary or its affix file gets updated.

I've CCed Kevin to this bug, so hopefully that's enough for him to be aware of the change upstream. Kevin, do you want me to post a note in your bug tracker as well?
Comment 7 :Ehsan Akhgari (busy, don't ask for review please) 2008-01-09 09:23:19 PST
Created attachment 296163 [details] [diff] [review]
Patch (v2)

Updated mozilla_words.diff as well in this patch.
Comment 8 Ryan VanderMeulen [:RyanVM] 2008-01-09 16:15:52 PST
Comment on attachment 296163 [details] [diff] [review]
Patch (v2)

Looks good, thanks.
Comment 9 Reed Loden [:reed] (use needinfo?) 2008-01-10 20:21:13 PST
Checking in extensions/spellcheck/locales/en-US/hunspell/en-US.aff;
/cvsroot/mozilla/extensions/spellcheck/locales/en-US/hunspell/en-US.aff,v  <--  en-US.aff
new revision: 1.3; previous revision: 1.2
done
Checking in extensions/spellcheck/locales/en-US/hunspell/mozilla_words.diff;
/cvsroot/mozilla/extensions/spellcheck/locales/en-US/hunspell/mozilla_words.diff,v  <--  mozilla_words.diff
new revision: 1.4; previous revision: 1.3
done
Comment 10 Reed Loden [:reed] (use needinfo?) 2008-01-11 00:25:54 PST
Backed out the patch due to bloat test crashes in libspellcheck.

Stack trace from SeaMonkey nye box:
Stack:
UNKNOWN 0x110420
AffixMgr::~AffixMgr()+0x0000009D [/home/andrew/tbox/SeaMonkey-Debug/Linux_2.6.22.14-72.fc6_Depend/mozilla/objdir/dist/bin/components/libspellchecker.so +0x00046E9D]
Hunspell::~Hunspell()+0x00000042 [/home/andrew/tbox/SeaMonkey-Debug/Linux_2.6.22.14-72.fc6_Depend/mozilla/objdir/dist/bin/components/libspellchecker.so +0x0004EA66]
mozHunspell::~mozHunspell()+0x00000059 [/home/andrew/tbox/SeaMonkey-Debug/Linux_2.6.22.14-72.fc6_Depend/mozilla/objdir/dist/bin/components/libspellchecker.so +0x0005088D]
mozHunspell::Release()+0x000000FE [/home/andrew/tbox/SeaMonkey-Debug/Linux_2.6.22.14-72.fc6_Depend/mozilla/objdir/dist/bin/components/libspellchecker.so +0x00050CA0]
nsCOMPtr_base::assign_assuming_AddRef(nsISupports*)+0x0000004C [/home/andrew/tbox/SeaMonkey-Debug/Linux_2.6.22.14-72.fc6_Depend/mozilla/objdir/dist/bin/libxpcom_core.so +0x0009F778]
nsCOMPtr_base::assign_with_AddRef(nsISupports*)+0x00000036 [/home/andrew/tbox/SeaMonkey-Debug/Linux_2.6.22.14-72.fc6_Depend/mozilla/objdir/dist/bin/libxpcom_core.so +0x0009F522]
nsCOMPtr<nsISupports>::operator=(nsISupports*)+0x00000025 [/home/andrew/tbox/SeaMonkey-Debug/Linux_2.6.22.14-72.fc6_Depend/mozilla/objdir/dist/bin/libxul.so +0x00048515]
UNKNOWN [/home/andrew/tbox/SeaMonkey-Debug/Linux_2.6.22.14-72.fc6_Depend/mozilla/objdir/dist/bin/libxpcom_core.so +0x00103765]
PL_DHashTableEnumerate+0x000000A4 [/home/andrew/tbox/SeaMonkey-Debug/Linux_2.6.22.14-72.fc6_Depend/mozilla/objdir/dist/bin/libxpcom_core.so +0x0009B12D]
nsComponentManagerImpl::FreeServices()+0x00000087 [/home/andrew/tbox/SeaMonkey-Debug/Linux_2.6.22.14-72.fc6_Depend/mozilla/objdir/dist/bin/libxpcom_core.so +0x00101733]
NS_ShutdownXPCOM_P+0x0000031E [/home/andrew/tbox/SeaMonkey-Debug/Linux_2.6.22.14-72.fc6_Depend/mozilla/objdir/dist/bin/libxpcom_core.so +0x000AE94E]
ScopedXPCOMStartup::~ScopedXPCOMStartup()+0x00000087 [/home/andrew/tbox/SeaMonkey-Debug/Linux_2.6.22.14-72.fc6_Depend/mozilla/objdir/dist/bin/libxul.so +0x00053213]
XRE_main+0x0000297C [/home/andrew/tbox/SeaMonkey-Debug/Linux_2.6.22.14-72.fc6_Depend/mozilla/objdir/dist/bin/libxul.so +0x00058740]
UNKNOWN [/home/andrew/tbox/SeaMonkey-Debug/Linux_2.6.22.14-72.fc6_Depend/mozilla/objdir/dist/bin/seamonkey-bin +0x0000117B]
__libc_start_main+0x000000DC [/lib/libc.so.6 +0x00015DEC]
Comment 11 dwitte@gmail.com 2008-01-11 00:32:09 PST
this is crashing on shutdown, when the service manager tries to destroy hunspell. a little more info from gdb:

#5  0x00002aaab87328e6 in SfxEntry::getNext (this=0x101010101010101)
    at /home/dwitte/builds/mozilla/mozilla/extensions/spellcheck/hunspell/src/affentry.hxx:168
#6  0x00002aaab8731616 in ~AffixMgr (this=0x2aaabd6c1010) at /home/dwitte/builds/mozilla/mozilla/extensions/spellcheck/hunspell/src/affixmgr.cpp:193
#7  0x00002aaab873bbfd in ~Hunspell (this=0x1692640) at /home/dwitte/builds/mozilla/mozilla/extensions/spellcheck/hunspell/src/hunspell.cpp:109
#8  0x00002aaab873dd91 in ~mozHunspell (this=0x16928a0) at /home/dwitte/builds/mozilla/mozilla/extensions/spellcheck/hunspell/src/mozHunspell.cpp:103
#9  0x00002aaab873e1eb in mozHunspell::Release (this=0x16928a0) at /home/dwitte/builds/mozilla/mozilla/extensions/spellcheck/hunspell/src/mozHunspell.cpp:78

looks like one of the SfxEntry pointers is bad, possibly AffixMgr too. this benign-looking checkin must've poked a bug in hunspell...
Comment 12 Reed Loden [:reed] (use needinfo?) 2008-01-11 00:37:19 PST
Németh, is the above bug known? If so, is there a fix?
Comment 13 :Ehsan Akhgari (busy, don't ask for review please) 2008-01-11 00:43:12 PST
Németh: looks like one of the entries in the sStart array are invalid...
Comment 14 Andrew Schultz 2008-01-11 01:20:17 PST
valgrind says
==25017== Conditional jump or move depends on uninitialised value(s)
==25017==    at 0x57AAECE: AffixMgr::~AffixMgr() (affixmgr.cpp:192)
==25017==    by 0x57B4936: Hunspell::~Hunspell() (hunspell.cpp:109)
==25017==    by 0x57B679D: mozHunspell::~mozHunspell() (mozHunspell.cpp:103)

further debugging indicates that an uninitialized element was added in AffixMgr::build_sfxtree(AffEntry*) -- setNextEQ(NULL) and setNextNE(NULL) were called, but not setNext(NULL), so the |next| member was uninitialized, and resulted in a crash.  Calling setNext(NULL) seems sufficient to fix the crash.
Comment 15 :Ehsan Akhgari (busy, don't ask for review please) 2008-01-11 01:29:12 PST
Created attachment 296502 [details] [diff] [review]
Fix the crash

Fix the crash according to comment 14 (thanks to Andrew for tracking this down).
Comment 16 :Ehsan Akhgari (busy, don't ask for review please) 2008-01-11 01:31:23 PST
Comment on attachment 296502 [details] [diff] [review]
Fix the crash

Németh: this should be pushed upstream if it's OK by you.
Comment 17 Németh László 2008-01-11 03:45:16 PST
(In reply to comment #16)
> (From update of attachment 296502 [details] [diff] [review])
> Németh: this should be pushed upstream if it's OK by you.
> 

It seems, the REP patch is wrong: it's need to modify
the REP count in the first REP line, too. I will check the crash.
Thanks, László
Comment 18 Ria Klaassen (not reading all bugmail) 2008-01-11 05:54:36 PST
The spell checker doesn't seem to recognize the word "is" anymore in the US language. I see a red underline :).
Comment 19 Ria Klaassen (not reading all bugmail) 2008-01-11 06:25:09 PST
Also "tabs" is underlined here :).
Comment 20 :Ehsan Akhgari (busy, don't ask for review please) 2008-01-11 09:38:37 PST
(In reply to comment #18)
> The spell checker doesn't seem to recognize the word "is" anymore in the US
> language. I see a red underline :).


(In reply to comment #19)
> Also "tabs" is underlined here :).

The patch for this bug was backed out.  Did you try to manually apply it?
Comment 21 Ria Klaassen (not reading all bugmail) 2008-01-11 09:56:49 PST
Sorry Ehsan, you're right. I didn't test the latest hourly. 
Comment 22 :Ehsan Akhgari (busy, don't ask for review please) 2008-01-11 10:17:20 PST
(In reply to comment #21)
> Sorry Ehsan, you're right. I didn't test the latest hourly. 

Still, if your spell checker can't recognize "is" or "tabs" (and its language is set to English!) then there's something terribly wrong with it.
Comment 23 Ryan VanderMeulen [:RyanVM] 2008-01-11 14:27:35 PST
Ehsan, were you planning on submitting an updated patch with the REP value incremented?
Comment 24 :Ehsan Akhgari (busy, don't ask for review please) 2008-01-11 21:39:15 PST
Created attachment 296668 [details] [diff] [review]
Patch (v3) (for check-in)

(In reply to comment #23)
> Ehsan, were you planning on submitting an updated patch with the REP value
> incremented?

Yup :-)
Comment 25 Ryan VanderMeulen [:RyanVM] 2008-01-13 05:55:20 PST
Comment on attachment 296668 [details] [diff] [review]
Patch (v3) (for check-in)

Working fine locally.
Comment 26 Reed Loden [:reed] (use needinfo?) 2008-01-19 19:04:48 PST
Comment on attachment 296668 [details] [diff] [review]
Patch (v3) (for check-in)

Same as the approved patch above but fixes a problem.
Comment 27 Reed Loden [:reed] (use needinfo?) 2008-01-21 16:14:31 PST
Checking in extensions/spellcheck/locales/en-US/hunspell/en-US.aff;
/cvsroot/mozilla/extensions/spellcheck/locales/en-US/hunspell/en-US.aff,v  <--  en-US.aff
new revision: 1.5; previous revision: 1.4
done
Checking in extensions/spellcheck/locales/en-US/hunspell/mozilla_words.diff;
/cvsroot/mozilla/extensions/spellcheck/locales/en-US/hunspell/mozilla_words.diff,v  <--  mozilla_words.diff
new revision: 1.6; previous revision: 1.5
done
Comment 28 Marcia Knous [:marcia - use ni] 2008-01-22 10:29:25 PST
I have not idea why (except I remember something of a bug related to the native mac spell checker), but this does not work on Mac using  Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b3pre) Gecko/2008012204 Minefield/3.0b3pre but works fine on Win Vista with today's nightly build.
Comment 29 Ryan VanderMeulen [:RyanVM] 2008-01-22 15:01:58 PST
Firefox on OSX uses the native OSX spellchecking, not Hunspell. Apple will need to fix it for that platform.
Comment 30 Scott MacGregor 2008-02-04 13:13:39 PST
Ehsan, is the code review request here still necessary?
Comment 31 :Ehsan Akhgari (busy, don't ask for review please) 2008-02-04 22:14:09 PST
Hmm, I'm not sure.  There definitely isn't any crash caused by this bug now, but I think the code fix is still worth looking into.  Németh may be able to comment here more precisely, but yes, I appreciate your sr on the patch once you get to it.  :-)
Comment 32 :Ehsan Akhgari (busy, don't ask for review please) 2008-08-19 14:27:52 PDT
Ping?
Comment 33 Ryan VanderMeulen [:RyanVM] 2008-08-19 16:56:51 PDT
What? It's RESOLVED FIXED for a reason.
Comment 34 Ryan VanderMeulen [:RyanVM] 2008-08-19 16:58:31 PDT
As for the crashing issue, I think that's fixed in a later version of Hunspell, but currently it fails during linking with libxul. Regardless, this bug is fixed. File another one for the crashing issue (though it would probably be better to wait until Hunspell is updated to the latest version).
Comment 35 Wayne Mery (:wsmwk, NI for questions) 2008-09-02 05:53:54 PDT
Comment on attachment 296502 [details] [diff] [review]
Fix the crash

clearing review request per Ehsan. if still a problem see comment 34
Comment 36 skierpage 2011-02-12 16:37:54 PST
This bug seems to have returned in Firefox 4 nightly and SeaMonkey 2.1b1
Mozilla/5.0 (X11; Linux x86_64; rv:2.0b7pre) Gecko/20101008 Firefox/4.0b7pre SeaMonkey/2.1b1
Mozilla/5.0 (X11; Linux x86_64; rv:2.0b12pre) Gecko/20110212 Firefox/4.0b12pre

Comparing them with an older Firefox 3.6 program, both lack the REP alot a_lot line in dictionaries/en-US.aff.  Matt Caywood's check-in for bug 479334 removed this and a few other changes in en-US.aff, but his comment in bug 479334 is "Hunspell now handles the alot case correctly".  Hmmm.

Thanks alot for all you do.  Lets take the reigns and not loose site of the goal of better spell-checking.
Comment 37 skierpage 2011-08-28 15:33:34 PDT
Created attachment 556433 [details] [diff] [review]
same fix to en-US.aff as before
Comment 38 skierpage 2011-08-28 15:36:56 PDT
I manually applied the two-line change to en-US.aff that's in attachment #296668 [details] [diff] [review] (that was reverted by bug 479334) to Firefox nightly and it still fixes this bug. Please could someone reapply it?!  I attached it as a patch but am not sure of hg patchset details, here's the gist of it below.

% diff -p -u -1 {firefox_org,firefox}/dictionaries/en-US.aff
--- firefox_org/dictionaries/en-US.aff  2011-08-28 04:06:01.000000000 -0700
+++ firefox/dictionaries/en-US.aff      2011-08-28 14:50:45.342893468 -0700
@@ -112,3 +112,3 @@ SFX L   0     ment       .
 
-REP 88
+REP 89
 REP a ei
@@ -119,2 +119,3 @@ REP ai ie
 REP ie ai
+REP alot a_lot
 REP are air
Comment 39 Josh Matthews [:jdm] 2011-08-28 16:07:56 PDT
Comment on attachment 556433 [details] [diff] [review]
same fix to en-US.aff as before

Checkin is the wrong flag here, I believe. Ehsan, what's the status of this patch?
Comment 40 :Ehsan Akhgari (busy, don't ask for review please) 2011-09-02 10:35:41 PDT
Relanded: http://hg.mozilla.org/integration/mozilla-inbound/rev/b1063cb1f393
Comment 41 Marco Bonardo [::mak] 2011-09-03 03:00:03 PDT
http://hg.mozilla.org/mozilla-central/rev/b1063cb1f393
Comment 42 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-09-24 19:31:18 PDT
We had removed this rule intentionally, since newer hunspell versions were supposed to handle this automatically (see bug 479334 comment 55). Was that just wrong?
Comment 43 :Ehsan Akhgari (busy, don't ask for review please) 2011-09-26 14:39:22 PDT
(In reply to Gavin Sharp (use gavin@gavinsharp.com for email) from comment #42)
> We had removed this rule intentionally, since newer hunspell versions were
> supposed to handle this automatically (see bug 479334 comment 55). Was that
> just wrong?

It appears so.  :(

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