The default bug view has changed. See this FAQ.

Spellchecker doesn't display suggestion list for misspelled words if Russian Spell dictionary is installed

VERIFIED FIXED in mozilla1.9alpha1

Status

()

Core
Spelling checker
--
major
VERIFIED FIXED
12 years ago
10 years ago

People

(Reporter: Alexander L. Slovesnik, Assigned: Scott MacGregor)

Tracking

(4 keywords)

1.8 Branch
mozilla1.9alpha1
x86
Windows XP
fixed1.8.1, regression, verified1.8.0.2, verified1.8.1.3
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8.1 +
blocking1.8.0.1 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [nvn-dl])

Attachments

(4 attachments, 2 obsolete attachments)

(Reporter)

Description

12 years ago
Steps to reproduce:
1. Install latest Thunderbird Branch Build (I've tested on Mozilla/5.0 (Windows;
U; Windows NT 5.1; ru; rv:1.8b4) Gecko/20050901 Thunderbird/1.0+ ID:2005090112
and  Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050904
Thunderbird/1.4 ID:2005090406)
2. Install Russian Spell Checker from
http://downloads.mozdev.org/dictionaries/spell-ru.xpi
3. Select Russian in Tools - Options - Composition - Spelling.
4. Go to File - New - Message and write misspelled word in Russian.
5. Select this word (it's underlined with red line if "spell as you type is
enabled) and right-click on it or go to Options - Spell Checking...

Actual results:
Thunderbird shows "No suggestions found"

Expected results:
Thunderbird should suggest list of words instead misspelled one.

Additional information:
This bug doesn't exist in Thunderbird 1.0.6
This bug doesn't exist with en-US spell checker.
This bug also exists in Seamonkey (Mozilla/5.0 (Windows; U; Windows NT 5.1;
ru-RU; rv:1.9a1) Gecko/20050904 SeaMonkey/1.1a)

Comment 1

12 years ago
AFAIK we only supports the en-US dictionary. All other problems with
dictionaries should be taken up with their respective owners. See bug 296123
comment 1.
Summary: Spellchecker doesn't display suggestion list for misspelled words if Russian Spell dictionary is installed → Spellchecker doesn't display suggestion list for misspelled words if Russian Spell dictionary is installed
(Reporter)

Comment 2

