848830 - App Updater dialog not localized in Nightly and Aurora channel (25+)

Status: VERIFIED FIXED

(Toolkit :: Application Update, defect)

Description

[Vlado Valastiak [:wladow]]

The App Updater dialog window (%MOZ_APP_DISPLAYNAME% is installing your updates and will start in a few moments…) is not localized in Windows L10n Nightly builds. The most likely applies to all toolkit apps, tested with latest L10n nightly builds of Firefox, Thunderbird and Seamonkey using Win7 64bit. Seen for past few days, not sure when this appeared though.

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20130306 Firefox/22.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20100101 Thunderbird/22.0a1
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20100101 Firefox/22.0 SeaMonkey/2.19a1

Comment 1

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

I just looked at a couple of latest l10n m-c builds and they are using the en-US strings which have %MOZ_APP_DISPLAYNAME% replaced.

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-central-l10n/firefox-22.0a1.af.win32.installer.exe

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-central-l10n/firefox-22.0a1.sk.win32.installer.exe

Please provide a link to a Firefox installer that exhibits this issue. Thanks!

Comment 2

[Vlado Valastiak [:wladow]]

And that's the bug. The message is not localized, it is using en-us strings instead of localized ones.

Comment 3

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

Ahhh... I thought %MOZ_APP_DISPLAYNAME% wasn't getting replaced during build.

This is a build problem when building each app's updater.ini which happens at
http://mxr.mozilla.org/mozilla-central/source/browser/locales/Makefile.in

This is most likely due to bug 780561.

Comment 5

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

glandium, I *think* this regressed due to bug 780561. Could you take a look?

Comment 6

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

Or it might be the compare-locales change Axel suggests in bug 865001.

Comment 7

[Mike Hommey [:glandium]]

It's possible that adding updater.ini in the NON_CHROME list in toolkit/mozapps/installer/l10n-repack.py would fix it.

Comment 8

[Mike Hommey [:glandium]]

Attachment: Add updater.ini to the list of localized files for repacks

Can someone test if this fixes it?

Comment 9

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

