392407 - appstrings.properties should not be using branded strings

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect, P5)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Per bug 302309 comment 23, embeddable code should not be using branded strings (at least until someone figures out a way to ship a default branding package and an override system).

Bug 112848 added branded strings to appstrings.properties (causing all or at least part of bug 359857), so appstrings.properties needs to be fixed.  (The current "proper" way to add "branded" strings in these files is to leave the shared files generic--"this application" or "the browser"--and insert the appname directly in each app's overrides file, e.g. browser/locales/en-US/chrome/overrides/appstrings.properties for Firefox.)

For some reason I never filed this back when bug 359857 popped up.

Comment 1

[Boris Zbarsky [:bzbarsky]]

<sigh>.  Yeah, we need to fix this.

Comment 2

[Magnus Melin [:mkmelin]]

Attachment: proposed fix (WIP)

Something like this? 
Still have to change the keys (and include changes from bug 376040 at the same time I suppose).

Comment 3

[Boris Zbarsky [:bzbarsky]]

There's no reason to expect the application to be a browser.  It could be, say, Thunderbird.  Or Songbird.  Or whatever.

Comment 4

[Magnus Melin [:mkmelin]]

True, but I don't know what would normally be a better word. As seamonkey don't override the default appstrings file, it will be the main normal user.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Er... wait.  Doesn't that remove the "Seamonkey" in Seamonkey, then?  Which means they will likely just create their own appstrings file, and stop being a user here altogether.  So I stand by comment 3.

Comment 6

[Magnus Melin [:mkmelin]]

I guess so. I'll use "the application" then, if no one has a better word in mind.

Comment 7

[Boris Zbarsky [:bzbarsky]]

So wait.  Existing apps that do have branding but don't override appstrings.properties... you'll try to FormatStringFromName, right?  What will happen then?

And I'd use "this application", not "that application".

Comment 8

[Magnus Melin [:mkmelin]]

Yes, they if they don't have branding they go through FormatStringFromName, which will do the same thing as GetStringFromName since there is no %S specified there.

Comment 9

[Magnus Melin [:mkmelin]]

*Have* branding.

Comment 10

[Magnus Melin [:mkmelin]]

Attachment: proposed fix, v2

This includes the proposed string fix from bug 376040 comment 0. (My favorites would be that or bug 376040 comment 12.)

Comment 11

[Magnus Melin [:mkmelin]]

Is ui‑review required? Though it would be nice if beltzner could make his thumbs up... Or should I just ask for approvalNext?

Comment 12

[Magnus Melin [:mkmelin]]

beltzner: ping on the ui-r?

Comment 13

[Magnus Melin [:mkmelin]]

Checking in dom/locales/en-US/chrome/appstrings.properties;
/cvsroot/mozilla/dom/locales/en-US/chrome/appstrings.properties,v  <--  appstrings.properties
new revision: 1.8; previous revision: 1.7
done
Checking in docshell/base/nsDocShell.cpp;
/cvsroot/mozilla/docshell/base/nsDocShell.cpp,v  <--  nsDocShell.cpp
new revision: 1.878; previous revision: 1.877
done
Checking in browser/locales/en-US/chrome/overrides/appstrings.properties;
/cvsroot/mozilla/browser/locales/en-US/chrome/overrides/appstrings.properties,v  <--  appstrings.properties
new revision: 1.11; previous revision: 1.10
done

->FIXED

Comment 14

[Marek Stępień [:marcoos, inactive]]

You might want to remove the localization note from the dom/ version of this file, and update the property name in the localization note in browser/.

Comment 15

[Magnus Melin [:mkmelin]]

Yeah, I actually noticed that just before checking in and fixed it.