Closed Bug 472795 Opened 15 years ago Closed 15 years ago

Updater should use brand name string and not hardcode Thunderbird

Categories

(Thunderbird :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0b2

People

(Reporter: robert.strong.bugs, Assigned: philor)

Details

Attachments

(1 file)

1.40 KB, patch
standard8
: review+
robert.strong.bugs
: review+
Details | Diff | Splinter Review
With the landing of the patch in bug 399153 this is now very simple to fix as
can be seen in the patch in bug 324758 that was checked in.
Attached patch Fix v.1Splinter Review
rs: just looking for an rs+ from you that I'm not confused, and that this is really expected to work for comm-central+mozilla1.9.1, and Firefox isn't doing it for 3.1 only because it missed string freeze there.
Assignee: nobody → philringnalda
Status: NEW → ASSIGNED
Attachment #356300 - Flags: review?(robert.bugzilla)
Attachment #356300 - Flags: review?(bugzilla)
Attachment #356300 - Flags: review?(robert.bugzilla) → review+
Comment on attachment 356300 [details] [diff] [review]
Fix v.1

This will work with 1.9.1 as well since I landed the patch that made this possible there. I'm not going to do the same for Firefox on 1.9.1 due to the string freeze.

btw: I was considering just defining the command in the makefile.in as follows:

ifdef MOZ_UPDATER
CAT_UPDATER_INI = cat $< $(srcdir)/updater_append.ini
ifeq ($(OS_ARCH),WINNT)
CAT_UPDATER_INI += $(srcdir)/../installer/windows/nsis/updater_append.ini
endif
CAT_UPDATER_INI += | \
                   sed -e "s/%MOZ_APP_DISPLAYNAME%/$(MOZ_APP_DISPLAYNAME)/" | \
                   sed -e "s/%AB_CD%/$(AB_CD)/" > $(FINAL_TARGET)/updater.ini

libs:: $(addprefix $(LOCALE_SRCDIR)/,updater/updater.ini)
	$(CAT_UPDATER_INI)
endif


not sure if it is really any cleaner though
Not to my eyes, anyway: the repetition's limited enough, and close enough, that it doesn't worry me as much as having to assemble it all in my head (though if you do change Fx that way somewhere down the line, we'd probably port it just to stay in sync).
(In reply to comment #3)
> Not to my eyes
Seriously, thank you for saying that. I do appreciate the input. :)
Target Milestone: --- → Thunderbird 3.0b2
Should we be having a localization note in here to state that if localizers change brandShortName then they may wish to drop the %MOZ_APP_DISPLAYNAME% in that string? If they don't use it, then they can just put %MOZ_APP_DISPLAYNAME% where they like.
(In reply to comment #5)
> Should we be having a localization note in here to state that if localizers
> change brandShortName then they may wish to drop the %MOZ_APP_DISPLAYNAME% in
> that string? If they don't use it, then they can just put %MOZ_APP_DISPLAYNAME%
> where they like.
The readstrings implementation relies on line numbers for the first four lines so if you do add a localization note keep that in mind.

btw: as time permits I will try to come up with a decent fix for the readstrings hack.
Attachment #356300 - Flags: review?(bugzilla) → review+
Heh, I'd never actually looked at readstrings: "this is a hack!" indeed :)

Compared to the undocumented "whatever's after an = on the third line of the file is the title, fourth is the text" not having a note for MOZ_APP_DISPLAYNAME seems pretty tame.
(In reply to comment #7)
> Heh, I'd never actually looked at readstrings: "this is a hack!" indeed :)
> 
> Compared to the undocumented "whatever's after an = on the third line of the
> file is the title, fourth is the text" not having a note for
> MOZ_APP_DISPLAYNAME seems pretty tame.
btw: a fix for the worst parts of the hack is in bug 473417
http://hg.mozilla.org/comm-central/rev/9fb7643249d9
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.