Currently, nsIStringBundle::GetStringFromName and FormatStringFromName take "wstring" parameters.. this is just silly because most (if not all) keys are pure ASCII. This leads to the following waste: - UCS2 literal strings throughout the code (they could be half the size) - useless conversion from UCS2 to UTF8 when looking up the key in nsIProperties - other useless conversion and waste when dealing with dynamically generated string keys... This obviously only hurts C++ though. I just did a search through the sourcebase and found 221 calls to GetStringFromName, of which 104 are using NS_LITERAL_STRING directly..I can only imagine about the rest. (I haven't looked for FormatStringFromName yet) Its a lot of callers to update, but boy I'll bet we'd see some decent gains and footprint reduction.
Summary: string bundles should use ASCII or UTF8 keys → [minimo]string bundles should use ASCII or UTF8 keys
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.5alpha
moving minimo bugs to the new bugzilla product.
Component: Internationalization → General
Product: Browser → Minimo
Target Milestone: mozilla1.5alpha → ---
Version: Trunk → unspecified
Assignee: alecf → nobody
Status: ASSIGNED → NEW
Component: General → Internationalization
Product: Minimo → Core
Summary: [minimo]string bundles should use ASCII or UTF8 keys → string bundles should use ASCII or UTF8 keys
-> me i'm working on this. patch coming up soon.
Assignee: nobody → dwitte
Created attachment 295037 [details] [diff] [review] stringbundle changes, v1 ok, these are the changes to the core nsIStringBundle & friends... the changes are pretty obvious from the idl, but here's what i did: 1) change the key params to be |string| instead of |wstring|. 2) change the outparams to be |AString| instead of |wstring|. i think that makes for a sane API... we don't really need an ACString key, a regular char* is very lightweight and most consumers are just passing in literals like that. making the outparam an AString really helps cut down on silly allocations/copying though. i'm not really sure whether we should do anything with |[array, size_is(length)] in wstring params| on formatStringFrom*... it seems like it should stay as a wide string, so i left it that way. anyway, this really tidies up the stringbundle implementation, and simplifies a lot of code throughout the tree... the patch for consumers is coming up shortly, once i get it building.
smontagu, jshin - would either of you be willing to review the stringbundle patch here? any comments would be appreciated!
Created attachment 295041 [details] [diff] [review] treewide changes, v1 and here's the whole tree... this is an enormous patch so i'm not sure how we'll do review here, i mean i doubt anyone wants to read all that ;) 192 files changed, 1381 insertions(+), 1779 deletions(-)
OS: Windows 2000 → All
QA Contact: amyy → i18n
Hardware: PC → All
Target Milestone: Future → ---
Should we forget about this? No review love here, and the patch has most likely rotted significantly; unless someone's willing to review this we should close it out.
Reassigning to nobody. If anyone wants to work on this, feel free!
Assignee: dwitte → smontagu
Status: NEW → RESOLVED
Last Resolved: 8 months ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1380227
You need to log in before you can comment on or make changes to this bug.