Error reading font prefs in the Slovenian locale

VERIFIED FIXED

Status

defect
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: peter_klofutar, Assigned: aleth)

Tracking

({late-l10n, regression})

Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird38+ fixed)

Details

Attachments

(1 attachment, 6 obsolete attachments)

Posted image Chat window with active chat.PNG (obsolete) —
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:36.0) Gecko/20100101 Firefox/36.0
Build ID: 20150305021524

Steps to reproduce:

Installed Thunderbird Aurora 38


Actual results:

When starting a chat with a contact, the bottom bar for entering text doesn't appear.


Expected results:

Box should have appeared, so entering text is possible.  The problems appears in Slovenian locale.  I tested English-US and chat worked fine.
The screenshot you show is when you're clicked on a group, which we wouldn't expect to have the box there. If you double click on a contact do you see the conversation box show up?

(I can't read the text there so I'm unsure exactly what state the chat window is in.)
When I double click on a contact, I get a bar on the bottom, that says "Start a conversation with <contact>".  When I click on it, the text box doesn't show up.
Can you take a look in the error console and see if something relevant appears there when you click the bar?
I get the following errors:

Časovni žig: 9.3.2015 8:56:15
Napaka: NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.getIntPref]
Izvorna datoteka: resource://gre/modules/imTextboxUtils.jsm
Vrstica: 33

Časovni žig: 9.3.2015 8:56:17
Napaka: TypeError: item.convView is null
Izvorna datoteka: chrome://messenger/content/chat/chat-messenger-overlay.js
Vrstica: 536
OK, so it fails trying to fetch the appropriate font.size.variable.* pref.

What's the value of the font.language.group pref in this locale?
Flags: needinfo?(peter_klofutar)
Also, is this a regression, i.e. did this work in a previous version in the Slovenian locale?
The value of that pref. is x-central-euro.  This worked in Aurora 36, problems started, when it updated to 37.
Flags: needinfo?(peter_klofutar)
Keywords: regression
Summary: Chat is not working → Error reading font prefs in the Slovenian locale
That's strange, font.size.variable.x-central-euro should be set by default.
Can you check the value of that pref? 
Does the problem happen with all protocols or just one (XMPP, IRC, ...)?

