Closed Bug 848830 Opened 7 years ago Closed 6 years ago

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

Categories

(Toolkit :: Application Update, defect)

x86_64
Windows 7
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla26
Tracking Status
firefox24 --- unaffected
firefox25 + verified
firefox26 + verified

People

(Reporter: wladow, Assigned: Pike)

References

Details

(Keywords: regression)

Attachments

(2 files, 2 obsolete files)

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
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!
And that's the bug. The message is not localized, it is using en-us strings instead of localized ones.
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.
Blocks: new-packager
Keywords: regression
Duplicate of this bug: 865001
glandium, I *think* this regressed due to bug 780561. Could you take a look?
Flags: needinfo?(mh+mozilla)
Or it might be the compare-locales change Axel suggests in bug 865001.
It's possible that adding updater.ini in the NON_CHROME list in toolkit/mozapps/installer/l10n-repack.py would fix it.
Flags: needinfo?(mh+mozilla)
Can someone test if this fixes it?
Assignee: nobody → mh+mozilla
(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 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
Attachment #777479 - Flags: feedback-
Attached patch bug848830 (obsolete) — Splinter Review
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?
Attachment #777479 - Attachment is obsolete: true
Attachment #778110 - Flags: review?(mh+mozilla)
Attached patch Fix rev2Splinter Review
Some garbage crept in.
Attachment #778110 - Attachment is obsolete: true
Attachment #778110 - Flags: review?(mh+mozilla)
Attachment #778114 - Flags: review?(mh+mozilla)
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.
(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 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)
Attachment #778114 - Flags: review?(mh+mozilla) → review+
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.
Attachment #778114 - Flags: review?(l10n) → review-
Mike / Axel, I'll try to figure out a solution over the weekend.

Axel, what commands are you using to test?
Assignee: mh+mozilla → robert.bugzilla
Flags: needinfo?(l10n)
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
Flags: needinfo?(l10n)
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.
Rob, any updates on this one?
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?
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.
(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.
What are prefs used for in langpacks?
Duplicate of this bug: 898620
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.
Attachment #788909 - Flags: review?(robert.bugzilla)
Attachment #788909 - Flags: review?(mh+mozilla)
Summary: App Updater dialog not localized in Nightly channel → App Updater dialog not localized in Nightly and Aurora channel (25+)
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
Attachment #788909 - Flags: review?(robert.bugzilla) → review+
Attachment #788909 - Flags: review?(mh+mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/47f6f2ff7f58
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
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.
Status: RESOLVED → VERIFIED
I also verified that the ar locale for the 8/10 nightly build exhibited this bug prior to verifying the fix.
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
Attachment #788909 - Flags: approval-mozilla-aurora?
Attachment #788909 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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.
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.
You need to log in before you can comment on or make changes to this bug.