336029 - "Firefox" is hardcoded in error messages (appstrings.properties)

Status: NEW

(Firefox :: General, defect, P5)

Description

[Adam Guthrie]

Instead of "Minefield" being used, "Firefox" is used instead on some error messages.

Steps to reproduce:
1. Open a new tab and type "arph://"; hit enter;

Results:
"Firefox" is used in the error message.

Expected Results:
"Minefield" is used in the error message.

To my understanding, we could not just use &brandShortName here, so this file would need to be changed to say "Minefield" where it now says "Firefox".

Comment 1

[Mike Kaply [:mkaply]]

The file in question here is:

http://lxr.mozilla.org/seamonkey/source/browser/locales/en-US/chrome/overrides/appstrings.properties

and it has quite a few hardcoded Firefoxes.

Can this be fixed be a processor? Or do we need to change the code to do substitution?

Comment 2

[Jeff Walden [:Waldo]]

So really, I say you either fix the underlying problem (bug 306369 last I remember) or just ignore this.  It's not like it affects end-user releases at all...

Comment 3

[Jeff Walden [:Waldo]]

(where by "ignore" I mean "ignore only if you're tweaking so it says Minefield in Minefield builds", since the fact that there has to be a Firefox override in this way is still a valid bug)

Comment 4

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #2)
> It's not like it affects end-user releases at all...

It affects releases that don't use official branding, and therefore don't want to have any references to "Firefox".

Comment 5

[Jeff Walden [:Waldo]]

Ah, point.  I still think we should fix the real bug, not hack a hack.

Comment 6

[Mike Kaply [:mkaply]]

