Closed
Bug 364035
Opened 18 years ago
Closed 17 years ago
Permit loading external spellcheck engine from libspellcheck
Categories
(Core :: Spelling checker, enhancement)
Core
Spelling checker
Tracking
()
RESOLVED
FIXED
mozilla1.9beta1
People
(Reporter: andris.pavenis, Assigned: andris.pavenis)
References
Details
(Keywords: dev-doc-complete)
Attachments
(1 file, 1 obsolete file)
7.10 KB,
patch
|
brettw
:
review+
mscott
:
superreview+
damons
:
approval1.9+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20061215 Minefield/3.0a1
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20061215 Minefield/3.0a1
I suggest and improvement of spell-check manager to permit loading external spell-check engine from extension. Modified version scans available contract ids for regular expression /.*\/spellcheck\/engine\/[^\/]+;1/ and if found interprets them as contract ids of additional spell-check engines.
Such feature would be useful for at least Finnish language where reasonable quality dictionary for MySpell is not available. For Finnish language there are other spell-check engines not based on MySpell, ispell (see http://hunspell-fi.org/).
Reproducible: Always
Expected Results:
Would like to have a possibility to use more than one spell-check engine from Mozilla programs.
Assignee | ||
Comment 1•18 years ago
|
||
Initial version of patch is tested with Firefox CVS versions (latest 2006/12/15).
Updated•18 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Product: Mozilla Application Suite → Core
Version: unspecified → Trunk
Updated•18 years ago
|
Component: General → Spelling checker
Updated•18 years ago
|
Assignee: general → mscott
QA Contact: general → spelling-checker
Comment 2•18 years ago
|
||
Are you going to ask for review on the patch? I'm not sure who the best reviewer would be, perhaps mscott or brettw.
Hardware: PC → All
Assignee | ||
Comment 3•18 years ago
|
||
Yes. Review would be nice.
Comment 4•18 years ago
|
||
I don't feel qualified to say whether your approach with the contract IDs is correct since I'm a little fuzzy on best practices there.
I think your method of generating dictionary names is wrong. You definitely can't have hardcoded strings for names like you did.
If I recall (I could be wrong), the dictionary name returned by the spellchecker is used as a lookup into a localized string bundle. So it will return "de" which will turn into "German" if you have an English Firefox, and "Deutsch" if you have a German one. Generating complex names in the spellchecker will break this system.
Localization with the spellchecker is especially nasty because it is used in both Firefox web content and in UI elements for random XUL. I remember having problems with this, because some strings are in toolkit used by XUL textbox code, and others are in Firefox used by it's frontend (browser.js).
Maybe you can invent a new scheme for naming so that the frontend can separate out the localized dictionary name and the engine name, or maybe just don't support different engines with the same language? I'm not sure about the proper approach here. You might get some good feedback by posting on mozilla.dev.apps.firefox.
Sorry if I'm not very helpful, I don't really work on the spellchecker any more aside from pontificating on the occasional bug.
Assignee | ||
Comment 5•18 years ago
|
||
About finding spell-check engine using contract ISs: that is best I figured out. I'm open to other suggestions. But I don't think that specifying predefined contract IDs could be a solution. Similar modification should permit adding spell-check engine from an extension, so we need some way how to look for installed engines. Any other suggestions?
About supporting more than 1 spell-check directory for the same language: current patch takes only the first one found with that name. MySpell is intentionally at the end of the search list, to enable to install external spell-check engine for the same language over the default one. Of course when 2 external spell-checkers support the same language with the same directory name, we cannot predict which will be chosen. (The code for eliminating duplicate directory names is not the fastest possible, but of course one can improve it)
PS. One more spell-check engine additionally to MySpell is installed as extension and working when writing this comment (Minefield 3.0a3pre 20070224, Linux) including switching between them.
Assignee | ||
Comment 6•18 years ago
|
||
How about the description of external spell-check engine in a separate file. Perhaps only real thing that we need to put there is external spell-check engine contract id.
For example putting file foo.engine in extensions dictionaries directory could tell libspellcheck to read it and to try to interpret as description of spell-check engine. So we could add external spell-checkers without the need to compare all contract its with some predefined pattern.
Could it be that better approach rather than the way used in earlier patch?
Comment 7•17 years ago
|
||
I'm not sure of the details, but I think it's possible to create your own implementation of a contract ID in an extension and override the old one. You then proxy calls that you don't want to handle through to the old one.
I'm not actually clear on the details of how to do this or whether this would be the most appropriate design. There may also be issues with the spellchecker that prevent this somehow. If it does work, you should be able to do everything only in an extension. I stopped working on the spellchecker after Firefox 2, and unfortunately it seems nobody has taken over. You are probably more likely to get help writing such an extension (try IRC) than getting somebody to reply with a policy decision on this (in my response above, I replied because I knew it was wrong, but I don't know what is right).
Assignee | ||
Comment 8•17 years ago
|
||
I don't think that overriding contract ids will help. I can replace MySpell also without such overriding, but if I'll do then I can only have one of MySpell or external spellcheck engine. I don't think to possible to go much further without modifying spellcheck manager.
I'll try to implement registration of spellcheck engine using nsICategoryMenager interface and provide a new patch when I'll have it. Does it seem a better way than my old patch?
Assignee | ||
Comment 9•17 years ago
|
||
This updated version of patch for external spellcheck engine support. It uses nsICategoryManager to retrieve information about installed spell-check engines. Extension with external spellchecker registers entry in category 'spell-check-engine' with key name equal to engine contractID. Entry value is not used and may be arbitrary. If there is duplicate dictionary names, then only the first is used. HunSpell is always the last in search order, so extension can override dictionary provided by HunSpell.
Attachment #248815 -
Attachment is obsolete: true
Assignee | ||
Updated•17 years ago
|
Attachment #277270 -
Flags: review?(mscott)
Comment 10•17 years ago
|
||
Comment on attachment 277270 [details] [diff] [review]
Updated patch for external spellcheck engine support.
This patch basically looks OK to me. I think you need SR for this to go in.
Attachment #277270 -
Flags: review?(mscott) → review+
Assignee | ||
Comment 11•17 years ago
|
||
Who could be possible super-reviewer?
PS. To be more sure that all is OK, tested additionally on Linux x86_64 (Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9a8pre) Gecko/2007090819 Minefield/3.0a8pre). Both English language language spell-check through HunSpell and Finnish language spell-check using installed extension work OK. No spell-check related problems were detected by valgrind.
Comment 12•17 years ago
|
||
Probably mscott? Maybe you should post on the newsgroup to ask. I'm not sure Scott checks bugmail.
Comment 13•17 years ago
|
||
Comment on attachment 277270 [details] [diff] [review]
Updated patch for external spellcheck engine support.
looks fine to me too.
Attachment #277270 -
Flags: superreview+
Assignee | ||
Updated•17 years ago
|
Attachment #277270 -
Flags: approval1.9?
Updated•17 years ago
|
Assignee: mscott → andris.pavenis
Updated•17 years ago
|
Status: NEW → ASSIGNED
Updated•17 years ago
|
Attachment #277270 -
Flags: approval1.9? → approval1.9+
Comment 14•17 years ago
|
||
Checking in extensions/spellcheck/src/mozSpellChecker.cpp;
/cvsroot/mozilla/extensions/spellcheck/src/mozSpellChecker.cpp,v <-- mozSpellChecker.cpp
new revision: 1.14; previous revision: 1.13
done
Checking in extensions/spellcheck/src/mozSpellChecker.h;
/cvsroot/mozilla/extensions/spellcheck/src/mozSpellChecker.h,v <-- mozSpellChecker.h
new revision: 1.5; previous revision: 1.4
done
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9 M9
Comment 15•17 years ago
|
||
As a general point of curiosity, pavenis, has Hunspell improved your experience with Finnish spellchecking?
Assignee | ||
Comment 16•17 years ago
|
||
About use of Hunspell for Finnish language
Initially there was a project for developing Finnsh language spell-checking support for Hunspell. It was however dropped after it was found that already more full different open source (GPL) implementation of Finnish spell-checker (Voikko) was in development. See
http://voikko.sourceforge.net/
for details (part of pages is in Finnish though).
Comment 17•17 years ago
|
||
any documentatition/howto on writing external spellchecking engines?
Assignee | ||
Comment 18•17 years ago
|
||
Not yet unfortunately.
I thought about suggesting it myself.
I would however like to have reasonably useful test example for that:
Full source of working extension which uses aspell as spell-checker or something similar could be a good example. Unfortunately I don't have such now.
Currently I only have extension which uses Finnish language spell-checker Voikko and it requires Voikko to be installed separately, which could be a problem for many users. Packing Voikko inside extension requires more code (not yet written). Additionally Finnish language spell-checker is perhaps not of much use for most people outside Finland.
Currently I could attach tar.bz2 archive of current extension sources as example to this bug with short comments unless there are objections (extension uses Mozilla build system). Extension will not really work unless Voikko is installed, but will not break anything when loaded. but at least sources could be an example how I have done that. Later perhaps Mozilla WIKI could be a better place for more detailed documentation.
Updated•16 years ago
|
Keywords: dev-doc-needed
Assignee | ||
Comment 19•16 years ago
|
||
What would be expected form of documentation?
- text or HTML file in mozilla/extensions/spellchecker/? (for example EXTERNAL-SPELLCHEKERS.txt)
- something in WIKI?
Comment 20•16 years ago
|
||
I think documenting it in the wiki would be more helpful.
Comment 21•16 years ago
|
||
Any progress on the documentation?
Comment 22•16 years ago
|
||
Is there anyone who's actually familiar with how to do this who could either add something to MDC or provide me with notes and an example so I can write it up? I'd like to get this documented ASAP.
Thanks!
Comment 23•16 years ago
|
||
If someone can at least throw some basic notes at http://developer.mozilla.org/en/Loading_external_spell_check_engines I can clean it up.
Assignee | ||
Comment 24•16 years ago
|
||
I noticed already the previous note. I have put some preliminary information at
https://developer.mozilla.org/En/Using_an_External_Spell-checker
(I read the last message after the page was added therefore the name is different. I can change it if needed).
Some things I which I would to add additionally:
- code example for registration and unregistration callback
- the best would perhaps a demo extension which recognizes only some words from the list but I do not however know whether I'll have time for it
The information already added includes link to the sources (on Sourceforge) of an extension which uses a real external spell checking engine.
Comment 25•16 years ago
|
||
I've done initial cleanup work on the article Andris posted:
https://developer.mozilla.org/En/Using_an_External_Spell-checker
It still could use some sample code, but the existing link to a project that does this is good enough IMHO to tag this as dev-doc-complete; I've marked the article as needing an example.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•