340634 - spell checker doesn't suggest "alot" correction

Status: RESOLVED FIXED

(Core :: Spelling checker, defect)

Description

[skierpage]

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]

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]

test alot 

*** This bug has been marked as a duplicate of 335813 ***

Comment 3

[skierpage]

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

[(no longer active)]

Attachment: Patch (v1)

Trivial patch.  With this applied, the trunk suggests "a lot" as the first suggestion for "alot".

Comment 5

[Scott MacGregor]

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]]

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

[(no longer active)]

Attachment: Patch (v2)

Updated mozilla_words.diff as well in this patch.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 296163 [details] [diff] [review]
Patch (v2)

Looks good, thanks.

Comment 9

[Reed Loden [:reed]]

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]]

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

[[e-mail]]

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]]

Németh, is the above bug known? If so, is there a fix?

Comment 13

[(no longer active)]

Németh: looks like one of the entries in the sStart array are invalid...

Comment 14

[Andrew Schultz]

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

[(no longer active)]

Attachment: Fix the crash

Fix the crash according to comment 14 (thanks to Andrew for tracking this down).

Comment 16

[(no longer active)]

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ó]

(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)]

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)]

Also "tabs" is underlined here :).

Comment 20

[(no longer active)]

(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)]

Sorry Ehsan, you're right. I didn't test the latest hourly. 

Comment 22

[(no longer active)]

(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]]

Ehsan, were you planning on submitting an updated patch with the REP value incremented?

Comment 24

[(no longer active)]

Attachment: 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]]

Comment on attachment 296668 [details] [diff] [review]
Patch (v3) (for check-in)

Working fine locally.

Comment 26

[Reed Loden [:reed]]

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]]

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]]

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]]

Firefox on OSX uses the native OSX spellchecking, not Hunspell. Apple will need to fix it for that platform.

Comment 30

[Scott MacGregor]

Ehsan, is the code review request here still necessary?

Comment 31

[(no longer active)]

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

[(no longer active)]

Ping?

Comment 33

[Ryan VanderMeulen [:RyanVM]]

What? It's RESOLVED FIXED for a reason.

Comment 34

[Ryan VanderMeulen [:RyanVM]]

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)]

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]

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]

Attachment: same fix to en-US.aff as before

Comment 38

[skierpage]

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]]

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

[(no longer active)]

Relanded: http://hg.mozilla.org/integration/mozilla-inbound/rev/b1063cb1f393

Comment 41

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/b1063cb1f393

Comment 42

[:Gavin Sharp [email: [e-mail]]]

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

[(no longer active)]

(In reply to Gavin Sharp (use [e-mail] 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.  :(