Closed
Bug 358079
Opened 18 years ago
Closed 17 years ago
Proper l10n for crash reporter client
Categories
(Toolkit :: Crash Reporting, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9beta1
People
(Reporter: ted, Assigned: ted)
References
Details
Attachments
(1 file)
(deleted),
patch
|
Pike
:
review+
benjamin
:
review+
|
Details | Diff | Splinter Review |
We'll need to move crashreporter.ini into toolkit/locale and whatever other build-fu is necessary to make the crashreporter client fully localizable.
Assignee | ||
Comment 1•18 years ago
|
||
We should switch this from an ini file to a properties file, since we're just parsing it for key=value pairs anyway. That should make l10n easier.
Comment 2•18 years ago
|
||
Yeah, what Ted said, as per discussion on IRC.
Assignee | ||
Updated•18 years ago
|
Flags: blocking1.9?
Summary: Proper l10n for airbag crash reporter client → Proper l10n for crash reporter client
Assignee | ||
Updated•18 years ago
|
Assignee: nobody → ted.mielczarek
Updated•18 years ago
|
Flags: blocking1.9? → blocking1.9+
Target Milestone: --- → mozilla1.9beta1
Comment 4•17 years ago
|
||
The parser at http://mxr.mozilla.org/mozilla/source/toolkit/airbag/client/crashreporter.cpp#130 doesn't do any kind of format checking.
It won't eat everything that we consider to be a .properties file, though, so in order to not stumble over that, we should fix the parser to be a little more generous.
Sample implementations are at http://lxr.mozilla.org/mozilla/source/xpcom/ds/nsPersistentProperties.cpp#146 and I'm using regexps at http://lxr.mozilla.org/mozilla/source/testing/tests/l10n/lib/Mozilla/Parser.py#95, too.
Basically, the difference is leading whitespace is ignored, the ':' can be a separator char, and whitespace around the separator char is ignored. Trailing spaces and tabs, too. The worst part is, we're still unescaping unicode escapes in .properties, which for whatever reason some tools out there might still serialize.
If you don't want to support all of that, we could just stick with the ini file, or preprocess a .properties file into an ini-ish file, but that would likely require moving some of the python test code out of testing into build.
Thinking about it some more, at least translate toolkit seems to do ini import and export, we should probably just use the ini file with its constrained grammar. MozillaTranslator has issues with integrating it, but offers a plain text translation of it, which should be good enough.
Assignee | ||
Comment 5•17 years ago
|
||
Some more info for my sanity:
http://developer.mozilla.org/en/docs/Localization_notes
bug 382128 may or may not be a blocker here, it's something we should definitely consider.
We'll need a note that the two replacements here cannot be reordered:
http://mxr.mozilla.org/mozilla/source/toolkit/airbag/client/crashreporter.ini#6
Comment 6•17 years ago
|
||
Update to the links,
http://mxr.mozilla.org/mozilla/source/toolkit/crashreporter/client/crashreporter.cpp#130
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/toolkit/crashreporter/client/crashreporter.ini&rev=1.7&mark=6
I'm not sure if bug 382128 should block, but maybe it doesn't matter. I'll comment there.
Ted, as M8 is out, care to retriage this one? Thanks
Assignee | ||
Updated•17 years ago
|
Target Milestone: mozilla1.9 M8 → mozilla1.9 M9
Assignee | ||
Comment 7•17 years ago
|
||
Ok, this patch moves crashreporter.ini to toolkit/locales/crashreporter and does a little bit of makefile stuff to make it work. It also adds l10n notes to the crashreporter.ini file.
Attachment #283391 -
Flags: review?
Assignee | ||
Updated•17 years ago
|
Attachment #283391 -
Flags: review? → review?(l10n)
Assignee | ||
Updated•17 years ago
|
Attachment #283391 -
Flags: review?(benjamin)
Comment 8•17 years ago
|
||
Can we have the crashreporter.ini strings in a .properties format? Just like the NSIS installer stuff?
Assignee | ||
Comment 9•17 years ago
|
||
You can see Pike and I discussed this, he seemed to think it would be ok. We'd have to do some work to make the crashreporter app parse real properties files as per comment 4.
Comment 10•17 years ago
|
||
Comment on attachment 283391 [details] [diff] [review]
move crashreporter.ini to toolkit/locales/crashreporter + build-fu to make it work + localization notes
r=me. We have .ini files in l10n already, so I'm OK with adding another one, and parsing .properties is just a lot more work than .ini.
Nice localization notes, thanks.
Attachment #283391 -
Flags: review?(l10n) → review+
Updated•17 years ago
|
Attachment #283391 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 11•17 years ago
|
||
Comment on attachment 283391 [details] [diff] [review]
move crashreporter.ini to toolkit/locales/crashreporter + build-fu to make it work + localization notes
Pretty trivial patch, but necessary for localization of the crash reporter client. Also note the blocking1.9+.
Attachment #283391 -
Flags: approval1.9?
Assignee | ||
Updated•17 years ago
|
Attachment #283391 -
Flags: approval1.9?
Assignee | ||
Comment 12•17 years ago
|
||
Checked in since we're back in orange and it's a blocker.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•