343931 - Set app name in InfoPlist.strings during build based on --enable-official-branding

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[(not reading, please use [e-mail] instead)]

have a branded version of InfoPlist.strings under other-licenses

suggested by robert.

when you have --enable-official-branding on mac, we should override dist/Firefox.app/Contents/Resources/English.lproj/InfoPlist.strings (which normally comes from 
mozilla/browser/app/macbuild/Contents/Resources/English.lproj/InfoPlist.strings) with a Firefox branded one from somewhere under mozilla/other-licenses.

see bug #343825 for more info about why this would be good.

Note, this file is UTF16 and contains something like:

  1 /* Localized versions of Info.plist keys */
  2 
  3 CFBundleName = "Bon Echo";
  4 NSHumanReadableCopyright = "Copyright © 1998 - 2006 Contributors.";

Note, the tbird version is not UTF16, so it is lxr'able and editable with simple editors.  (See http://lxr.mozilla.org/mozilla1.8/source/mail/app/macbuild/Contents/Resources/English.lproj/InfoPlist.strings)

Does anyone know why the firefox one is UTF16?

Comment 1

[(not reading, please use [e-mail] instead)]

worth fixing for b2, to prevent future pain.

Comment 2

[Mark Mentovai]

The file is supposed to be UTF-16, although UTF-8 usually works.  Translators of apps that go through the traditional OS X localization process get upset when .strings files are UTF-8 because Apple's localization tools insist that they be UTF-16.

What we should do is have InfoPlist.strings.in, in UTF-8, and let sed replace a token with $MOZ_APP_DISPLAYNAME, and pipe its output into iconv to produce a UTF-16 InfoPlist.strings.  That way, we won't need a branded version in other-licenses, we can let the right file with the right content and right encoding be generated dynamcially.

Then we should think about doing the same thing with the copyright year, to eliminate the depth of the annual "happy new year" changes.

Finally, get rid of that damn period in NSHumanReadableCopyright.

Comment 3

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

I think Mark has the right approach

Comment 4

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

Attachment: patch - untested

Seth - could you verify this does the right thing. I didn't bother with including the cvs remove of the old plist file.

Comment 5

[(not reading, please use [e-mail] instead)]

> Seth - could you verify this does the right thing.

testing now...

Comment 6

[(not reading, please use [e-mail] instead)]

I tested it, and this patch works for me.

robert and I were wondering about:

NSHumanReadableCopyright = "Copyright © 1998 - 2006 Contributors.";

a) where does the copyright item appear? How can you see it?
b) Is it convention to not have the "." at the end?

Comment 7

[Mark Mentovai]

NSHumanReadableCopyright is "required," but it's only used by the default Cocoa about box.  We don't use the default Cocoa about box, even when we're a Cocoa app.  But because Apple calls the key "required," we should supply something rational anyway.

And get rid of the period.

And I'd prefer to see APP_NAME enclosed in some @ guards to make it clear that we do substitution.

And I'd prefer to see the transformation done in a pipeline, avoiding the need for unnecessary temporary files.

+	sed -e "s/@APP_NAME@/$(APP_NAME)/" $(srcdir)/macbuild/Contents/Resources/English.lproj/InfoPlist.strings.in | iconv -f UTF-8 -t UTF-16 > $(DIST)/$(APP_NAME).app/Contents/Resources/$(AB).lproj/InfoPlist.strings

Comment 8

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

Mark, would you be willing to fix this bug... I'd like to simplify the change from nightly to the official build process but I don't have a Mac currently and would prefer not to introduce inconsistencies between the Mac OS X build files or to go through the files to make them consistent.

Comment 9

[Mark Mentovai]

Attachment: Patch for Firefox and Thunderbird, tested

Comment 10

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

Mark, changing the product removed the review request you had set

Comment 11

[Mark Mentovai]

Comment on attachment 229855 [details] [diff] [review]
Patch for Firefox and Thunderbird, tested

Doh!  I hate it when that happens.

Comment 12

[Mark Mentovai]

Checked in on the trunk.

Comment 13

[Mark Mentovai]

Comment on attachment 229855 [details] [diff] [review]
Patch for Firefox and Thunderbird, tested

This is a build config-only change that eases the transitions back and forth between BonEcho and Firefox.

Comment 14

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 229855 [details] [diff] [review]
Patch for Firefox and Thunderbird, tested

a=drivers, please land on the 181branch

Comment 15

[Mark Mentovai]

Checked in on MOZILLA_1_8_BRANCH for 1.8.1b2.

Comment 16

[Robert Kaiser]

Hmm, this looks nice... We should probably do the same for SeaMonkey...

Comment 17

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

I'm seeing this in my Firefox trunk build:

...
creating security/manager/locales/Makefile
creating browser/Makefile
Unknown variable:browser/app/Makefile:367:	sed -e "s/@APP_VERSION@/$(APP_VERSION)/" -e "s/@APP_NAME@/$(APP_NAME)/" $(srcdir)/macbuild/Contents/Info.plist.in > $(DIST)/$(APP_NAME).app/Contents/Info.plist

This is in the long list of makefiles that are created by configure. Looking at today's tinderbox logs for Firefox & Thunderbird, Trunk and 1.8 branch, I see much the same.

Is the escaping of the @ used in bug 345388 needed here too ? 

Also, where would I check in the Mac Firefox 2.0b2 RC builds that the app name and version are being set correctly ?

Comment 18

[Mark Mentovai]

Yes, this is the escaping thing.