216382 - need to put the spell checker dictionary files (should not be in components dir)

Status: RESOLVED FIXED

(Core :: Spelling checker, defect)

Description

[(not reading, please use [e-mail] instead)]

from mscott:

we want to fix where they store the dictionary files.

right now, the build puts them into the components directory.

but they aren't components.

from
http://lxr.mozilla.org/mozilla/source/extensions/spellcheck/myspell/dictionaries/Makefile.in#47

DICTIONARY_FILES=\
	$(srcdir)/en-US.dic \
	$(srcdir)/en-US.aff \
	$(NULL)

libs::
	$(INSTALL) $(DICTIONARY_FILES) $(DIST)/bin/components/myspell

mscott, can you clarify where they should be going?

Comment 1

[R.K.Aa.]

*** Bug 238092 has been marked as a duplicate of this bug. ***

Comment 2

[Michiel van Leeuwen (email: mvl+moz@)]

my suggestion:
first, look in $profile/dictionaries
then, $app/dictionaries
then, $app/components/dictionaries.
the last is only to not break current .xpi dictionary packages.

Comment 3

[timeless]

mvl: i'd like to plug gre/dictionaries.

if i have 20 gecko apps, i don't want 20 dictionaries.

Comment 4

[Benjamin Smedberg]

OK, couple of questions:

1) are these dictionaries openoffice-compatible?
2) are these dictionaries the openoffice dictionaries?
3) can we make this a "system" function (for example, install to
/usr/lib/dictionaries and/or ~/.dictionaties)

Comment 5

[Michiel van Leeuwen (email: mvl+moz@)]

(In reply to comment #4)
> 1) are these dictionaries openoffice-compatible?
Yes.

> 2) are these dictionaries the openoffice dictionaries?
In most cases, yes. 

> 3) can we make this a "system" function (for example, install to
> /usr/lib/dictionaries and/or ~/.dictionaties)

On my redhat defora box, the openoffice dict are in
/usr/lib/openoffice/share/dict/ooo/
I dont know if openoffice looks somewhere else. I think we could do without a
system location. Just the profile or the mozilla app dir. If a user don't wants
to copy, there are symlinks. (too bad windows don't know about that. at least
not in a way accessible to the regular user) 

Comment 6

[Robert Strong (they/them - no direct email)]

Last I looked at the code there were comments to the affect that the location
should be set via a pref... wouldn't that suffice instead of hardcoding one
location as is done currently or hardcoding multiple locations as is being
suggested? Even if there are hardcoded locations a pref would still provide value.

Comment 7

[Michiel van Leeuwen (email: mvl+moz@)]

bsmedberg: do you have ideas on this? What is an easy place to put the
spellcheck files to be able to install them using the EM? (I think install.js
can make most locations work when installing in seamonkey)
(I really want to get this fixed, so that i can install extensions somewhere in
my profile)

Comment 8

[Benjamin Smedberg]

I think we should use a list of directories
(nsIDirectoryServiceProvider2.getFiles) like we do for plugins, chrome
manifests, etc.

The problem with this has always been Seamonkey's lack of a seamonkey-only
dirserviceprovider, and my insistence that we shouldn't burden
nsAppDirectoryServiceDefs with additional stuff.

nsXREDirServiceProvider can easily provide what we need, and I even can think of
a way to make it app-only (need to file a bug on that).

Comment 9

[Stewart Gordon]

A further benefit (at least on Mac OS X, maybe others) is that dictionaries will
remain installed when Mozilla is upgraded.

Comment 10

[Toralf Lund]

Any progress on this? I thoungt I might mention that I want this, too, and
unlike the poster of comment #5 I think we quite defintely ought to look in a
"system" location (/usr/share/dict) or similar. Dictionaries should obviously be
installed in such a way that the may be shared by applications and users. User
profile install is generally bad, IMO; "global" per-application is slightly
better, but its quite annoying that you have to install the same dictionaries
all over again to support another application using the same format. Sharing via
symlinks is slightly better, and it's actually what I do now, but this also
requires work that ought to be unneccessary, and I very much consider it a hack.

Another problem is that Mozilla expects files in a slightly different format
from what the OOo install normally uses, so you can't just symlink the directroy. 

Comment 11

[Toralf Lund]

