The default bug view has changed. See this FAQ.

Chosen dictionary intermittently resets from en-GB to en-US after session restart

RESOLVED FIXED in Firefox 43

Status

()

Core
Spelling checker
RESOLVED FIXED
5 years ago
a year ago

People

(Reporter: emorley, Assigned: Jorg K (GMT+1))

Tracking

(Blocks: 1 bug)

Trunk
mozilla43
x86
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox43 fixed)

Details

Attachments

(3 attachments, 3 obsolete attachments)

Created attachment 587852 [details]
about:support

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0a1) Gecko/20120111 Firefox/12.0a1
See attachment for about:support dump.

Installed dictionaries: Only en-GB (British English Dictionary addon v1.19.1)

I haven't been able to get it to occur consistently, but STR along the lines of:
1) In a gmail app-tab start new draft email
2) Type a word that has a different spelling in the US eg neighbour
3) Spot that the word is marked as misspelt, check context menu - find it is set to US English
4) Set back to UK English
5) Finish email + send, return to gmail inbox
6) Carry on browsing, restart session at some point.

Expected:
Once set to en-GB, preference is remembered even after Nightly restart.

Actual:
~10-20% of the time the pref is reset back to en-US after session restart.

Comment 1

5 years ago
I'm seeing the same thing.
Ed, did you get a chance to debug this?
I will look into this.
Sorry for no rely so far - I was having great trouble identifying when I'd successfully repeated the STR, since the change only seemed to become apparent on Firefox restart. Inspection of prefs.js after app exit, showed the pref had changed from en-GB to en-US - so I decided to annotate parts of mozHunspell::SetDictionary in a local debug build to be able to see in the console when I'd successfully recreated the STR, but then struggled to get it to do so in my local build :-( 

I also tried doing what ehsan suggested and setting a breakpoint on mozHunspell::SetDictionary, but my lack of RAM/ancient CPU makes MSVC debugging pretty much unusable :-(( 
(even ignoring the incessant assertions on startup and on gmail, eg bug 714398 - how people put up with debug builds is beyond me - though maybe things are less annoying on an i7 or something)
I couldn't reproduce this with the latest nightly builds. Paul, are you able to get this still?

Comment 6

5 years ago
I'm seeing something similar -- a switch from en-CA to fr-BE, happening over and over again.  Using 12.0~b3+build1-0ubuntu0.12.04.1~mfn2 (so, an ubuntu-built package) on ubuntu precise.  inspection of prefs.js shows 
user_pref("spellchecker.dictionary", "fr_BE");

But about:config shows:
spellchecker.dictionary;en_CA

not sure what the issue is...

Updated

3 years ago
Blocks: 1073827

Comment 7

2 years ago
Seeing similar problem in latest 64 bit nightly. 38.0a1
STR:
Change dictionary to English Australia. 
Shut browser down
Restart Browser 
Dictionary has reverted to en-us

Comment 8

2 years ago
I also continue to see this problem, with current FDE and also with Firefox Beta 36.0b2 (both on Arch linux)
(Assignee)

Updated

2 years ago
See Also: → bug 1200533
(Assignee)

Comment 9

2 years ago
This was analysed in bug 1200533. The problem is that en-AU or en-GB should be stored as so-called "content preference", however, these only get stored if the dictionary is set to another language. So setting de-DE or es-ES on an "en" site sticks, setting en-AU and en-GB is explicitly discarded. Code here:
https://dxr.mozilla.org/mozilla-central/source/editor/composer/nsEditorSpellCheck.cpp#616
(Assignee)

Comment 10

2 years ago
Created attachment 8655814 [details] [diff] [review]
Proposed solution

(In reply to Ehsan Akhgari (don't ask for review please) from comment #2)
> Ed, did you get a chance to debug this?

I'm not Ed, but yes, this has been analysed.

Perhaps, despite Ehsan's general veto to touch this code, we can land a small fix here that affects many English speakers who prefer "en-GB" or "en-AU" over "en-US".
Attachment #8655814 - Flags: review?(roc)
Comment on attachment 8655814 [details] [diff] [review]
Proposed solution

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

I agree
Attachment #8655814 - Flags: review?(roc) → review+
(Assignee)

Comment 12

2 years ago
(In reply to Ehsan Akhgari (don't ask for review please) from comment #2)
> Ed, did you get a chance to debug this?
Assignee: nobody → mozilla
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 13

2 years ago
Jorg, does the patch also cover the case where some other non-"en" language is propagated to the default (via the preference), causing it to pop up on all windows with neither content pref or lang attribute? E.g. in the situation described in the reproduction, where you also have the set of "de" dictionaries installed, and visit a "de" site?

(There's also the fact that those sites pick a “random” de, usually de-at, even if I tell them I want de-de — but this I think should be covered by the patch)
(Assignee)

Comment 14

2 years ago
The patch covers exactly the problem described. A specific en-XX choice gets reverted to en-US. Of course similar examples exist for de-XX being reverted to de-YY.

When you first visit an "en" site, an en-XX dictionary will be chosen at random, equally for a "de" site, perhaps de-AT. If you changed this, before this user selection was discarded, now it will stick. 

The other thing you're complaining about, if I understand it correctly, is not part of the bug.

The following problem still persists:
Have two windows, window 1 and window 2. In window 2 show a site that has no "lang" attribute and where no content preference has been set. So this site uses a fallback, typically from the preference "spellchecker.dictionary" (or if not set, from the locale).

Now in window 1, change the language. This will currently always set "spellchecker.dictionary". If you now return to window 2, the language will change to the one set in window 1 based on the preference.

This is (sadly) established behaviour. Changing this behaviour was subject to bug 1073840(*), but that has met too much opposition. I am hoping to fix up the fallback behaviour a little in bug 1200533, but the latter will maintain the established behaviour.

(*) The aim of bug 1073840 was *not* to change "spellchecker.dictionary" every time a dictionary was set elsewhere.

Comment 15

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/c11d024e51fd
Keywords: checkin-needed
Can you please add a test for this?  test_add_remove_dictionaries.xul shows you how you can install dictionaries in a test and helper_bug1170484.js used in test_bug1170484.html shows you how to click on the spell checking context menu to change the language (helper_bug1170484.js shows how to pick a spelling suggestion from the context menu which should be similar.)
(Assignee)

Comment 17

2 years ago
I can, but only if do you a (kind) review of bug 1200533 for me (keeping in mind that the first part of the patch already landed here) ;-)

Since it's doable to add dictionaries while testing, bug 1200533 should be fully testable.
Comment hidden (obsolete)
Comment hidden (obsolete)
Comment hidden (obsolete)
(Assignee)

Updated

2 years ago
Keywords: leave-open
I really don't have enough bandwidth these days.  Please ask roc.  He should have not r+'ed without a test.  ;-)
Flags: needinfo?(ehsan)
https://hg.mozilla.org/mozilla-central/rev/c11d024e51fd
(Assignee)

Comment 23

2 years ago
Created attachment 8656164 [details] [diff] [review]
Test for the patch (v1)

Here's the test so we don't get into trouble with Ehsan ;-)

Limited try run here:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=130f54e04540
(no idea how I managed to push 19 changesets, I intended to push two, see whether it works).

I should say that I copied this from test_bug678842.html and test_add_remove_dictionaries.xul.
So if it's not right, I blame it on the original author(s) ;-)
Attachment #8656061 - Attachment is obsolete: true
Attachment #8656164 - Flags: review?(roc)

Comment 24

2 years ago
I don't know if this is even the right bug to answer your question, but I'm getting kind of confused with all the activity, thanks a lot by the way for wading into this.

(In reply to Jorg K (GMT+2) from comment #14)
> The other thing you're complaining about, if I understand it correctly, is
> not part of the bug.
> 
> The following problem still persists:
> Have two windows, window 1 and window 2. In window 2 show a site that has no
> "lang" attribute and where no content preference has been set. So this site
> uses a fallback, typically from the preference "spellchecker.dictionary" (or
> if not set, from the locale).
> 
> Now in window 1, change the language. This will currently always set
> "spellchecker.dictionary". If you now return to window 2, the language will
> change to the one set in window 1 based on the preference.

Almost, but not quite. Let me try to put it into these terms.

- In window 1, visit a site with no lang attribute and no content preference. It uses the fallback, which, let's just assume, will be the correct dictionary.
- In window 2, visit a site with a "lang" attribute of "de". This sets "spellchecker.dictionary" to "de". Both windows at this point have the correct dictionary set (I think).
- Restart the browser (with "Show my windows and tabs from last time").
- Now both windows have some "de-XX" dictionary selected.
- Bonus: if I then select "en-gb" on window 1, and restart the browser again, window 1 will be back to "de-XX".

Can probably also be replicated by closing window 1 and opening the same site again on window 3. I'm not willing to test right now (I have a dirty work-around in place) but I can if you say it would help.
(Assignee)

Comment 25

2 years ago
This bug is not a good place to discuss anything any more, the fix has landed, and as soon as the test lands, this will be marked fixed.

Also, your description is sadly wrong:
Window 1 will use a fallback, most likely the last value of "spellchecker.dictionary".
If in window 2 you visit a site with lang="de", this WILL NOT set the "spellchecker.dictionary".

"spellchecker.dictionary" will only be set when you actively select another dictionary via the right-click (context) menu. As detailed before, this can affect all other windows that are based on "spellchecker.dictionary". The "spellchecker.dictionary" is always set to a full dictionary name, never the language code ("de") alone. This behaviour won't change.

My remaining fixes are in bug 1200533. However, what you're describing is not covered there, so there would also not be the right place to comment.

If you want to be heard, please give a 100% reproducible description of what you are experiencing and put it into the meta-bug 1073827. From there, we can allocate it to the right bug.

I repeat: The problem must be 100% reproducible, you should remove/rename your content-prefs.sqlite before you start, or start on a new profile (start Firefox with the -p option). Please specify all the dictionaries you have installed and all the websites you visit.

Comment 26

2 years ago
Thanks.

My list of installed dictionaries won't change any time soon, so with a fresh profile, I can reproduce this easily at any point.

To avoid creating redundant work for you, what I'll do is I'll wait for your batch of fixes to land, then build that tree, and see if it still happens. I have a feeling it won't, because what I'm seeing might be a side-effect of one of the things you're fixing. If it does happen, I'll give all the detail I can produce.
(Assignee)

Comment 27

2 years ago
The fix has landed (comment #22). Of course you can use a Nightly build in a day or so.
Sadly my try server build was for Linux only.
(Assignee)

Comment 28

2 years ago
Created attachment 8656246 [details] [diff] [review]
Test for the patch (v2)

Need to remove en-GB dictionary after use.
If not, later tests use it and fail, since it's a fake dictionary with nothing in it.

New try run (just for the "oth" test that failed before):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9d83581b40d5
Attachment #8656164 - Attachment is obsolete: true
Attachment #8656164 - Flags: review?(roc)
Attachment #8656246 - Flags: review?(roc)
Attachment #8656246 - Flags: review?(roc) → review+
(Assignee)

Comment 29

2 years ago
Oops, got the try run wrong, it's -o for "other", not -oth.
So here we go again with: try: -b o -p linux64 -u mochitest-o[x64] -t none
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cd977eb0180f
(Assignee)

Comment 30

2 years ago
Try run green, so all good.

Dear Sheriff:

Please check in the second patch "Test for the patch (v2)".
The first part was already checked here: (comment #22)
https://hg.mozilla.org/mozilla-central/rev/c11d024e51fd

Sorry about the extra hassle.
Keywords: leave-open → checkin-needed

Comment 31

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/3ecb75931636
Keywords: checkin-needed
Thanks for the test!
(Assignee)

Comment 33

2 years ago
And there will be many more tests in bug 1200533 ;-)

Updated

2 years ago
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox43: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
https://hg.mozilla.org/mozilla-central/rev/3ecb75931636
(Assignee)

Updated

2 years ago
Duplicate of this bug: 959785
Thank you for fixing this Jorg! :-)
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1206556
(Assignee)

Updated

a year ago
Duplicate of this bug: 1061650
(Assignee)

Updated

a year ago
Duplicate of this bug: 720052
You need to log in before you can comment on or make changes to this bug.