501596 - Problem with Serbian localization when click on "check for updates"

Status: RESOLVED FIXED

(Mozilla Localizations :: sr / Serbian, defect)

Description

[Saša Petrović]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; sr; rv:1.9.1) Gecko/20090624 Firefox/3.5 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; sr; rv:1.9.1) Gecko/20090624 Firefox/3.5 (.NET CLR 3.5.30729)

With Serbian translation, when I click on Check for updates, I get a window with folowing

Грешка у рашчлањавању ИксЕмЕла (XML): неодређени унос
Место: chrome://mozapps/content/update/updates.xul
Ред 46, колона 52:    <label id="noUpdatesAutoEnabled" hidden="true">&noupdatesautoenabled.intro;</label>
---------------------------------------------------^

and no update check. 
This was problem in all betas and RCs

Reproducible: Always

Steps to Reproduce:
1.Firefox 3.5 in Serbian
2.Помоћ > Провера доградњи (on English "check for updates menu in help menu"



Expected Results:  
I should open update checking box.

Comment 1

[u88484]

Just pasting this here for whoever looks at this bug to save them some time

http://mxr.mozilla.org/l10n-mozilla1.9.1/source/sr/toolkit/chrome/mozapps/update/updates.dtd#10

<!ENTITY noupdatesautoenabled.intro "Нема нових доградњи.  &brandShorName; ће периодични проверавати доградње.">

Comment 2

[u88484]

Sasa, could you please confirm if this happens in Firefox's safe mode?

Comment 3

[Saša Petrović]

(In reply to comment #2)
> Sasa, could you please confirm if this happens in Firefox's safe mode?

Same problem in Safe mode

Comment 4

[Milos Dinic [:Milos]]

Same for me. Marking this as new, `cos it`s confirmed.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Axel, this looks really identical to bug 498753 for [mk]. :( Should we stop offering updates for this locale and remove the installers from the ftp server to avoid that more people are affected?

Comment 6

[Pascal Chevrel:pascalc]

the entity is  missing a T &brandShorName; should be &brandShortName;

Comment 7

[Pascal Chevrel:pascalc]

Should we as an emergency measure remove Serbian from product-details and put a redirect to en-US from the Serbian download page?

Comment 8

[Pascal Chevrel:pascalc]

After talking to Seth on the phone, I updated product-details to remove Serbian and redirected the mozilla.com/sr/ page to en-US.

r29254 , r29253 and then r29257 (to remove completely Serbian and Macedonian from the main dl box)

Comment 9

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

Henrik, thanks for the heads up. As comment #1 and comment #6 noted this is due to updates.dtd using &brandShorName which is incorrect vs. &brandShortName.
http://mxr.mozilla.org/l10n-mozilla1.9.1/source/sr/toolkit/chrome/mozapps/update/updates.dtd#10

Comment 10

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

(In reply to comment #0)
> With Serbian translation, when I click on Check for updates, ...

It's the same for background update checks if Firefox is set to "Ask me what we want to do", and I'd expect the same for background check + prompt when ready to apply.

> This was problem in all betas and RCs

Please file a bug as soon as you notice a problem, thanks.

In fact, the bogus entity was introduced in 
 http://hg.mozilla.org/releases/l10n-mozilla-1.9.1/sr/rev/76811180f829
and is present in b3 onwards.

I'll work on pulling the major update path from 3.0.11 to 3.5 final.

Comment 11

[Filip Miletic]

Ok, I am back home from work.  Let's patch this...

Please stand by until a patch arrives.

Comment 12

[Filip Miletic]

Attachment: Fixes the bogus entity.

Comment 13

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

(In reply to comment #10)
> In fact, the bogus entity was introduced in 
>  http://hg.mozilla.org/releases/l10n-mozilla-1.9.1/sr/rev/76811180f829
> and is present in b3 onwards.
Except sr only shipped from b5 onwards.

I pulled (just like mk in bug 501344)
* Builds from the releases dir
* live 3.0.11 snippets for beta/release channels, and betatest/releasetest (backed up to aus-staging:~cltbld/3.0.11-3.5-mu-pulled-sr/)

Comment 14

[Filip Miletic]

Hi.  The fix is in, can someone r+ it?

What is the plan to bring Serbian ff back to downloads?

Comment 15

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

Comment on attachment 386435 [details] [diff] [review]
Fixes the bogus entity.

There's no error in Fx3.0, see
  http://mxr.mozilla.org/l10n/source/sr/toolkit/chrome/mozapps/update/updates.dtd#13 
so the approval1.9.0.1x flags aren't appropriate. You'd want the (non-existent) approval-1.9.1.1, setting r? on Axel instead.

Comment 16

[Filip Miletic]

FWIW, I tested the patch on my local FF 3.5 and it seems to work well.

Comment 17

[Tony Chung [:tchung]]

Can we confirm no other locales see the same problem?

Comment 18

[Filip Miletic]

(In reply to comment #17)
> Can we confirm no other locales see the same problem?

Not sure what you mean.  This is a fix only for the Serbian locale.

Comment 19

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

(In reply to comment #17)
> Can we confirm no other locales see the same problem?
Looks like he mistyped &brandShortName for Thunderbird as well
http://mxr.mozilla.org/l10n-central/source/he/mail/chrome/messenger/accountCreation.dtd#39

As for other mistyping of &brandShortName or other entities it appears that l10n doesn't have any scripts in place to try to detect these types of mistakes.

Comment 20

[Staś Małolepszy :stas]

Comment on attachment 386435 [details] [diff] [review]
Fixes the bogus entity.

r=me.

We actually don't need the review for things like this [1] now that we're on hg. Filip, please land this fix on the 1.9.1 branch and l10n-central and let us know (for example here in this bug) about the landing and the new changeset from which we should build. We will then update the l10n-changesets accordingly.

([1] Note that any changes to region.properties or searchengines still require a review)

Comment 21

[Friedel Wolff]

(In reply to comment #19)
> As for other mistyping of &brandShortName or other entities it appears that
> l10n doesn't have any scripts in place to try to detect these types of
> mistakes.

Filip, if you want to avoid these issues, have a look at pofilter from the Translate Toolkit:
http://translate.sourceforge.net/wiki/toolkit/pofilter

The l10n infrastructure shouldn't allow a translation to break the product, but while it does, this QA tool should find all issues that can break the product. It would have avoided this and the Macedonian issue.

The critical tests with  pofilter --mozilla  are printf, variables and xmltags.
Reviewing these tests should catch stuff like this. (XML entities are tested as part of the "variables" test.)

Some of the other tests are more for cosmetic things, and there could be some
false positives. It might be good to review them anyway.

So to test:
(assuming sr_po contains all your PO files of the sr l10n files)
pofilter --mozilla --test=printf --test=variables --test=xmltags  sr_po
sr_filter

sr_filter/ will contain the suspicious translations to review.

Comment 22

[Filip Miletic]

(In reply to comment #20)
> (From update of attachment 386435 [details] [diff] [review])
> r=me.
> 
> We actually don't need the review for things like this [1] now that we're on
> hg. Filip, please land this fix on the 1.9.1 branch and l10n-central and let us
> know (for example here in this bug) about the landing and the new changeset
> from which we should build. We will then update the l10n-changesets
> accordingly.
> 

Landed as: 878a83718455 for 1.9.1
> ([1] Note that any changes to region.properties or searchengines still require
> a review)

Comment 23

[Filip Miletic]

(In reply to comment #21)
> Filip, if you want to avoid these issues, have a look at pofilter from the
> Translate Toolkit:
> http://translate.sourceforge.net/wiki/toolkit/pofilter
> [...] Reviewing these tests should catch stuff like this. (XML entities are tested as
> part of the "variables" test.)

Friedel, thanks for the tip.

I tried it immediately, as I don't really want my translations to contain critical errors that go undetected into the release.

However, I also don't think the output of the program is useful in its current form.  

At least for me, it generates a lot of files buried in subdirs, which must be checked manually.  This is about as painful and error-prone as hand-checking the original files.  The check must be fully automated, and either give a green light, or a red light as output.

You could make pofilter output more useful if you split the 'QA testing' functionality of pofilter (which produces files for manual review), and the 'unit test' pofilter, which looks for problems, and if it finds one, stops with an error, pointing at the problem text.  

The latter would be a very useful addition to my moz2po-based workflow, and I think others would find it useful too.

Comment 24

[Milos Dinic [:Milos]]

I have made Firefox 3.5(sr) available for download from my server, intended for downloads from Serbian Community site. So, can you predict when will this push happen, so I can publish appropriate notification for all our users?

Comment 25

[Axel Hecht [:Pike]]

Milos, please pull those ASAP. Really.

You're just confusing people with builds that they can't update.

Comment 26

[Milos Dinic [:Milos]]

(In reply to comment #25)
> Milos, please pull those ASAP. Really.
> 
> You're just confusing people with builds that they can't update.

Don`t worry, I did. I`ve left download image, `cos I`m afraid that I may discourage users to use new version, but I`ve created JScript alert onlclick to say that downloads are disabled temporarily and that we`ll try to fix this soon.

Comment 27

[Milos Dinic [:Milos]]

Sorry for bugging, but I got many complaints from users not being able to download localized Firefox 3.5 (Serbian). As I can see here, this patch is good, so we only wait for a push to happen. Can we at least have an estimate on when will push happen, please?

Comment 28

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

It is most unusual to redo a locale after a release has shipped, and we're trying to decide what the best way forward is. There is no estimate at this point.

Comment 29

[Samuel Sidler (old account; do not CC)]

Did this fix land on the Serbian 1.9.1 branch?

Comment 30

[Filip Miletic]

(In reply to comment #29)
> Did this fix land on the Serbian 1.9.1 branch?
See:
https://bugzilla.mozilla.org/show_bug.cgi?id=501596#c22

Comment 31

[Milos Dinic [:Milos]]

This works for me.