(In reply to Mike Hommey [:glandium] from comment #8)
> Created attachment 777479 [details] [diff] [review]
> Add updater.ini to the list of localized files for repacks
> 
> Can someone test if this fixes it?
I might be able to find some time tomorrow if no one else does.

Comment 10

[Axel Hecht [:Pike]]

Comment on attachment 777479 [details] [diff] [review]
Add updater.ini to the list of localized files for repacks

tested on linux, and updater.ini ends up completely missing.

ahecht@ubuntu:~/src/mozilla-central/obj-repack-fx/dist$ tar -jtf firefox-25.0a1.fr.linux-x86_64.tar.bz2 |grep updater
firefox/icons/updater.png
firefox/updater
ahecht@ubuntu:~/src/mozilla-central/obj-repack-fx/dist$ tar -jtf firefox-25.0a1.en-US.linux-x86_64.tar.bz2 |grep updater
firefox/updater
firefox/icons/updater.png
firefox/updater.ini


Without this patch, I had the en-US original

Comment 11

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

This was caused by bug 787180.

http://hg.mozilla.org/mozilla-central/diff/8279af77dc28/browser/locales/Makefile.in

Comment 12

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

Attachment: bug848830

Not sure if this is acceptable given the changes in bug 787180 but this fixes it for me.

Axel, if Mike is ok with this could you verify it does the right thing for you?

Comment 13

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

Attachment: Fix rev2

Some garbage crept in.

Comment 14

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

Note: it looks like crashreporter-override.ini might be placed in the wrong directory /browser/crashreporter-override.ini as well but if so, that would be a separate bug.

Comment 15

[Mike Hommey [:glandium]]

(In reply to Robert Strong [:rstrong] (do not email) from comment #14)
> Note: it looks like crashreporter-override.ini might be placed in the wrong
> directory /browser/crashreporter-override.ini as well but if so, that would
> be a separate bug.

toolkit/xre/nsAppRunner.cpp reads it from the app directory, which makes it at its right place in browser/.

Comment 16

[Mike Hommey [:glandium]]

Comment on attachment 778114 [details] [diff] [review]
Fix rev2

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

::: browser/locales/Makefile.in
@@ +204,5 @@
>  ifeq ($(OS_ARCH),WINNT)
>  	cat $< $(srcdir)/../installer/windows/nsis/updater_append.ini | \
>  	  sed -e "s/^InfoText=/Info=/" -e "s/^TitleText=/Title=/" | \
>  	  sed -e "s/%MOZ_APP_DISPLAYNAME%/$(MOZ_APP_DISPLAYNAME)/" > \
> +	  $(FINAL_TARGET)/../updater.ini

Kind of horrible, but should work. (and I can't think of something better)

Comment 17

[Axel Hecht [:Pike]]

Comment on attachment 778114 [details] [diff] [review]
Fix rev2

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

Sadly, this doesn't work. I don't have a lot of time to dig into it, but we need partly the fix in l10n_repack.py.

But right now, the FINAL_TARGET includes browser, for reasons I haven't found beyond the fact that it's apparently in DIST_SUBDIR. Not sure why that's there, and what we should do about it.

I'm also confused that the file's in bin in the Makefile, but not in bin afterwards.

Which means, I got to the point to get a localized updater.ini into the package, but it ends up being in firefox/browser/bin/updater.ini instead of firefox/updater.ini.

Comment 18

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

Mike / Axel, I'll try to figure out a solution over the weekend.

Axel, what commands are you using to test?

Comment 19

[Axel Hecht [:Pike]]

I'm using http://hg.mozilla.org/l10n-central/fr/ as test case. I don't bother about l10n-merge, as I don't intend to run the build.

I configure a regular build with 
ac_add_options --with-l10n-base=parent-of-fr

... then make -C config to get nsinstall

and cd browser/locales

make installers-fr

Comment 20

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

Interesting... That is what I did except using my existing en-US build and the de locale and it worked for me. I'll double check some things but I think we'd like to go with something cleaner anyways.

Comment 21

[Axel Hecht [:Pike]]

Rob, any updates on this one?

Comment 22

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

Axel, not yet. I also used the same steps as you did in comment #19 with the patch and it worked for me. Do you see where things are going wrong when you repackage with the patch?

Comment 23

[Axel Hecht [:Pike]]

The thing confusing me most is the DIST_SUBDIR stuff.

I got an email about prefs in the langpack .xpi being in browser/... as well.

I just don't have any idea where DIST_SUBDIR would come from.

Comment 24

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

(In reply to Axel Hecht [:Pike] from comment #23)
> The thing confusing me most is the DIST_SUBDIR stuff.
> 
> I got an email about prefs in the langpack .xpi being in browser/... as well.
> 
> I just don't have any idea where DIST_SUBDIR would come from.
It isn't used by the updater.ini as far as I can see though $(DIST)/bin/ was being used. Perhaps the makefile wasn't updated with the patch on your build? For example, I've had to run make first after applying a patch to a makefile.

Comment 25

[Mike Hommey [:glandium]]

What are prefs used for in langpacks?

Comment 27

[Axel Hecht [:Pike]]

Attachment: bug-848830-localize-updater-wip.patch

OK, I gave this another whirl, but only on mac. Seems that with both the FINAL_TARGET and a line in l10n-repack.py, I get the updater.ini localized in dmgs.

This is really a trial-and-error patch, so I'm more putting this out there than claiming I had a solution.

Comment 28

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

Comment on attachment 788909 [details] [diff] [review]
bug-848830-localize-updater-wip.patch

That looks fine to me though I haven't looked at how l10n-repack.py works

Comment 29

[Axel Hecht [:Pike]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/47f6f2ff7f58

Comment 30

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/47f6f2ff7f58

Comment 31

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

Verified using Nightly using the ar locale on Windows, Linux, and Mac OS X so this should be good to go if you'd like to uplift to Aurora or Beta.

Comment 32

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

I also verified that the ar locale for the 8/10 nightly build exhibited this bug prior to verifying the fix.

Comment 33

[Axel Hecht [:Pike]]

Comment on attachment 788909 [details] [diff] [review]
bug-848830-localize-updater-wip.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: No localization of updater dialog
Testing completed (on m-c, etc.): Verified on trunk
Risk to taking this patch (and alternatives if risky): 
String or IDL/UUID changes made by this patch: None

Comment 34

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7e0472b0e361

Comment 35

[Simona Badau, Desktop QA]

Mozilla/5.0 (Windows NT 6.1; rv:25.0) Gecko/20100101 Firefox/25.0
Build ID: 20131001024718

Verified as fixed on Firefox 25 beta 4 - the App Updater dialog is localized for each of the following l10n builds: ru, tr, es-ES, and de. I verified by updating from Firefox 25 beta 1 to Firefox 25 beta 4 while app.update.staging.enabled was set to false (to slow things down in order to get the updater dialog).

If anyone thinks I should verify the fix also on other l10n builds, please let me know.

Comment 36

[Simona Badau, Desktop QA]

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0
Build ID: 20131004004003

Verified as fixed on Firefox 26.0a2 - the App Updater dialog is localized for each of the following l10n builds: ru, tr, ar an ro.