Closed Bug 1402822 Opened 5 years ago Closed 2 months ago

Spellchecker needs to allow multiple language dictionaries at the same time

Categories

(Core :: Spelling checker, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
100 Branch
Tracking Status
relnote-firefox --- 100+
firefox100 --- fixed

People

(Reporter: BenB, Assigned: dminor)

References

(Depends on 2 open bugs, Blocks 4 open bugs, Regressed 4 open bugs)

Details

(Keywords: parity-chrome, Whiteboard: Please, no comments unrelated to implementation)

Attachments

(10 files)

72.85 KB, patch
Details | Diff | Splinter Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
Problem:
I routinely write emails in 3 different languages. They are all mixed: one email in English, another in German, and some in French. In some cases, I even write 2 different languages to the same person, depending on who else is or was involved in the thread, or depending on the subject.

Importance:
The problem arises for all users who speak several languages. E.g. they speak their local language with their friends, and English with international correspondents. In other words, this bug is important for all international communication (unless your native language happens to be English).

Non-solutions:
I cannot specify the language for each email. That costs too much time, and I'll rather ignore the spell-checker altogether.

Solution:
The only solution is to once - as a configuration - select a number of languages that I know how to write, and then to allow all words in these languages.

This is what Android keyboard does as well.

Out of scope:
If we can automatically detect the language that the user writes, that would be nice, but that would be another bug, an enhancement on top of this bug here.

Etiquette:
*****************************************************
*** Comments from end-users are not allowed here. ***
*****************************************************
These should be posted in bug 69687 instead, as they would be distracting here. Thanks for your understanding. This is an engineering implementation bug. But if you are waiting for this feature and want it to be implemented fast, please vote for this bug here.
How does this differ from bug 481884 that you haven't touched since 2014?

(In reply to Ben Bucksch (:BenB) from comment #0)
> If we can automatically detect the language that the user writes, that would
> be nice, but that would be another bug

Bug 1203024

> *** Comments from end-users are not allowed here. ***

You can ask someone with the right privileges to restrict comments.
Flags: needinfo?(ben.bucksch)
Whiteboard: Please, no comments unrelated to implementation
Flags: needinfo?(ben.bucksch)
Duplicate of this bug: 481884
Duplicate of this bug: 557621
Duplicate of this bug: 693831
> Duplicate of this bug: 481884

Is the older bug duplicate newer? Okay...
Depends on: 481884
Duplicate of this bug: 481884
Priority: -- → P3

Chrome has option "All your languages" (combined): https://bugzilla.mozilla.org/attachment.cgi?id=8845416
Chrome puts different language suggestions together: https://bugzilla.mozilla.org/attachment.cgi?id=9075388

So, Chrome fixed exactly this bug here, in the same way as suggested here.

This is arguably the correct solution. I often mix words from other languages in a sentence.
This is also the easiest solution, because you don't need a complicated and error-prone language detection. You simply make the dictionary the super set of all the languages (typically 2 or 3) that the user selected.

Keywords: parity-chrome

My current work-around is adding ~100K of the most common words from my native dictionary to the 'persdict.dat' file (within your Firefox profile folder). Works OK without any noticeable performance penalty, but I (and probably many others) would definitely love to see multi-language spell checking in Firefox. This is a feature Chrome has since 47.

Hello, I'd like to work on this bug. I've already developed a possible solution and would like some feedback.

Can someone assign this bug to me and guide me through the patch process?

Carlo: Happy to do that! I've assigned the bug to you.
In case you stop working on it, without the fix actually landing in the code tree, please unassign the bug again.
If you have a patch, be sure to ask for review while you attach the patch. The dropdrown should normally suggest appropriate reviewers.
I've also sent you a welcome message with some additional help.

Assignee: nobody → vespacarlo
Status: NEW → ASSIGNED

You can ask any question related to patch submission / etc in https://chat.mozilla.org/#/room/#introduction:mozilla.org

There are other links in the topic of that channel which you may find helpful.

Hi, I'd just like to add my vote to please fix this issue. Technically it's a feature request, but it's a feature that is sorely missed by many, many people around the world.

Actually, Chrome's solution to this is quite elegant: the spellcheck language is set not by a radio button, but by check boxes, and apparently, the browser simply "combines" all selected languages when spellchecking (rather than spellchecking only in the 1 selected language).

(In reply to cOgnaut from comment #13)

Actually, Chrome's solution to this is quite elegant: the spellcheck language is set not by a radio button, but by check boxes, and apparently, the browser simply "combines" all selected languages when spellchecking (rather than spellchecking only in the 1 selected language).

This is what I implemented, but I have been really busy recently and don't have the time to actually finish working on it, for now.
Especially because on my macbook it takes ages to compile.

I hope to get some free time to finally finish the patch and post it here for review.

This is what I implemented

Can you share the code, please?

Assignee: vespacarlo → nobody
Status: ASSIGNED → NEW

Here it is. This is after a git pull --rebase.
Unfortunately, mach tells me there's something in my tree that doesn't let it compile, so I don't even know if it compiles now. I would have to checkout everything again from scratch and run a full compile, but it would take hours.

The code worked when I wrote it, the patch was missing a couple of failing tests to be fixed and general polishing / splitting in multiple smaller commits.

The result was that the dictionary selection menu was not a radio group anymore but a multiple select and misspelling suggestions would come out in round robin for each message, because hunspell doesn't return a score to sort by.

(In reply to Carlo from comment #16)
thanks for your contribution Carlo.

Assignee: nobody → dminor
Status: NEW → ASSIGNED
Duplicate of this bug: 616108

🎉
@Dan Minor: Thank you for working on this!

I am looking for this too.

For the most part these are simple updates to account for multiple
dictionaries and the fact that SetDictionaries is async whereas
SetDictionary was sync.

Fixing test_async_UpdateCurrentDictionary was more involved because
there were flaws in the existing test and it is more difficult to
harmonize the UpdateCurrentDictionary callback with the changes to make
SetDictionaries async.

Depends on D140244

Attachment #9266351 - Attachment description: WIP: Bug 1402822 - Support multiple dictionaries to mozISpellCheckingEngine.idl → Bug 1402822 - Support multiple dictionaries in mozISpellCheckingEngine.idl; r=smaug!
Attachment #9266352 - Attachment description: WIP: Bug 1402822 - Support multiple hunspell instances in mozHunspell → Bug 1402822 - Support multiple hunspell instances in mozHunspell; r=smaug!
Attachment #9266353 - Attachment description: WIP: Bug 1402822 - Support multiple dictionaries to mozSpellChecker → Bug 1402822 - Support multiple dictionaries in mozSpellChecker; r=smaug!
Attachment #9266354 - Attachment description: WIP: Bug 1402822 - Support multiple dictionaries in RemoteSpellcheckEngine → Bug 1402822 - Support multiple dictionaries in RemoteSpellcheckEngine; r=smaug!
Attachment #9266355 - Attachment description: WIP: Bug 1402822 - Support multiple dictionaries in nsIEditorSpellCheck → Bug 1402822 - Support multiple dictionaries in nsIEditorSpellCheck; r=smaug!
Attachment #9266356 - Attachment description: WIP: Bug 1402822 - Support multiple dictionaries in EditorSpellCheck → Bug 1402822 - Support multiple dictionaries in EditorSpellCheck; r=smaug!
Attachment #9266357 - Attachment description: WIP: Bug 1402822 - Fix dictionary extension loading code in XPIProvider.jsm → Bug 1402822 - Fix dictionary extension loading code in XPIProvider.jsm; r=smaug!
Attachment #9266358 - Attachment description: WIP: Bug 1402822 - Update unit tests for multiple dictionaries → Bug 1402822 - Update unit tests for multiple dictionaries; r=smaug!
Attachment #9266359 - Attachment description: WIP: Bug 1402822 - Add mochitest for multiple dictionaries → Bug 1402822 - Add mochitest for multiple dictionaries; r=smaug!
Duplicate of this bug: 1759018
Attachment #9266355 - Attachment description: Bug 1402822 - Support multiple dictionaries in nsIEditorSpellCheck; r=smaug! → Bug 1402822 - Support multiple dictionaries in nsIEditorSpellCheck; r=smaug!,gijs!
Attachment #9266355 - Attachment description: Bug 1402822 - Support multiple dictionaries in nsIEditorSpellCheck; r=smaug!,gijs! → Bug 1402822 - Support multiple dictionaries in InlineSpellChecker; r=smaug!,gijs!

There's failures on test-verification with these changes, but there's failures on test-verification on these tests without the changes as well, e.g. https://treeherder.mozilla.org/jobs?repo=try&revision=99fedcc1a058b3377c7def7938d3f27173edaee6 where I added some comments to trigger test-verification on the existing tests.

Fixing https://bugzilla.mozilla.org/show_bug.cgi?id=1760824 should help make the tests less flaky. I've tried to improve them a bit as part of these changes as well.

Pushed by dminor@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/264664a44f49
Support multiple dictionaries in mozISpellCheckingEngine.idl; r=smaug
https://hg.mozilla.org/integration/autoland/rev/1af11d34c213
Support multiple hunspell instances in mozHunspell; r=smaug
https://hg.mozilla.org/integration/autoland/rev/043d88291491
Support multiple dictionaries in mozSpellChecker; r=smaug
https://hg.mozilla.org/integration/autoland/rev/4a3b7ee6a51e
Support multiple dictionaries in RemoteSpellcheckEngine; r=smaug
https://hg.mozilla.org/integration/autoland/rev/74f061f43c77
Support multiple dictionaries in EditorSpellCheck; r=smaug
https://hg.mozilla.org/integration/autoland/rev/338485f1d803
Support multiple dictionaries in InlineSpellChecker; r=smaug,Gijs
https://hg.mozilla.org/integration/autoland/rev/64b43b5b1bba
Fix dictionary extension loading code in XPIProvider.jsm; r=smaug,extension-reviewers,mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/e86c493e43a4
Update unit tests for multiple dictionaries; r=smaug
https://hg.mozilla.org/integration/autoland/rev/465a18abbd0e
Add mochitest for multiple dictionaries; r=smaug
Regressions: 1761085
Regressions: 1761062
Regressions: 1761187
Regressions: 1761160
Regressions: 1761073
Regressions: 1761071
Regressions: 1720297
Regressions: 1761074
Depends on: 1761416
Depends on: 1761417
Depends on: 1761419
Depends on: 1761425
Depends on: 1761433

Release Note Request (optional, but appreciated)
[Why is this notable]: Great new feature for polyglots
[Affects Firefox for Android]: I don't think so?
[Suggested wording]: The spell checker allows multiple language dictionaries at the same time
[Links (documentation, blog post, etc)]:

relnote-firefox: --- → ?
Flags: qe-verify?
Depends on: 1761452
Depends on: 1761458
Depends on: 1761464
Depends on: 1761467
Regressions: 1761114

Has the about:config → spellchecker.dictionary setting been adapted to support multiple dictionaries? In my tests setting it to something comma-separated like "ru,en-US" does nothing.

Regressions: 1761064
Depends on: 1763808
No longer depends on: 1761458
Blocks: 1761273
Severity: normal → --
No longer depends on: 481884
No longer depends on: 1761416
See Also: → 1761415
Blocks: 1761417
No longer depends on: 1761417
No longer depends on: 1761419
Duplicate of this bug: 676500
No longer blocks: 1760896, 1761425, 1761433, 1761467
See Also: → 1760896
Depends on: 1764506
Depends on: 1764508
Depends on: 1764524
See Also: → 1733738
See Also: 1733738

(In reply to Alexander Gavrilov from comment #36)

Has the about:config → spellchecker.dictionary setting been adapted to support multiple dictionaries? In my tests setting it to something comma-separated like "ru,en-US" does nothing.

We first try to match the element or document language and then use this preference. So if we think the input is in en-US, and en-US is in the list of dictionaries in "spellchecker.dictionary", we'll only enable the en-US dictionary. If there's no match on the element or document, then all of the dictionaries in the pref are used.

element or document language ... is ... en-US ..., we'll only enable the en-US dictionary

So, if Gmail or Yahoo or RoundCube document was set to "en-US", that means I can only type English emails? Doesn't that counter the purpose of this bug?

that means I can only type English emails?

You can type in any language, but only English dictionary will be enabled for such pages. You will have to manually enable multiple dictionaries via spellchecker context menu.

When I wrote "can type", I meant of course with the spellchecker working in the language, assuming I have the right dictionary installed, without the spellchecker trying to tell me that all my words are wrong and everything has a red underline, which is quite annoying. (Underline for illustration of the effect.)

It would be quite a pity to do all this work in this bug, and then the spellchecker still assumes English, just because Gmail or Yahoo Mail or RoundCube or whatever web app is in English. How would RoundCube know which language I am writing the email in? I write emails (and text in webapps) in multiple languages, so this logic as stated in comment 38 would render the fix here moot in such situations and keep the spellchecker broken for all languages but one.

I often even mix multiple languages in the same text, for various reasons. In a multi-language context, this is quite common.

Yes, I wrote about this in https://bugzilla.mozilla.org/show_bug.cgi?id=1073827#c40
See also https://bugzilla.mozilla.org/show_bug.cgi?id=1073827#c33

I think the priority of choosing dictionaries should be discussed in that ticket

The default for a site you haven't visited is to match the language of the document or the edit control. If you change the dictionaries manually, they should be remembered for the site, so this should only be a problem the first time you visit a site.

I think it's worth discussing whether we should change this behaviour when the user has explicitly enabled multiple dictionaries at the same time, but I think that would best be done either in a new bug or maybe one of the existing bugs above.

(In reply to Dan Minor [:dminor] from comment #38)

(In reply to Alexander Gavrilov from comment #36)

Has the about:config → spellchecker.dictionary setting been adapted to support multiple dictionaries? In my tests setting it to something comma-separated like "ru,en-US" does nothing.

We first try to match the element or document language and then use this preference. So if we think the input is in en-US, and en-US is in the list of dictionaries in "spellchecker.dictionary", we'll only enable the en-US dictionary. If there's no match on the element or document, then all of the dictionaries in the pref are used.

The thing people are looking for is something like https://imgur.com/a/2JUXlwr where you set a default set of languages (toggle on/off) that the browser checks when doing spellchecking.

Flags: needinfo?(ben.bucksch)
Flags: needinfo?(ben.bucksch)
You need to log in before you can comment on or make changes to this bug.