"Avant-Garde" Not Spellchecked Properly

RESOLVED FIXED in mozilla22

Status

()

Core
Spelling checker
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: draco18s, Assigned: Joshua Gaines)

Tracking

Trunk
mozilla22
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [mentor=ehsan])

Attachments

(1 attachment, 6 obsolete attachments)

(Reporter)

Description

5 years ago
User Agent: Mozilla/5.0 (Windows NT 6.0; WOW64; rv:16.0) Gecko/20100101 Firefox/16.0
Build ID: 20121024073032

Steps to reproduce:

Type Avant-Gaurd
R-click to correct the spelling


Actual results:

Avant-Gaurd was corrected to Avaunt-Gaurd.  Which Firefox indicates is also spelled incorrectly.  R-clicking again, it suggests Avaunt-Gaurd.  Which is also incorrect, and is underlined.  Which corrects to Avaunt-Guard.  Which is also not the correct spelling.


Expected results:

It should correctly spellcheck to "Avant-Garde"
Mozilla/5.0 (X11; Linux i686; rv:20.0) Gecko/20121204 Firefox/20.0

Confirming spell-checking issue.
Status: UNCONFIRMED → NEW
Component: Untriaged → Spelling checker
Ever confirmed: true
OS: Windows Vista → All
Product: Firefox → Core
Hardware: x86_64 → All
Version: 16 Branch → Trunk
See <https://developer.mozilla.org/en-US/docs/Adding_a_new_word_to_the_en-US_dictionary> for how to fix this bug.
Whiteboard: [mentor=ehsan]
(Reporter)

Comment 3