(In reply to comment #10)

> 
> Another problem is that Mozilla expects files in a slightly different format
> from what the OOo install normally uses, so you can't just symlink the directroy. 
That should be file *names* of a different format. Mozilla uses
<language>-<country> or sometimes just <language> in the "root" of the file
name, while most OOo dictionaries have <language>_<country> - although the name
is actually configurable. In other words, the Mozilla wordlist file for British
English is en-GB.dic, while in the OOo dictionary directory it's called
en_GB.dic. For Norwegian Bokmål, the names are nb.dic and nb_NO.dic, respectively.

Comment 12

[Robert Strong (they/them - no direct email)]

*** Bug 301166 has been marked as a duplicate of this bug. ***

Comment 13

[Asa Dotzler [:asa]]

not gonna block on this but if someone comes up with a patch, please nominate.

Comment 14

[Paul Tomlin]

See bug 313240 comment 1, bug 313240 comment 4,  bug 313240 comment 11
Also bug 272440 

This should be a blocker. 

Either make it possible for people, who do not have permission to write into the
application directory, to install dictionaries [into their profile] or disable
the 'install new dictionary' UI. Consider the implication of disabling the UI
before thinking that's a viable option though.

Above all, do not *lie* to the user by saying installation was successful when
it clearly wasn't. 

Simply ignoring this is not an option.

Comment 15

[u63580]

Any chance that bug 225468, bug 267390 and bug 216382 can be rationalised to one bug?

Comment 16

[Jo Hermans]

*** Bug 325111 has been marked as a duplicate of this bug. ***

Comment 17

[Benjamin Smedberg]

*** Bug 332412 has been marked as a duplicate of this bug. ***

Comment 18

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

*** Bug 335331 has been marked as a duplicate of this bug. ***

Comment 19

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

*** Bug 338072 has been marked as a duplicate of this bug. ***

Comment 20

[Mike Schroepfer]

Brett any thoughts on this r.e. blocking for 1.8.1?

Comment 21

[Brett Wilson]

I talked to cbeard a little bit about dictionary install. I think we really need to change it. I think we really should fix this for 2.0.

Comment 22

[Benjamin Smedberg]

I can do this next week if nobody gets to it sooner: from an inspection of the code it doesn't actually look that hard (and we'll get the ability to ship dictionaries in extensions as an easy bonus).

Comment 23

[Mike Connor [:mconnor]]

Ok, over to Benjamin then.

Comment 24

[Benjamin Smedberg]

Attachment: Use directoryservice, rev. 1

Brett, let me know if you won't be able to look at this in the next few days.

Comment 25

[Neil Deakin]

Comment on attachment 227279 [details] [diff] [review]
Use directoryservice, rev. 1

>+  // SetDictionary can be called multiple times, so we might have a
>+  // valid mMySpell instance which needs cleaned up.
>+  if (mMySpell)
>+    delete mMySpell;
> 

no need to null check here.

+  if (aResult)
+    NS_ADDREF(*aResult = mNext);
+

NS_IF_ADDREF?

Comment 26

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

You missed a file when relanding this, causing me crashes due to an uninitialized nsBaseHastable<>.  I just relanded the factory changes.

Comment 27

[Jim Jeffery not reading bug-mail 1/2/11]

It appears that these changes have completly broken spell check in today's build:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060629 Minefield/3.0a1,Firefox ID:2006062904 [cairo]

Comment 28

[Nick Thomas [:nthomas] (UTC+12)]

(In reply to comment #27)
> It appears that these changes have completly broken spell check in today's
> build:
> 
> Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060629
> Minefield/3.0a1,Firefox ID:2006062904 [cairo]

There don't seem to be any changes to the various package files in the patch that landed.

Comment 29

[Benjamin Smedberg]

Fixed on trunk.

Comment 30

[Benjamin Smedberg]

Attachment: 1.8 branch merge, rev. 1

This is a consolidated branch patch with the regression fixes from bug 343126 fixed.

Comment 31

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 227573 [details] [diff] [review]
1.8 branch merge, rev. 1

approved as per 181drivers

Comment 32

[Benjamin Smedberg]

I had to revise the 1.8.1 checkin because 1.8.1 doesn't have nsUnicharPtrHashKey, so I switched to use nsStringHashKey

Comment 33

[Ed Davies]

I was about to report three separate bugs against Thunderbird 1.5.0.8 spell checking but, as this is being/has been worked on, I'll just note them here as I'm not sure if they'll have been resolved by  the patch.  Comments are for Ubuntu Linux at least though I assume they apply much more widely.

1. Installation of downloaded dictionary appears to have worked (dialog box says so) but actually it hasn't worked (dictionary doesn't show in Edit->Preferences->Composition->Spelling->Language drop down list).  In my case, of course, it failed because I was running as a non-root user but Thunderbird was installed as the root.

This point has been noted in comment #14 by Paul Tomlin.  I'd like to second his comment that saying that the installation was successful when it clearly wasn't is not an option.  The lack of a report of the failed installation is, in my opinion, much more serious than the root cause of the failure.

2. Using "sudo thunderbird" allows the installation but sets only read permission for root on the dictionaries.  Something like "sudo chmod a+r en-GB*" in the components/myspell directory is required to make them usable to non-root users.

3. If dictionaries are present but not readable (as per 2) then no warning is given - instead all words are reported as incorrectly spelt.