12 years ago
(In reply to comment #1)
> AFAIK we only supports the en-US dictionary. All other problems with
> dictionaries should be taken up with their respective owners. See bug 296123
> comment 1.
This bug is not about "some words are missing in spell dictionary". This bug is
about "Thunderbird 1.0+/1.4 is not working with Russian Spell dictionary at
ALL". Thunderbird 1.0+/1.4 is not displaying suggestion list for misspelled
russian words. Never.
This is clearly a regression, cause Thunderbird 1.0.6 works pretty well with
Russian Spell dictionary.
And I never have seen in Thunderbird FAQ or any other Mozilla documentation that
only en-US spell dictionary is supported. Are you want to say that Thunderbird
developers should care only about compatibility with en-US spell checker and
free to break compatibility with all other spell checkers?
I'll try to find regression window on this week.
(Reporter)

Comment 3

12 years ago
OK, I found regression window.
http://archive.mozilla.org/pub/thunderbird/nightly/2005-01-12-06-trunk/ doesn't
have this bug.
http://archive.mozilla.org/pub/thunderbird/nightly/2005-01-13-06-trunk/ have
this bug.

Looking at regression window, it looks like caused by Bug 240600

Comment 4

12 years ago
I just tested this out with the Spanish (Mexican) dictionary and it works fine
(gives me the correct suggestion, etc.). So this must be a problem with the
Russian dictionary itself, which in that case, it's not Thunderbird's problem,
but rather the person who made the dictionary's. 
Or it is a problem with dictionaries in other charsets, in wich case it is a
mozilla problem.
Alexander, can you give an example of a russian word that should give a suggestion?
(Reporter)

Comment 6

12 years ago
(In reply to comment #5)
> Alexander, can you give an example of a russian word that should give a
suggestion?

Sure. For example enter word "кройний" and spellcheck it. Proposed replacement
is "крайний". I'll post screenshot.
(Reporter)

Comment 7

12 years ago
Created attachment 195661 [details]
Screenshot of working ru spellcheck (2005-01-11-06 build)
(Reporter)

Comment 8

12 years ago
Created attachment 195662 [details]
Working Bulgarian spellchecker (2005-01-11-06-trunk)

I've downloaded and checked Bulgarian spell checker. It has same error.
Posting screenshots.
(Reporter)

Comment 9

12 years ago
Created attachment 195664 [details]
Broken Bulgarian spellchecker (2005-01-13-06-trunk)

Broken Bulgarian spellchecker (2005-01-13-06-trunk)
FYI, Bulgarian is using cyrillic alphabet, that's why I'm posting it.

Updated

12 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
> Or it is a problem with dictionaries in other charsets

I bet this is it.  The patch for bug 240600 has the following in mozMySpell::Suggest :

336   *aSuggestionCount = mMySpell->suggest(&wlst, NS_LossyConvertUCS2toASCII(aWord).get());

That'll work for ASCII or ISO-8859-1, but will absolutely fail for any other encoding.  Shouldn't we be using UTF8 here?  I guess that will break all the ISO-8859-1 dictionaries...  What encoding are myspell dictionaries defined to be in?

We should really fix this on branch, imo...
Blocks: 240600
Flags: blocking1.9a1?
Flags: blocking1.8.1?
Flags: blocking1.8.0.1?

Comment 11

12 years ago
You probably have to clone/break out/rewrite the conversion code from mozMySpell::Check, and whomever does that should of course fix the leak...
(Assignee)

Comment 12

12 years ago
Created attachment 205417 [details] [diff] [review]
a fix

I haven't tested this on the russian dictionary yet. 

I fixed (hopefully) a bunch of memory leaks that were listed in individual bugs: Bug 319710, Bug 319709, Bug319707 . I stepped through the various failure points in the debugger by hand to verify that the arrays and strings were getting released properly.

mvl, what do you think of this?
Attachment #205417 - Flags: review?
(Assignee)

Comment 13

12 years ago
Comment on attachment 205417 [details] [diff] [review]
a fix

weird, bugzilla didn't show me a list of possible matches for your name.
Attachment #205417 - Flags: review? → review?(mvl)
(Assignee)

Comment 14

12 years ago
This fix does indeed fix the problem with the russion dictionary for me.
(Assignee)

Comment 15

12 years ago
Created attachment 205422 [details] [diff] [review]
updated fix

the function prototype in the mozMySpell class had the class name scope attached to it by accident. (mozMySpell::ConvertCharset --> ConvertCharset in mozMySpell.h).
Attachment #205417 - Attachment is obsolete: true
Attachment #205422 - Flags: review?(mvl)
Attachment #205417 - Flags: review?(mvl)
Doesn't ConvertCharset() double-free if Convert() fails?  It'll free, and then the nsXPIDLCString on the caller's end will free...
(Assignee)

Comment 17

12 years ago
Created attachment 205459 [details] [diff] [review]
update to fix the double free in ConvertName
Attachment #205422 - Attachment is obsolete: true
Attachment #205459 - Flags: review?(mvl)
Attachment #205422 - Flags: review?(mvl)
(Assignee)

Comment 18

12 years ago
Axel and I talked this over and we aren't going to respin Thunderbird 1.5 for this. We don't ship the l10n dictionary files by default, users have to go install them and this regression only effects a few of the locales.

But we would like to see it go into 1.8.0.1 after a reviewed patch bakes for a while on the trunk for 1.5.0.1. 

We'll be releasing noting this for the handful of locales (Russian, Bulgarian, Greek, Hebrew, etc) which are effected by this.
(Assignee)

Updated

12 years ago
Status: NEW → ASSIGNED
> only effects a few of the locales.

I'd expect it to affect at least (from the 1.0.x localization list):

Chinese, Greek, Gujarati, Hebrew, Japanese, Korean, Punjabi, Russian, Turkish.

Not sure how this matches up with the list of 1.5 localizations, of course.  Also not sure what our actual userbase looks like (eg I know there're a lot of Firefox users in Japan, but I dunno about Thunderbird).
(Assignee)

Comment 20

12 years ago
(In reply to comment #19)
> > only effects a few of the locales.
> 
> I'd expect it to affect at least (from the 1.0.x localization list):
> 
> Chinese, Greek, Gujarati, Hebrew, Japanese, Korean, Punjabi, Russian, Turkish.

There are no dictionaries available for Chinese, Japanese and Korean and thus this issue does not apply to those locales.

 
Ah, ok.  That makes sense, then.

Comment 22

12 years ago
On the long run a good solution might be to migrate to hunspell which will the successor of myspell in OpenOffice.org code from version 2.0.2. Hunspell is backward compatible with myspell dictionaries but can handle Unicode and has other good enhancements over the myspell codebase. http://hunspell.sourceforge.net/
Comment on attachment 205459 [details] [diff] [review]
update to fix the double free in ConvertName


>-  nsresult rv = mEncoder->GetMaxLength(aWord, inLength, &outLength);
>-  // NS_ERROR_UENC_NOMAPPING is a NS_SUCCESS, no error.
>-  if (NS_FAILED(rv) || rv == NS_ERROR_UENC_NOMAPPING) {
>-    // not a word in the current charset, so likely not
>-    // a word in the current language
>-    return PR_FALSE;
>-  }

That check for NS_ERROR_UENC_NOMAPPING seems to be gone now. Is it not needed anymore?

Rest looks good. r=mvl
Attachment #205459 - Flags: review?(mvl) → review+
(Assignee)

Comment 24

11 years ago
Comment on attachment 205459 [details] [diff] [review]
update to fix the double free in ConvertName

bz since he showed original interest in this bug. I can find someone else if your too busy.
Attachment #205459 - Flags: superreview?(bzbarsky)
(Assignee)

Updated

11 years ago
Blocks: 319707
(Assignee)

Updated

11 years ago
Blocks: 319710
(Assignee)

Updated

11 years ago
Blocks: 319709
Keywords: regression
(Assignee)

Updated

11 years ago
Attachment #205459 - Attachment description: updatee to fix the double free in ConvertName → update to fix the double free in ConvertName
Comment on attachment 205459 [details] [diff] [review]
update to fix the double free in ConvertName

This is good stuff, Scott!

sr=bzbarsky
Attachment #205459 - Flags: superreview?(bzbarsky) → superreview+
(Assignee)

Comment 26

11 years ago
fixed on the trunk.
(Assignee)

Comment 27

11 years ago
Triage Team: I've verified that the patch is working on a trunk release build from this morning. Using a russian dictionary, the spell checker is now properly detecting misspelled words.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Is this going to be accepted for the Thunderbird 1.5 release? If so then maybe it should go in. If it's not then I don't see why we'd need it on the 1.8.0 branch in time for .0.1, and could wait until 0.2 and get more testing
Flags: blocking1.8.0.2?
Flags: blocking1.8.0.1?
Flags: blocking1.8.0.1-
(Assignee)

Comment 29

11 years ago
Dan, we didn't have time nor the resources to respin thunderbird 1.5 for this bug but it's a prety bad bug for cyrillic langauges and we would have liked to have taken it. 

The code change itself is to the spell checker which isn't built for Firefox so the testing impact for this is limited to Thunderbird only and if we had the resources we would have tried to respin 1.5 for it. Asking again just in case, in light of this not effecting Firefox.
Flags: blocking1.8.0.1- → blocking1.8.0.1?
Daniel, there are other consumers of the 1.8 branch spellchecker than Thunderbird (Nvu and Seamonkey come to mind).
Attachment #205459 - Flags: approval1.8.1?
Attachment #205459 - Flags: approval1.8.0.1?
Flags: blocking1.8.1?
Flags: blocking1.8.1+
Flags: blocking1.8.0.2?
Flags: blocking1.8.0.1?
Flags: blocking1.8.0.1+
Comment on attachment 205459 [details] [diff] [review]
update to fix the double free in ConvertName

a=dveditz for drivers
Attachment #205459 - Flags: approval1.8.1?
Attachment #205459 - Flags: approval1.8.1+
Attachment #205459 - Flags: approval1.8.0.1?
Attachment #205459 - Flags: approval1.8.0.1+
(Assignee)

Comment 32

11 years ago
fixed on both branches. Clearing the 1.9a1 blocking flag since it's already fixed on the trunk. 
Flags: blocking1.9a1?
Keywords: fixed1.8.0.1, fixed1.8.1
Target Milestone: --- → mozilla1.9alpha
(Assignee)

Comment 33

11 years ago
Alexander, do you want to verify that the fix is working in the 1.5.0.2 test builds?

The russian translation can be found here:

ftp://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/latest-mozilla1.8.0-l10n

Thanks!
(Reporter)

Comment 34

11 years ago
Verified with Mozilla/5.0 (Windows; U; Windows NT 5.1; ru; rv:1.8.0.1) Gecko/20060227 Thunderbird/1.5.0.2 ID:2006022704
Thanks a lot, Scott.
Status: RESOLVED → VERIFIED
(Assignee)

Comment 35

11 years ago
1.8.0.2 is technically the first release with this Thunderbird fix. 
Keywords: fixed1.8.0.1 → fixed1.8.0.2

Updated

11 years ago
Whiteboard: [nvn-dl]
verified per comment #34 - just adding whiteboard note
Whiteboard: [nvn-dl] → [nvn-dl] [qa:verified-tb-1802]

Updated

10 years ago
Keywords: fixed1.8.0.2 → verified1.8.0.2
Whiteboard: [nvn-dl] [qa:verified-tb-1802] → [nvn-dl]

Comment 37

10 years ago
verified tb2.0 rc2 linux with ru dictionary. Note had to disable extension checking since the ru dictionary is not compatible with 2.0.0.0.
Keywords: verified1.8.1.3
You need to log in before you can comment on or make changes to this bug.