What's puzzling me is that it's not clear to me why that pref value should be accessed at all in TB.
(In reply to aleth [:aleth] from comment #8)
> That's strange, font.size.variable.x-central-euro should be set by default.

It is not set for me either (can't be seen via TB's config editor).

And Options->Display->Formatting->Default font field is empty (and also the size). I think that is reported somewhere else. The Preferences dialog allows me to set up the fonts and sizes (also the serif and monospace are missing). And then the pref font.size.variable.x-central-euro is set to the non-default value I have chosen.
(In reply to :aceman from comment #10)
> (In reply to aleth [:aleth] from comment #8)
> > That's strange, font.size.variable.x-central-euro should be set by default.
> 
> It is not set for me either (can't be seen via TB's config editor).
> 
> And Options->Display->Formatting->Default font field is empty (and also the
> size). I think that is reported somewhere else. The Preferences dialog
> allows me to set up the fonts and sizes (also the serif and monospace are
> missing). And then the pref font.size.variable.x-central-euro is set to the
> non-default value I have chosen.

So it looks like the problem is that the default values are currently only set for OSX: 

https://dxr.mozilla.org/comm-central/source/mail/app/profile/all-thunderbird.js#590

And x-central-euro doesn't seem to exist at all for Firefox.

Do you know someone knowledgeable about l10n/fonts who would know what the correct fix would be here?
Status: UNCONFIRMED → NEW
Ever confirmed: true
It happens also on IRC.
(In reply to peter_klofutar from comment #12)
> It happens also on IRC.

Right, since comment 11 it's clear why it's happening now, registerTextbox calls getValues in imTextboxUtils and that tries to get the nonexistent default value of the pref.
Can you tell me what value font.language.group has in Slovenian Firefox? (I hope it matches, but it's a bit surprising that Firefox seems to be missing some default values too).
Flags: needinfo?(peter_klofutar)
(In reply to aleth [:aleth] from comment #14)
> Can you tell me what value font.language.group has in Slovenian Firefox? (I
> hope it matches, but it's a bit surprising that Firefox seems to be missing
> some default values too).

In fact x-central-euro no longer exists in Firefox since bug 756022, which was not ported to c-c at the time.
Flags: needinfo?(peter_klofutar)
Bug 756022 removed three language groups, this patch restores the missing font.size prefs as a workaround, which should be enough for the narrow cause of this bug.

Of course that's not really the correct fix, which would be to fully port bug 756022, which is nontrivial in that it's likely easy to miss something unless one is familiar with the way TB handles charsets etc, which I am not. However, there's probably a whole set of other bugs caused by mismatches due to those language groups being removed from gecko but not TB (as hinted at by aceman above). The question is whether it isn't too late to do this for TB38.

Needinfoing rkent to raise awareness of this issue.
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Flags: needinfo?(rkent)
Attachment #8575238 - Flags: review?(mkmelin+mozilla)
(In reply to aleth [:aleth] from comment #14)
> Can you tell me what value font.language.group has in Slovenian Firefox? (I
> hope it matches, but it's a bit surprising that Firefox seems to be missing
> some default values too).
It's the same.
(In reply to peter_klofutar from comment #17)
> (In reply to aleth [:aleth] from comment #14)
> > Can you tell me what value font.language.group has in Slovenian Firefox? (I
> > hope it matches, but it's a bit surprising that Firefox seems to be missing
> > some default values too).
> It's the same.

If this is true for new profiles, that's a bug in the Firefox Slovenian l10n, since https://dxr.mozilla.org/mozilla-central/source/toolkit/locales/en-US/chrome/global/intl.properties#35 clearly states it must be one of the values in https://dxr.mozilla.org/mozilla-central/source/browser/components/preferences/fonts.xul#50 and x-central-euro no longer exists there.
I'm really not the right guy to discuss character set issues. Joshua and Magnus, is this likely to be a serious issue in TB 38? Can we backport a change, or does it affect localization?
Flags: needinfo?(rkent) → needinfo?(Pidgeot18)
Blocks: 756022
(In reply to Kent James (:rkent) from comment #19)
> Can we backport a
> change, or does it affect localization?

https://hg.mozilla.org/mozilla-central/rev/2a9dfb605be4 introduced new strings. Still, c-c really needs to backport this.
The proposed patch could be added to TB 38 with no ramifications AFAICT, so we'll consider taking it when reviewed.
(In reply to aleth [:aleth] from comment #18)
>
> If this is true for new profiles, that's a bug in the Firefox Slovenian
> l10n, since
> https://dxr.mozilla.org/mozilla-central/source/toolkit/locales/en-US/chrome/
> global/intl.properties#35 clearly states it must be one of the values in
> https://dxr.mozilla.org/mozilla-central/source/browser/components/
> preferences/fonts.xul#50 and x-central-euro no longer exists there.

It's the same for new profiles.
(In reply to Kent James (:rkent) from comment #19)
> I'm really not the right guy to discuss character set issues. Joshua and
> Magnus, is this likely to be a serious issue in TB 38? Can we backport a
> change, or does it affect localization?

I think hsivonen answered this question. If they're changing how some of the fonts setting works, we need to change our dialog to conform to those changes, not paper it over with pref changes that will mean nothing internally.
Flags: needinfo?(Pidgeot18)
Sure, let's backport this to c-c, but do we have to add a late string at this point, or can we just accept the pref?
Attachment #8575238 - Attachment description: fontdefaults.diff → Minimal possible temporary fix - adds back some prefs removed from gecko
Attachment #8575238 - Flags: review?(mkmelin+mozilla)
Posted patch Port bug 756022 (obsolete) — Splinter Review
Port of bug 756022.

On its own this is not enough - it's missing the charset aspects of the removal of x-central-euro/x-baltic/tr. I don't know enough about how charsets relate to email encodings to be confident in doing this part.

There's also bug 910192 which should likely be ported at the same time for consistency.

If we take this for 38, we will have to tell localizers about the late strings and the langgroup changes.
Attachment #8574296 - Attachment is obsolete: true
Attachment #8577400 - Flags: review?(Pidgeot18)
(In reply to aleth [:aleth] from comment #25)
> Created attachment 8577400 [details] [diff] [review]
> Port bug 756022
> 
> Port of bug 756022.
> 
> On its own this is not enough - it's missing the charset aspects of the
> removal of x-central-euro/x-baltic/tr. I don't know enough about how
> charsets relate to email encodings to be confident in doing this part.

i.e. is naively replacing the removed groups with x-western in mailnews/intl/charsetData.properties the right thing to do?
Posted patch Port bug 756022 v2 (obsolete) — Splinter Review
On second thoughts, I added the naive changes. I suppose I'm just concerned I'm missing non-obvious consequences of this shift.
Attachment #8577400 - Attachment is obsolete: true
Attachment #8577400 - Flags: review?(Pidgeot18)
Attachment #8577423 - Flags: review?(Pidgeot18)
Posted patch Port bug 756022 v3 (obsolete) — Splinter Review
I looked at porting bug 910192, but discovered that while TB's fonts.xul references the obsolete intl.charset.default, it doesn't actually use it.

So it appears there is nothing to port. Sorry for the bugspam!

Instead of intl.charset.* and the associated Text Encoding groupbox, TB has its own charset prefs. Please check that the recent gecko changes in the linked bugs don't require some sort of parallel changes there.
Attachment #8577423 - Attachment is obsolete: true
Attachment #8577423 - Flags: review?(Pidgeot18)
Attachment #8577433 - Flags: review?(Pidgeot18)
Posted patch Port bug 756022 v4 (obsolete) — Splinter Review
Fix missed entity name change.
Attachment #8577433 - Attachment is obsolete: true
Attachment #8577433 - Flags: review?(Pidgeot18)
Attachment #8577449 - Flags: review?(Pidgeot18)
Typo fix. Sorry, clearly not my best day for patch QA...
Attachment #8577449 - Attachment is obsolete: true
Attachment #8577449 - Flags: review?(Pidgeot18)
Attachment #8577459 - Flags: review?(Pidgeot18)
(In reply to peter_klofutar from comment #22)
> (In reply to aleth [:aleth] from comment #18)
> >
> > If this is true for new profiles, that's a bug in the Firefox Slovenian
> > l10n, since
> > https://dxr.mozilla.org/mozilla-central/source/toolkit/locales/en-US/chrome/
> > global/intl.properties#35 clearly states it must be one of the values in
> > https://dxr.mozilla.org/mozilla-central/source/browser/components/
> > preferences/fonts.xul#50 and x-central-euro no longer exists there.
> 
> It's the same for new profiles.

And indeed, despite this being a change in gecko 34, http://hg.mozilla.org/l10n-central/sk/file/4887034a4291/toolkit/chrome/global/intl.properties#l41 has not been updated.

This might have happened becaue the pref name did not change, so localizers were not aware they had to update this. Hsivonen, it may be worth reaching out to the affected locales?
Flags: needinfo?(hsivonen)
Attachment #8577459 - Flags: review?(Pidgeot18) → review?(mkmelin+mozilla)
Comment on attachment 8577459 [details] [diff] [review]
Port bug 756022 v5

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

Looks good afikt, r=mkmelin
Attachment #8577459 - Flags: review?(mkmelin+mozilla) → review+
OS: Windows 7 → All
Hardware: x86_64 → All
Comment on attachment 8577459 [details] [diff] [review]
Port bug 756022 v5

[Approval Request Comment]
Regression caused by (bug #): 756022
Risk to taking this patch (and alternatives if risky): see earlier discussion in the bug
Attachment #8577459 - Flags: approval-comm-aurora?
Note the bug mentioned in the bug description won't actually be fixed by this patch unless the m-c localizers for the affected locales all fix the font.language.group pref value.

smontagu: As you wrote the original patch, and as the incorrect pref values affect Firefox too and the next merge date is drawing close, I thought I'd better needinfo you about this as well.
Flags: needinfo?(smontagu)
Attachment #8575238 - Attachment is obsolete: true
Duplicate of this bug: 1021900
(In reply to aleth [:aleth] from comment #26)
> > On its own this is not enough - it's missing the charset aspects of the
> > removal of x-central-euro/x-baltic/tr. I don't know enough about how
> > charsets relate to email encodings to be confident in doing this part.
> 
> i.e. is naively replacing the removed groups with x-western in
> mailnews/intl/charsetData.properties the right thing to do?

It seems my central european language also got replaced with "x-western", when I look in FF 36.
(In reply to aleth [:aleth] from comment #26)
> > On its own this is not enough - it's missing the charset aspects of the
> > removal of x-central-euro/x-baltic/tr. I don't know enough about how
> > charsets relate to email encodings to be confident in doing this part.
> 
> i.e. is naively replacing the removed groups with x-western in
> mailnews/intl/charsetData.properties the right thing to do?

That was the goal of bug 756022 -- to merge all the removed groups into x-western
Flags: needinfo?(smontagu)
(In reply to Simon Montagu :smontagu from comment #38)
> (In reply to aleth [:aleth] from comment #26)
> > > On its own this is not enough - it's missing the charset aspects of the
> > > removal of x-central-euro/x-baltic/tr. I don't know enough about how
> > > charsets relate to email encodings to be confident in doing this part.
> > 
> > i.e. is naively replacing the removed groups with x-western in
> > mailnews/intl/charsetData.properties the right thing to do?
> 
> That was the goal of bug 756022 -- to merge all the removed groups into
> x-western

Right, that wasn't why I needinfo'd you though. The problem is that some of the locales affected by this change (e.g. sk, ro, there may be more) never changed the value of the font.language.group pref accordingly. They should be given a heads-up or someone with the requisite privileges should make the change for them.
Flags: needinfo?(smontagu)
Flags: needinfo?(hsivonen)
Depends on: 1146177
Keywords: late-l10n
smontagu: Sorry to bother you again, but I was wondering what the process would be to get the changes made in the bugs you filed in comment #40 uplifted to aurora (38)? This would be very valuable as the next ESR release will be based off 38 (for Thunderbird, which only has ESR releases, but also for Firefox).
Flags: needinfo?(smontagu)
Ah, looking at some of those bugs that have been fixed, it looks like the localizers have already taken care of it :-)
Flags: needinfo?(smontagu)
Peter, could you verify this is fixed next time you get an update of Slovenian TB38, please? Thanks!
Flags: needinfo?(peter_klofutar)
I don't really work on Thunderbird (nor I localize it for my language), but breaking string freeze on the last week of the Aurora cycle is far from acceptable. 

Even more considering that 38 is the ESR release, and some locales will have to stick with these untranslated strings, in a visible part of the UI, for 6 months.
While at it, I suggest implementing a Thunderbird/SeaMonkey equivalent of bug 1147311.
Depends on: 1147526
(In reply to Henri Sivonen (:hsivonen) from comment #46)
> While at it, I suggest implementing a Thunderbird/SeaMonkey equivalent of
> bug 1147311.

Thanks, good idea!
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
(In reply to aleth [:aleth] from comment #44)
> Peter, could you verify this is fixed next time you get an update of
> Slovenian TB38, please? Thanks!

It's working now, thanks (TB 39).
Flags: needinfo?(peter_klofutar)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.