spell checker doesn't suggest "alot" correction

RESOLVED FIXED in mozilla9

Status

()

Core
Spelling checker
RESOLVED FIXED
11 years ago
2 years ago

People

(Reporter: skierpage, Assigned: Ehsan)

Tracking

Trunk
mozilla9
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

11 years ago
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

11 years ago
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

11 years ago
test alot 

*** This bug has been marked as a duplicate of 335813 ***
Status: UNCONFIRMED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → DUPLICATE
(Reporter)

Comment 3

10 years ago
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".
Status: RESOLVED → UNCONFIRMED
Resolution: DUPLICATE → ---
Status: UNCONFIRMED → NEW
Ever confirmed: true
Created attachment 295597 [details] [diff] [review]
Patch (v1)

Trivial patch.  With this applied, the trunk suggests "a lot" as the first suggestion for "alot".
Assignee: mscott → ehsan.akhgari
Status: NEW → ASSIGNED
Attachment #295597 - Flags: superreview?(mscott)
Attachment #295597 - Flags: review?(mscott)
OS: Windows XP → All
Hardware: PC → All
Target Milestone: --- → mozilla1.9 M11

Comment 5

10 years ago
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?
Attachment #295597 - Flags: review?(mscott) → review?(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?
Attachment #295597 - Flags: review?(ryanvm) → review-
Created attachment 296163 [details] [diff] [review]
Patch (v2)

Updated mozilla_words.diff as well in this patch.
Attachment #295597 - Attachment is obsolete: true
Attachment #296163 - Flags: superreview?(ryanvm)
Attachment #296163 - Flags: review?(ryanvm)
Attachment #295597 - Flags: superreview?(mscott)
Comment on attachment 296163 [details] [diff] [review]
Patch (v2)

Looks good, thanks.
Attachment #296163 - Flags: superreview?(ryanvm)
Attachment #296163 - Flags: superreview+
Attachment #296163 - Flags: review?(ryanvm)
Attachment #296163 - Flags: review+
Attachment #296163 - Flags: approval1.9?

Updated

10 years ago
Attachment #296163 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
Attachment #296163 - Flags: superreview+ → superreview?(mscott)
Keywords: checkin-needed

Updated

10 years ago
Attachment #296163 - Flags: superreview?(mscott) → superreview+
Keywords: checkin-needed
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
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
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]
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 11

10 years ago
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...
Németh, is the above bug known? If so, is there a fix?
Németh: looks like one of the entries in the sStart array are invalid...

Comment 14

10 years ago
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.
Created attachment 296502 [details] [diff] [review]
Fix the crash

Fix the crash according to comment 14 (thanks to Andrew for tracking this down).
Attachment #296502 - Flags: superreview?(mscott)
Attachment #296502 - Flags: review?
Comment on attachment 296502 [details] [diff] [review]
Fix the crash

Németh: this should be pushed upstream if it's OK by you.
Attachment #296502 - Flags: review? → review?(nemeth)

Comment 17

10 years ago
(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ó
The spell checker doesn't seem to recognize the word "is" anymore in the US language. I see a red underline :).
Also "tabs" is underlined here :).
(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?
Sorry Ehsan, you're right. I didn't test the latest hourly. 
(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.
Ehsan, were you planning on submitting an updated patch with the REP value incremented?
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 :-)
Attachment #296163 - Attachment is obsolete: true
Attachment #296668 - Flags: superreview?(mscott)
Attachment #296668 - Flags: review?(ryanvm)
Comment on attachment 296668 [details] [diff] [review]
Patch (v3) (for check-in)

Working fine locally.
Attachment #296668 - Flags: review?(ryanvm) → review+

Updated

10 years ago
Attachment #296668 - Flags: superreview?(mscott) → superreview+
Comment on attachment 296668 [details] [diff] [review]
Patch (v3) (for check-in)

Same as the approved patch above but fixes a problem.
Attachment #296668 - Flags: approval1.9?

Updated

10 years ago
Attachment #296668 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
Attachment #296668 - Attachment description: Patch (v3) → Patch (v3) (for check-in)
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
Status: REOPENED → RESOLVED
Last Resolved: 10 years ago10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
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.
Firefox on OSX uses the native OSX spellchecking, not Hunspell. Apple will need to fix it for that platform.

Comment 30

10 years ago
Ehsan, is the code review request here still necessary?
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.  :-)
Ping?
What? It's RESOLVED FIXED for a reason.
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).
Attachment #296502 - Flags: superreview?(mscott)
Attachment #296502 - Flags: review?(nemeth)
Comment on attachment 296502 [details] [diff] [review]
Fix the crash

clearing review request per Ehsan. if still a problem see comment 34
(Reporter)

Comment 36

6 years ago
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Comment 37

6 years ago
Created attachment 556433 [details] [diff] [review]
same fix to en-US.aff as before
Attachment #556433 - Flags: checkin+
(Reporter)

Comment 38

6 years ago
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 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?
Attachment #556433 - Flags: checkin+
Relanded: http://hg.mozilla.org/integration/mozilla-inbound/rev/b1063cb1f393
Target Milestone: mozilla1.9beta3 → mozilla9
http://hg.mozilla.org/mozilla-central/rev/b1063cb1f393
Status: REOPENED → RESOLVED
Last Resolved: 10 years ago6 years ago
Resolution: --- → FIXED
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?
(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.  :(
Depends on: 1168802
You need to log in before you can comment on or make changes to this bug.