Closed Bug 392407 Opened 13 years ago Closed 12 years ago

appstrings.properties should not be using branded strings

Categories

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

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: alqahira, Assigned: mkmelin)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

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.
Flags: blocking1.9?
<sigh>.  Yeah, we need to fix this.
Flags: blocking1.9? → blocking1.9+
Attached patch proposed fix (WIP) (obsolete) — Splinter Review
Something like this? 
Still have to change the keys (and include changes from bug 376040 at the same time I suppose).
Assignee: nobody → mkmelin+mozilla
Status: NEW → ASSIGNED
OS: Mac OS X → All
Hardware: Macintosh → All
Priority: -- → P5
Attachment #281080 - Flags: review?(bzbarsky)
There's no reason to expect the application to be a browser.  It could be, say, Thunderbird.  Or Songbird.  Or whatever.
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.
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.
I guess so. I'll use "the application" then, if no one has a better word in mind.
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".
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.
*Have* branding.
Attached patch proposed fix, v2Splinter Review
This includes the proposed string fix from bug 376040 comment 0. (My favorites would be that or bug 376040 comment 12.)
Attachment #281080 - Attachment is obsolete: true
Attachment #288009 - Flags: ui-review?(beltzner)
Attachment #281080 - Flags: review?(bzbarsky)
Attachment #288009 - Flags: superreview?(bzbarsky)
Attachment #288009 - Flags: review?(bzbarsky)
Attachment #288009 - Flags: superreview?(bzbarsky)
Attachment #288009 - Flags: superreview+
Attachment #288009 - Flags: review?(bzbarsky)
Attachment #288009 - Flags: review+
Is ui‑review required? Though it would be nice if beltzner could make his thumbs up... Or should I just ask for approvalNext?
beltzner: ping on the ui-r?
Attachment #288009 - Flags: ui-review?(beltzner) → ui-review+
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
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Keywords: late-l10n
Resolution: --- → FIXED
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/.
Yeah, I actually noticed that just before checking in and fixed it.
You need to log in before you can comment on or make changes to this bug.