346486 - Hard-coded application name in browser.properties

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Behnam Esfahbod [:zwnj]]

./browser/locales/en-US/chrome/browser/browser.properties:imageWarningBlocked=Firefox will now always block images from this site
./browser/locales/en-US/chrome/browser/browser.properties:imageWarningAllowed=Firefox will now allow images from this site

Comment 1

[Joey Minta]

We should also take care of bug 345349 comment #22 here.

Comment 2

[Joey Minta]

Attachment: verbage tweaks

This patch makes 3 different tweaks I've seen recommended to the notification we display.
1.) Don't hardcode the app-name as Firefox.
2.) End the sentence with a period.
3.) Use the host, rather than 'this site.'

Comment 3

[Anbo Motohiko]

> +imageWarningBlocked=%S will now always block images from %S.
> +imageWarningAllowed=%S will now allow images from %S.
Why same '%S'?
Localizers cannot omit brand name, and cannot change order.
IMHO first '%S' should be '%brand%' and replaced by brandShortName.


But I'm not an official localizer, please ask them first.

Comment 4

[Joey Minta]

(In reply to comment #3)
> > +imageWarningBlocked=%S will now always block images from %S.
> > +imageWarningAllowed=%S will now allow images from %S.
> Why same '%S'?
> Localizers cannot omit brand name, and cannot change order.
> IMHO first '%S' should be '%brand%' and replaced by brandShortName.
> 
I followed the same format as the popup warning:
http://lxr.mozilla.org/seamonkey/source/browser/locales/en-US/chrome/browser/browser.properties#82

Comment 5

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

Comment on attachment 231494 [details] [diff] [review]
verbage tweaks

Good with the verbiage tweaks, make sure we actually pass both variables. Have you tested this to ensure it works? I bet you have, you smartie!

Comment 6

[Joey Minta]

(In reply to comment #5)
> (From update of attachment 231494 [details] [diff] [review] [edit])
> Good with the verbiage tweaks, make sure we actually pass both variables. Have
> you tested this to ensure it works? I bet you have, you smartie!
> 
I did test, but I don't understand the "make sure we actually pass both variables" bit.  When would we not?  are you afraid we'll end up with something like "Firefox will now block images from Firefox."?  If that's the concern, getFormattedString is smart enough to handle that.

Comment 7

[Mike Connor [:mconnor]]

Comment on attachment 231494 [details] [diff] [review]
verbage tweaks

r=me, but you need to rev the property names so l10n sees this

also a=me, so this gets in ASAP, this is a simple string change necessary to fix inappropriate use of branding strings.

Comment 8

[Axel Hecht [:Pike]]

For future reference, it helps a great deal to use explicit ordering in properties, i.e., 

%1$S will now always block images from %2$S.

A localization note to describe both params is generally good to have, too.

Comment 9

[Joey Minta]

Landed on both trunk and branch.
Checking in browser/locales/en-US/chrome/browser/browser.properties;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/browser.properties,v  <--  browser.properties
new revision: 1.29; previous revision: 1.28
done
Checking in browser/base/content/browser.js;
/cvsroot/mozilla/browser/base/content/browser.js,v  <--  browser.js
new revision: 1.679; previous revision: 1.678

Checking in browser/base/content/browser.js;
/cvsroot/mozilla/browser/base/content/browser.js,v  <--  browser.js
new revision: 1.479.2.182; previous revision: 1.479.2.181
Checking in browser/locales/en-US/chrome/browser/browser.properties;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/browser.properties,v  <--  browser.properties
new revision: 1.20.2.10; previous revision: 1.20.2.9

Comment 10

[Reed Loden [:reed]]

This caused bug 349410.