(In reply to comment #5)
> Ah, point.  I still think we should fix the real bug, not hack a hack.
> 

Is appstring.properties a hack?

Comment 7

[Jeff Walden [:Waldo]]

The problem is that we're completely replacing the string files rather than using the strings anyone else would get.  What *should* happen is that the code that uses the strings should be modified to accept an additional parameter, the name of the application, and then anyone using this could create a branding strings file and get all the good string love with no copying needed.  That's bug 306369, and that's the real bug that should be fixed -- chrome/overrides is the hack to which I referred.

(This doesn't quite address everything due to gender issues with other languages, but I don't see that going away until l20n or something happens, and we've "lived" with the status quo for long enough already anyway, and there are established workarounds for this anyway, like "program Firefox" to get a consistent gender.)

Comment 8

[Mike Kaply [:mkaply]]

These all appear to be handled in one place in the code:

http://lxr.mozilla.org/seamonkey/source/docshell/base/nsDocShell.cpp#2836

Comment 9

[Mike Kaply [:mkaply]]

OK, I'm sorry to be dense here, but I'm not really understanding why this happened in the first place and I can't find history.

appstrings.properties is in the dom dir and Firefox overrides it. I see that.

But appstrings.properties is only used by nsDocShell (as far as I can see)

Why wasn't appstrings.properties just fixed to allow substitution and the code in nsDocShell fixed?

Why was the decision made to override instead of just getting it right in the dom directory?

Comment 10

[Jeff Walden [:Waldo]]

Because it was late in the cycle and this was easier, less time-consuming, and less regression-prone than doing it the right way.

Comment 11

[Benjamin Smedberg]

There is also the recurring issue of embedders wanting things to work without having to have a branding package... which means that we don't *know* the name of the application.

Comment 12

[Mike Kaply [:mkaply]]

There are embedders and there are rebranders.

Do we generally expect embedders to use the netError messages or to use popup messages? I would think that rebranders would use the netError messages and they would need brand substitution.

Note that although an attempt was made to create messages that work in either cases, the error messages are much terser than they were before, so if you turn off XUL errors in Firefox, the error mesages aren't that good.

redirect comes to mind.

Also note that some of the Firefox messages were improved without adding "Firefox" but the dom appstrings.properties was not updated.

Comment 13

[Mike Kaply [:mkaply]]

I'm running across this issue again.

Any thoughts on ever fixing this hack?

It just keeps getting worse as more strings are added to appstrings.properties.

Comment 14

[Justin Dolske [:Dolske]]

(In reply to comment #13)

> It just keeps getting worse as more strings are added to appstrings.properties.

In browser/locales/en-US/chrome/overrides/appstrings.properties?

The "Firefox" strings there haven't been touched since the file was added to Hg, so since Firefox 3.0 (and were probably there too).

Comment 15

[Mike Kaply [:mkaply]]

No, but more strings get added to appstrings.properties that DON'T contain Firefox which seems unnecessary. 

What about splitting into two files? Then at least there are less strings to worry about?

Comment 18

[Mike Hommey [:glandium]]

copy/pasting part of my comment in bug 650274:

A while ago, I attempted to generically fix this using numbered parameters in the strings, as these strings are actually being formatted in the netwerk code, using parameters. For compatibility purpose, I tried to keep the current parameters at the same index, but it turns out that the formatting code doesn't like formatting strings with, e.g. %3$S when there is no %2$S in the string. By the way, it looks like the application name is actually now passed as 3rd parameter, cf. the externalProtocolPrompt string. So maybe a solution would be to fix the formatting functions to allow %3$S when there is no %2$S in the format string, and replace Firefox with %3$S ?

Comment 19

[Axel Hecht [:Pike]]

If we fix the formatting code, we'd need to adjust the check for this in compare-locales, and deploy it. At least on the dashboard, perhaps even on releng.

The way that localizers hack around this when they need to (Japanese never says Firefox), is to use a zero width string. Not that I'd like us doing that here.

Comment 20

[Matt McCutchen]

(In reply to Mike Hommey [:glandium] from comment #18)
> So maybe
> a solution would be to fix the formatting functions to allow %3$S when there
> is no %2$S in the format string, and replace Firefox with %3$S ?

In general, a C sprintf-style function needs to know the data types of the previous parameters in order to traverse the va_list to the requested parameter.  Using unnumbered parameter references as a source of such knowledge would be a valid RFE.  In this setting, one could alternatively hack in a flag for nsStringBundle::FormatString to tell nsTextFormatter::smprintf that all the parameters are strings.

But mixing numbered and unnumbered parameter references makes the format string confusing to read.  Why not switch completely to numbered references in every format string that needs non-left-to-right parameter interpolation?

Comment 21

[Axel Hecht [:Pike]]

We're not talking about numbered vs unnumbered, we're talking about a string like

"%1$S is done with %3$S" vs "%2$S does %1$S with %3$S".

Comment 22

[Mike Hommey [:glandium]]

Another way this could be handled, but that would require much more work, would be to use named parameter references.

Like, for example, in python:
print '%(language)s has %(number)03d quote types.' % {"language": "Python", "number": 2}

The advantage is that the names would allow self-explanatory entries for localizers (currently, we need a comment explaining what %2$S is)

Comment 23

[Mike Kaply [:mkaply]]

I know there was a bug somewhere about allowing NSPR string substitutions to support missing values.

I can't find it.

Comment 24

[Matt McCutchen]

Re comment #21: I see, the trouble is adding a new parameter after one that some locales don't use.  IMO, formatting the undesired intervening parameter at zero width is a reasonable way to clue nsTextFormatter::smprintf in to the parameter type without affecting the output.

The real solution is for nsStringBundle::FormatString, which knows the parameter types, to indicate them somehow to nsTextFormatter::smprintf; this is not something localizers should have to worry about.  Some options:

1. Make a special case for all strings, as mentioned before.
2. Wrap the arguments in some variant type like in Java, Cyclone, and dynamically typed languages.
3. Pass another format string that just indicates the parameter types and is not used for formatting.  The parameter type string would be the subject of -Wformat, merit a louder warning if non-literal, and permit type errors in the format string to be caught at runtime rather than cause segfaults.  This would actually be pretty natural to implement: have dosprintf pass BuildArgArray the parameter type string instead of the format string, and as a separate enhancement add type error checking to dosprintf.

Re comment #22: Agreed with everything you say, but named parameters are really an enhancement orthogonal to what we need here, which is to provide complete parameter type information to nsTextFormatter::smprintf, via either formatting at zero width or an enhanced API.  It would be possible (though most people would judge it silly) to establish parameter names but not types, which indeed would not solve our problem.

Comment 25

[O. Atsushi (Torisugari)]

Attachment: proposal v1

I'm not too sure this can work for l10n tools...

BTW, @MOZ_APP_DISPLAYNAME@ is "Firefox", "Nightly", "Aurora" and so on.

Comment 26

[Mike Kaply [:mkaply]]

We really need to find a way to do this via the normal string routes versus build stuff.

If we use build flags, it can't be overridden via properties files.

Comment 27

[O. Atsushi (Torisugari)]

(In reply to Michael Kaply (mkaply) from comment #26)
> We really need to find a way to do this via the normal string routes versus
> build stuff.
> 
> If we use build flags, it can't be overridden via properties files.

Could you explain the situation you are assuming?

The patch is about the orverriding properties file. So I don't expect that it is going to be overridden.

Comment 28

[Mike Kaply [:mkaply]]

I have clients for which I rebrand Firefox. By changing brand.dtd and brand.properties and placing an add-on in distribution/bundles, these are overridden at startup.

So there is no reason to build Firefox to get these changes.

As it stands today, because appstrings.properties doesn't use brand.properties to get its branding, I have to override all of appstrings.properties in order to get the new values.

I would prefer that appstring.properties properly use brand.properties

See:

https://bugzilla.mozilla.org/show_bug.cgi?id=306369

Comment 29

[O. Atsushi (Torisugari)]

Aha, thank you.

Comment 30

[Mike Hommey [:glandium]]

(In reply to O. Atsushi (Torisugari) from comment #25)
> Created attachment 606001 [details] [diff] [review]
> proposal v1
> 
> I'm not too sure this can work for l10n tools...
> 
> BTW, @MOZ_APP_DISPLAYNAME@ is "Firefox", "Nightly", "Aurora" and so on.

FWIW, I'm doing something similar for Iceweasel, and it works for l10n (except I haven't modified the l10n part yet).
http://anonscm.debian.org/gitweb/?p=pkg-mozilla/iceweasel.git;a=blob;f=debian/patches/iceweasel-branding/Use-MOZ_APP_DISPLAYNAME-to-fill-appstrings.propertie.patch;h=277a35c573c0e28543b300077246f1663dd176e8;hb=b4940c252e1744e328b30737b666e4e73bfff5b7

Comment 31

[:aceman]

Hi, why can't these strings use %S placeholders that get replaced by the &brandShortName; ? Some of the strings already have %S so it seems possible.

Comment 32

[Mike Hommey [:glandium]]

(In reply to :aceman from comment #31)
> Hi, why can't these strings use %S placeholders that get replaced by the
> &brandShortName; ? Some of the strings already have %S so it seems possible.

See comment 18.

Comment 33

[Axel Hecht [:Pike]]

Comment on attachment 606001 [details] [diff] [review]
proposal v1

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

I think we should use params, and I don't see comment 18 to counter that argument. Adding preprocessor variables is likely going to lead to errors.

Also, we'll need to change they keys in the file (and the overloads, IIRC) to have l10n pick up the changed strings.

Comment 34

[O. Atsushi (Torisugari)]

(In reply to Axel Hecht [:Pike] from comment #33)
> I think we should use params, and I don't see comment 18 to counter that
> argument.

More than 99 % of users don't change the browser's branding name, since they install it, till they uninstall it. Under such a situation, "make the value dynamic" is a terrific idea.

And I don't like to call a hack like comment 28 "rebranding", because it can't change the executable names(e.g. "firefox.exe") etc. Browser's branding already heavily depends on build configs. You guys are seriously going to "fix" these problems? How?
http://mxr.mozilla.org/mozilla-central/search?string=MOZ_APP_NAME
http://mxr.mozilla.org/mozilla-central/search?string=MOZ_APP_DISPLAYNAME

On the other hand, we should not use hard-coded string because of actual rebranding.

> Adding preprocessor variables is likely going to lead to errors.
I think we should fix this kind of problems, if any.

> Also, we'll need to change they keys in the file (and the overloads, IIRC)
> to have l10n pick up the changed strings.

Sure.

Comment 35

[Mike Kaply [:mkaply]]

> And I don't like to call a hack like comment 28 "rebranding", because it can't change the executable names(e.g. "firefox.exe") etc. Browser's branding already heavily depends on build configs.

Well that depends on your definition of branding. I can rebrand the entire browser with an add-on and then simply rename the executable. There's absolutely no need to build Firefox.

I can just about 100% rebrand the browser using an add-on.

Any exceptions to that are bugs.

The reason brand.dtd and brand.properties exist are exactly to solve this problem. There should be a way to use them to do the product substitutions in these strings.

Comment 36

[O. Atsushi (Torisugari)]

(In reply to Michael Kaply (mkaply) from comment #35)
> Well that depends on your definition of branding. I can rebrand the entire
> browser with an add-on and then simply rename the executable.

Hmm. I'm surprised to hear that "renaming executable" works in a specific environment. That is not installer versions, and you don't have to worry about Windows DDE name, right? I'm curious how security update is working for wrongly named file in the package, but this is off topic.


> There's absolutely no need to build Firefox.

That's another story, since "my" branding includes the fox icon on the executable files and everything in the branding directory.
http://mxr.mozilla.org/mozilla-central/source/browser/branding/
As long as they are used, the software is branded as "Firefox".

Anyway, the patch won't harm "params" folks at all, for they can keep the discussion on bug 306369, though I think we should remove brand.dtd and brand.properties etc. from the mobile version. I don't mind this bug is a intermediate workaround, i.e. not the complete fix, because I guess the lifetime of bug 306369 would be by far longer than this bug. And after this patch's checkin, what extension authors should do changes nothing.

At this point, what's really worth talking about seems only
> Adding preprocessor variables is likely going to lead to errors.

If this is really serious and we have no way to avoid it, I'm sorry to rebranders and I give up.

Alex?

Comment 37

[Mike Kaply [:mkaply]]

(In reply to O. Atsushi (Torisugari) from comment #36)
> (In reply to Michael Kaply (mkaply) from comment #35)
> > Well that depends on your definition of branding. I can rebrand the entire
> > browser with an add-on and then simply rename the executable.
> 
> Hmm. I'm surprised to hear that "renaming executable" works in a specific
> environment. That is not installer versions, and you don't have to worry
> about Windows DDE name, right? I'm curious how security update is working
> for wrongly named file in the package, but this is off topic.

We don't touch the DDE name, and we start our executable with -no-remote. Also
Also we do our own security updates.

You are correct that security updates would not work if you rename the executable.

> 
> 
> > There's absolutely no need to build Firefox.
> 
> That's another story, since "my" branding includes the fox icon on the
> executable files and everything in the branding directory.
> http://mxr.mozilla.org/mozilla-central/source/browser/branding/
> As long as they are used, the software is branded as "Firefox".

You can replace the icons in executables using tooling.
> 
> Anyway, the patch won't harm "params" folks at all, for they can keep the
> discussion on bug 306369, though I think we should remove brand.dtd and
> brand.properties etc. from the mobile version. I don't mind this bug is a
> intermediate workaround, i.e. not the complete fix, because I guess the
> lifetime of bug 306369 would be by far longer than this bug. And after this
> patch's checkin, what extension authors should do changes nothing.

I still don't completely understand why brand.properties can't be used in this case.
I know there was a reason, but I need to go back and review it.

> At this point, what's really worth talking about seems only
> > Adding preprocessor variables is likely going to lead to errors.
> 
> If this is really serious and we have no way to avoid it, I'm sorry to
> rebranders and I give up.
> 
> Alex?

Comment 38

[O. Atsushi (Torisugari)]

> I still don't completely understand why brand.properties can't be used in
> this case.
> I know there was a reason, but I need to go back and review it.

If I understand right, the branded "appstrings.properties" is overriding the unbranded "appstring.properties". We have to use the same number of variables. Otherwise, it will break either Firefox or XULRunner. That is what comment #18 ～ comment #24 say.

And my answer is that we should not treat the brand as a variable. It's a constant parameter after we ship a product. All we want to do is to change the value before we ship it. Why docshell has to substitute the value at runtime?

Comment 39

[Mike Kaply [:mkaply]]

> And my answer is that we should not treat the brand as a variable. It's a constant parameter after we ship a product.

It's a variable in every other file in the entire product. That's why "Nightly" is called "Nightly" instead of Firefox.

We should try to figure out a clever way to solve the original problem.

Comment 40

[O. Atsushi (Torisugari)]

(In reply to Michael Kaply (mkaply) from comment #39)
> It's a variable in every other file in the entire product.
Actually even you are not changing the brand name after you ship your product, as you showed so far.

> then simply rename the executable.
> You can replace the icons in executables using tooling.
> Also we do our own security updates.

According to these comments, I'm quite sure you do understand the brand name should be a constant value.

> We should try to figure out a clever way to solve the original problem.
That's not a good reason to stop this 6-year-old bug fixed. Please find the way before your next post.

Comment 41

[Mike Kaply [:mkaply]]

There are multiple solutions.

1. Rewrite printf code to accept missing values (non sequential).

2. From comment 19 - The way that localizers hack around this when they need to (Japanese never says Firefox), is to use a zero width string. Not that I'd like us doing that here.

Well that's not the best solution, it's better than doing it a build time.

3. Don't use the brand in these error messages at all. Just use the strings from appstrings.properties.

There's really no reason that

The file %S cannot be found. Please check the location and try again.
needs to be
Firefox can't find the file at %S.

In fact, some of the appstrings.properties message are better.

4. Change appstrings.properties to use a substitution for the browser name and in the generic case, use "The application" or something like that:

Firefox can't find the server at %S.
(The application) can't find the server at $S

Comment 42

[O. Atsushi (Torisugari)]

(In reply to Michael Kaply (mkaply) from comment #41)

[1] is another bug. Nobody here supports [2]. I totally agree about [3] and [4]. But, this "brand name in l10n string" was introduced so as to improve user experiences. These days I guess these decisions would depend on UX team or browser's module owner. When you are to file a new bug, please CC me if that's not filed yet.

Comment 43

[O. Atsushi (Torisugari)]

Attachment: proposal v2

Addressing comment #33

> Also, we'll need to change they keys in the file (and the overloads, IIRC)
> to have l10n pick up the changed strings.

This patch affects even non-changed files (dtd and xhtml) but that's by design.

Comment 44

[Axel Hecht [:Pike]]

Comment on attachment 606854 [details] [diff] [review]
proposal v2

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

r-, preprocessing the l10n files is making the situation worse than it is today. Do use brand.* instead, http://hg.mozilla.org/mozilla-central/rev/ff1e250b7f23 show how to do that, already done that way in appstrings.properties.

Comment 45

[Mike Hommey [:glandium]]

(In reply to Axel Hecht [:Pike] from comment #44)
> Comment on attachment 606854 [details] [diff] [review]
> proposal v2
> 
> Review of attachment 606854 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r-, preprocessing the l10n files is making the situation worse than it is
> today. Do use brand.* instead,
> http://hg.mozilla.org/mozilla-central/rev/ff1e250b7f23 show how to do that,
> already done that way in appstrings.properties.

The problem is that this doesn't scale. You end up with an asymmetry between the toolkit appstrings.properties, and the override one, the latter having one extra %S. It works in that particular case because the toolkit appstrings.properties doesn't have any %S, but it brings us back to comment 18 for other strings.

Comment 46

[Axel Hecht [:Pike]]

Introducing a new variable format into l10n is *way* more harmful, as it needs to scale up a hundred times (for each localization)

Comment 47

[O. Atsushi (Torisugari)]

(In reply to Axel Hecht [:Pike] from comment #44)
> r-, preprocessing the l10n files is making the situation worse than it is
> today. 

We are already doing tons of "#" comments remove with preprocessing macro. Could you explain why you call it "worse"? What's the exact problem?

> Do use brand.* instead,
> http://hg.mozilla.org/mozilla-central/rev/ff1e250b7f23 show how to do that,
> already done that way in appstrings.properties.

Oh, run-time check! This is not only a dirty hack (note that core code has dependency on product files), but also complicated. Inside docshell, there is no good explanation why "malformedURI" doesn't need the brand name while "protocoNotFound" needs it. I'd like to insist that /browser/* should absorb all the differences came from browser's UX. That's the only reasonable way to solve the difference among languages, as you said in comment #19. What should we do when some localizers want to use the word "Firefox" in "malformedURI" while en-US don't?

(In reply to Axel Hecht [:Pike] from comment #46)
> Introducing a new variable format into l10n is *way* more harmful, as it
> needs to scale up a hundred times (for each localization)

For example?

Comment 48

[O. Atsushi (Torisugari)]

By the way, B2G's brandShortName and MOZ_APP_DISPLAYNAME are both "B2G", but
http://mxr.mozilla.org/mozilla-central/source/b2g/locales/en-US/chrome/overrides/appstrings.properties
says "Firefox". Is this intentional or just a bug?

Comment 49

[Mike Kaply [:mkaply]]

I'll agree that that part of that code is a hack. Especially given that the string bundle is retrieved in two places.

But the check for the brandBundle is not a hack. That's how we determine whether or not there is a brand at all (This might be a XUL Runner application).

Looking through the strings in the appstrings.propertes, I'm not so sure the comment 18 thing is an issue anymore.

We can safely make "Firefox" %2$S in the strings where there are multiple strings. And make it %1$S in the other cases. And only pass the extra parameter if there is a brand. In the default case, it would be ignored because there is no substitution, and in the Firefox case it would be used.

I don't see any strings that would break in this case.

The only really interesting string in this file is externalProtocolPrompt and it doesn't reference Firefox in its error.

Comment 50

[Mike Hommey [:glandium]]

(In reply to Michael Kaply (mkaply) from comment #49)
> Looking through the strings in the appstrings.propertes, I'm not so sure the
> comment 18 thing is an issue anymore.

It is when you look at the docshell code and how the string arguments are given. Or you're going to change a lot of code in docshell.

Comment 51

[Mike Kaply [:mkaply]]

> It is when you look at the docshell code and how the string arguments are given. Or you're going to change a lot of code in docshell.

Changing code in docshell is kind of a given. Right now it passes no brand parameters at all (except in the confirmRepost case).

The docshell changes would involve changing all strings to accept a parameter (no more simple formatting), and always tacking on the brand as the last string in the list that is passed.

We shouldn't be afraid to change docshell in an effort to do the right thing.

Comment 52

[Jeff Walden [:Waldo]]

(In reply to Michael Kaply (mkaply) from comment #51)
> We shouldn't be afraid to change docshell in an effort to do the right thing.

While this is true, that sort of incremental change seems a bit like lipstick on a pig to me.  :-)  Docshell could probably expose a better interface for responding to network errors, that doesn't require the sort of string-coding done here to customize.

Which isn't to say that an expedient fix is necessarily wrong -- just that if an effort to do the right thing means changing docshell, we shouldn't have tunnel vision about what the "right thing" would be, based on what it is now.

Comment 53

[O. Atsushi (Torisugari)]

(In reply to Michael Kaply (mkaply) from comment #51)
> The docshell changes would involve changing all strings to accept a
> parameter (no more simple formatting), and always tacking on the brand as
> the last string in the list that is passed.

I think we should change C++ side anyway. But what you are suggesting seems to me a little too complicated and wasteful.

Now I'm not too sure whether or not I underestimate the harm of "introducing a new variable format into l10n". If we had introduced build-time substitution earlier, this bug have been fixed easily. And it seems to me we need it sooner or later (per comment #19). Then everything will be very simple, even for localizers. No?

Comment 54

[Mike Kaply [:mkaply]]

We have an established process for putting brand names into DTD and properties.

We use the files are brand.dtd and brand.properties.

Building yet another way to put branding into files may get things done faster, but it's not the correct thing to do here.

We have brand.dtd and brand.properties for a reason.

Comment 55

[Axel Hecht [:Pike]]

All variables that are unknown or in a new format get translated in a good chunk of locales. Like, I've seen an en-US string of "width: 500px" end up with translated width, and 500 being transcribed into the numbers of that locale.

Also, as mkaply points out, it's just not the right thing to do. Not just for rebranding by external people, but also because in some localizations, we transcribe "Firefox", so it'd be the wrong string at build time.

Comment 56

[O. Atsushi (Torisugari)]

(In reply to Axel Hecht [:Pike] from comment #55)
> All variables that are unknown or in a new format get translated in a good
> chunk of locales. Like, I've seen an en-US string of "width: 500px" end up
> with translated width, and 500 being transcribed into the numbers of that
> locale.
That seems fine for me. "@MOZ_APP_DISPLAYNAME@" is apparently one word. If it's still confusing, We can add a "#" comment to prevent it. We can also rename it to "@MOZAPPDISPLAYNAME@".

> Also, as mkaply points out, it's just not the right thing to do. Not just
> for rebranding by external people, but also because in some localizations,
> we transcribe "Firefox", so it'd be the wrong string at build time.

This is exactly what I wanted to know. I don't mean to all the locales should use @MOZ_APP_DISPLAYNAME@. But what's important is the fact that transcription has already finished when you build a given locale package. i.e. We should move the transcription information from  /browser/locales/en_US/... to /browser/branding/***/locales/en_US/... Then we can run a new build script to set a proper name, say, $(MOZ_APP_LOCALIZED_DISPLAYNAME_AB_CD) at configure.sh in the branding directory. Otherwise, there's no chance for rebranders to pick up this transcription information in a specific locale.

Comment 57

[Axel Hecht [:Pike]]

(In reply to O. Atsushi (Torisugari) from comment #56)
> (In reply to Axel Hecht [:Pike] from comment #55)
> > All variables that are unknown or in a new format get translated in a good
> > chunk of locales. Like, I've seen an en-US string of "width: 500px" end up
> > with translated width, and 500 being transcribed into the numbers of that
> > locale.
> That seems fine for me. "@MOZ_APP_DISPLAYNAME@" is apparently one word. If
> it's still confusing, We can add a "#" comment to prevent it. We can also
> rename it to "@MOZAPPDISPLAYNAME@".

@MOZ_ANWENDUNG_DARSTELLUNGSNAME@ is fine for you? 'Cause that's what I was talking about, literally translating the variable name.

> > Also, as mkaply points out, it's just not the right thing to do. Not just
> > for rebranding by external people, but also because in some localizations,
> > we transcribe "Firefox", so it'd be the wrong string at build time.
> 
> This is exactly what I wanted to know. I don't mean to all the locales
> should use @MOZ_APP_DISPLAYNAME@. But what's important is the fact that
> transcription has already finished when you build a given locale package.
> i.e. We should move the transcription information from 
> /browser/locales/en_US/... to /browser/branding/***/locales/en_US/... Then
> we can run a new build script to set a proper name, say,
> $(MOZ_APP_LOCALIZED_DISPLAYNAME_AB_CD) at configure.sh in the branding
> directory. Otherwise, there's no chance for rebranders to pick up this
> transcription information in a specific locale.

Sorry, I don't understand what you're trying to do here.

Comment 58

[O. Atsushi (Torisugari)]

(In reply to Axel Hecht [:Pike] from comment #57)
> @MOZ_ANWENDUNG_DARSTELLUNGSNAME@ is fine for you? 'Cause that's what I was
> talking about, literally translating the variable name.
So you don't think it's a edge case? "#filter substitution" requires exact matching. So "@MOZ_ANWENDUNG_DARSTELLUNGSNAME@" would make the build red/orange. Isn't it enough?

> Sorry, I don't understand what you're trying to do here.
I'll try to write a PoC in a simple format tomorrow.

Comment 59

[O. Atsushi (Torisugari)]

Attachment: PoC v1

I hope this is descriptive enough.

The new file is located at /browser/branding/*/locales/en-US/*, which is both l10n and branding sensitive information.

Comment 60

[O. Atsushi (Torisugari)]

Attachment: proposal v3

Comment 61

[Axel Hecht [:Pike]]

Comment on attachment 607608 [details] [diff] [review]
proposal v3

Seriously, just feeding patches with new variables or file formats my way is just not going to get this bug moved forward.

*Please* listen to the advice I give, otherwise you're wasting both your time and mine.

Comment 63

[O. Atsushi (Torisugari)]

(In reply to Axel Hecht [:Pike] from comment #61)
> Comment on attachment 607608 [details] [diff] [review]
> proposal v3
> 
> Seriously, just feeding patches with new variables or file formats my way is
> just not going to get this bug moved forward.
> 
> *Please* listen to the advice I give, otherwise you're wasting both your
> time and mine.

Where is your advice in this bug? You have not explained why my patch is not acceptable for you so far. I'm suprised that you seriously think this bug can be fixed without "new variable".

Comment 64

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

I had a patch in bug 545533 that was doing string substitution with brandhShortName, though the effort stalled (and Axel rightly wasn't wild about the idea). Not sure if that's something we can resurrect or if what's happening here is going down a better path, but now we have 1 less open bug.

Comment 65

[Mike Kaply [:mkaply]]

What you put together was essentially the same patch I was going to build.

Comment 66

[O. Atsushi (Torisugari)]

According to comment #19, we can't hard-code |processBrand = true| like Paul's patch...

Comment 67

[Mike Kaply [:mkaply]]

> According to comment #19, we can't hard-code |processBrand = true| like Paul's patch...

We can simply pass the brand as the last parameter to all the strings and they can choose to use it or not.

Comment 68

[Axel Hecht [:Pike]]

My advice is to use brand.properties.

Please don't try to hack around that. FWIW, build failures are not acceptable for l10n, and localizers editing makefiles is not gonna happen for sure.

Comment 69

[Athira S]

I am a beginner and I would like to contribute to Open source.I would like to fix this bug
I have already build mozilla.can somebody help me to get started to fix this bug?

Comment 71

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is relatively low, S4. However, the bug has 4 duplicates.
:mossop, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 72

[Mike Kaply [:mkaply]]

Flod, I know we were fixing this for netError.

Can we fix this for appstrings too (two versions, dom and browser)

Comment 73

[Francesco Lodolo [:flod]]

At this point, this would likely need someone from the Firefox dev team. Our priority was to migrate DTDs for netError, because the migration was the most complicated we've done so far, and we kept delaying it for this reason.

The only person in my team who could work on it is Eemeli, and he already has a bunch of large stacks in flight for .properties plurals (bug 1793557, bug 1791178, bug 1760029), on top of work unrelated to Firefox.

Comment 74

[Mike Kaply [:mkaply]]

I think we should leave this at S4 then.