5 years ago
(In reply to Ehsan Akhgari [:ehsan] from comment #2)
> See
> <https://developer.mozilla.org/en-US/docs/Adding_a_new_word_to_the_en-
> US_dictionary> for how to fix this bug.

I know how to add new words to the dictionary, that wasn't actually the problem.  The problem is that it suggested a spelling and then said that that new spelling was ALSO WRONG.  The fact that the suggestions were incorrectly spelled was a side note.
Well that's because we don't have avant-garde in our en-US dictionary, which makes it impossible for the spell checker to show the correct suggestion.
(Reporter)

Comment 5

5 years ago
(In reply to Ehsan Akhgari [:ehsan] from comment #4)
> Well that's because we don't have avant-garde in our en-US dictionary, which
> makes it impossible for the spell checker to show the correct suggestion.

Did you actually read the initial report?

Avant-Gaurd -> Avaunt-Gaurd -> Avaunt-Gaurd -> Avaunt-Guard

Each of the first three are underlined in red and using the spellcheck feature it suggests the next one over.  If "Avaunt-Gaurd" is a suggested spelling, meaning that it's in the dictionary, why is the spellchecker saying it's wrong?

Also, if "avant-garde" isn't in the dictionary, because it's not en-US, then why are "Avaunt-Gaurd", "Avaunt-Gaurd", and "Avaunt-Guard" in the dictionary (and SPELLED WRONG)?
(In reply to comment #5)
> (In reply to Ehsan Akhgari [:ehsan] from comment #4)
> > Well that's because we don't have avant-garde in our en-US dictionary, which
> > makes it impossible for the spell checker to show the correct suggestion.
> 
> Did you actually read the initial report?
> 
> Avant-Gaurd -> Avaunt-Gaurd -> Avaunt-Gaurd -> Avaunt-Guard
> 
> Each of the first three are underlined in red and using the spellcheck feature
> it suggests the next one over.  If "Avaunt-Gaurd" is a suggested spelling,
> meaning that it's in the dictionary, why is the spellchecker saying it's wrong?
> 
> Also, if "avant-garde" isn't in the dictionary, because it's not en-US, then
> why are "Avaunt-Gaurd", "Avaunt-Gaurd", and "Avaunt-Guard" in the dictionary
> (and SPELLED WRONG)?

None of these words are in the dictionary.  This is just the spell checker getting mislead because of the hyphen in this word.  Please see <http://mxr.mozilla.org/mozilla-central/source/extensions/spellcheck/locales/en-US/hunspell/en-US.dic> for the dictionary source that I'm talking about.
(Reporter)

Comment 7

5 years ago
(In reply to Ehsan Akhgari [:ehsan] from comment #6)
> This is just the spell checker
> getting mislead because of the hyphen

Sounds like a bug!
(Assignee)

Comment 8

4 years ago
I would like to take a stab at the this bug.
Great, thanks!
Assignee: nobody → jwgain0
(Assignee)

Comment 10

4 years ago
Ok I have come up with a fix for this bug. It was simple, I just added "Avant-garde" and "avant-garde" to the US-EN.dic file. Now when you type the incorrect spelling "avant-gaurd", then "avant-garde" is now suggested as a correction. I am attaching two patch files. Patch1.txt is where I altered the extensions\spellcheck\locales\en-US\hunspell\dictionary-sourceshunspell-en_US-20081205.dic to contain the new words. Patch2.txt is just the diff of the upstream dictionary file where the lower lever dictionaries are merged together into one main dictionary.

Now, I am just waiting on how to proceed since this is my first bug fix. Do I need to create some sort of automated test for this fix? I have obvisouly built firefox with my changes and manually tested it.
(Assignee)

Comment 11

4 years ago
Created attachment 706935 [details] [diff] [review]
Patch1

This patch is just the diff of where I added the word to the lower level hunspell-en_US-20081205.dic found in \extensions\spellcheck\locales\en-US\hunspell\dictionary-sources
Attachment #706935 - Flags: review?(ehsan)
(Assignee)

Comment 12

4 years ago
Created attachment 706936 [details] [diff] [review]
Patch2

This patch is just a diff of the final dictionary file after the make merges the lower dictionary files together.
Attachment #706936 - Flags: review?(ehsan)
Comment on attachment 706935 [details] [diff] [review]
Patch1

Review of attachment 706935 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks a lot for your patch, Joshua!

Please make a patch which contains all of the changes to the repository, not one patch per file.  See <https://developer.mozilla.org/en-US/docs/Creating_a_patch> for an explanation on how to do that.

Also, although we usually require automated tests alongside patches, I don't think there's much point in an automated test for this, so you don't need to add one here.

Waiting to see your next patch! :-)

::: extensions/spellcheck/locales/en-US/hunspell/en-US.dic
@@ +1265,5 @@
>  Autumn/M
>  Av/M
>  Ava/M
>  Avalon
> +Avant-garde

You shouldn't need to add the upper-case version.
Attachment #706935 - Flags: review?(ehsan)
Attachment #706936 - Flags: review?(ehsan)
(Assignee)

Comment 14

4 years ago
Created attachment 709372 [details] [diff] [review]
FullPatch

This is a full diff of the altered directory.
Attachment #706935 - Attachment is obsolete: true
Attachment #706936 - Attachment is obsolete: true
Attachment #709372 - Flags: review?
Comment on attachment 709372 [details] [diff] [review]
FullPatch

Review of attachment 709372 [details] [diff] [review]:
-----------------------------------------------------------------

::: extensions/spellcheck/locales/en-US/hunspell/en-US.dic
@@ +1265,5 @@
>  Autumn/M
>  Av/M
>  Ava/M
>  Avalon
> +Avant-garde

As I said before, you don't need the upper-case version.
Attachment #709372 - Flags: review?
(Assignee)

Comment 16

4 years ago
Created attachment 709465 [details] [diff] [review]
Capitalized form of avant-garde removed

Lets try this one more time
Attachment #709372 - Attachment is obsolete: true
Attachment #709465 - Flags: review?
Comment on attachment 709465 [details] [diff] [review]
Capitalized form of avant-garde removed

Review of attachment 709465 [details] [diff] [review]:
-----------------------------------------------------------------

::: extensions/spellcheck/locales/en-US/hunspell/en-US.dic
@@ +1265,5 @@
>  Autumn/M
>  Av/M
>  Ava/M
>  Avalon
> +Avant-garde

Still here!  Am I missing something?
Attachment #709465 - Flags: review?
(Assignee)

Comment 18

4 years ago
Created attachment 715459 [details] [diff] [review]
Capitialized form of Avant-garde removed

Okay I apologize for my previous patch submissions. Turns out that removing the word from the lower stream dictionaries does not remove it from the upper level merged dictionary. I was thinking that each time I would build the upper level dictionary was reconstructed from the lower level dictionaries. Anyways, I hope this is the correct patch needed.
Attachment #709465 - Attachment is obsolete: true
Attachment #715459 - Flags: review?
(Assignee)

Updated

4 years ago
Attachment #715459 - Flags: review? → review?(ehsan)
Comment on attachment 715459 [details] [diff] [review]
Capitialized form of Avant-garde removed

Review of attachment 715459 [details] [diff] [review]:
-----------------------------------------------------------------

::: extensions/spellcheck/locales/en-US/hunspell/en-US.dic
@@ +1,1 @@
> +57444

Hmm, this count is wrong.  I'd expect attempting to load this dictionary to crash, etc.  :(

I suggest that you start over on a clean tree, and don't use the previous changes made to the dictionaries...  Sorry that this proved to be this much of a hassle.  :-)
Attachment #715459 - Flags: review?(ehsan)
(Assignee)

Comment 20

4 years ago
Created attachment 718415 [details] [diff] [review]
Clean Tree Patch

Hopefully I got it write this time. I pulled a clean tree and added only the lower case word to the dictionary. Sorry I didn't knock this simple fix out quickly.
Attachment #715459 - Attachment is obsolete: true
Attachment #718415 - Flags: review?(ehsan)
Comment on attachment 718415 [details] [diff] [review]
Clean Tree Patch

Review of attachment 718415 [details] [diff] [review]:
-----------------------------------------------------------------

The en-US.dic file seems to be missing from this patch...
Attachment #718415 - Flags: review?(ehsan)
(Assignee)

Comment 22

4 years ago
Created attachment 718776 [details] [diff] [review]
Fixed? Final Patch?

So I think I am going for the record of patch attempts for a trivial fix. I am very sorry if I cause any inconvenience. I forgot to merge the dictionaries and move the new en-US.dic to the top directory. I am hoping this I have go it correct this time.
Attachment #718415 - Attachment is obsolete: true
Attachment #718776 - Flags: review?(ehsan)
Comment on attachment 718776 [details] [diff] [review]
Fixed? Final Patch?

Review of attachment 718776 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great, thanks a lot!  And no worries about the number of iterations, it was astonishing to see you stick around this whole time and persevere until everything is right!  Thanks for doing that!  :-)
Attachment #718776 - Flags: review?(ehsan) → review+
https://hg.mozilla.org/mozilla-central/rev/eb562a97db7c
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in before you can comment on or make changes to this bug.