Closed
Bug 340723
Opened 19 years ago
Closed 19 years ago
calendar locales file contains #ifdef
Categories
(Calendar :: Sunbird Only, defect)
Calendar
Sunbird Only
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: kairo, Assigned: mattwillis)
Details
(Keywords: l12y)
Attachments
(2 files, 1 obsolete file)
1.50 KB,
patch
|
Pike
:
first-review+
|
Details | Diff | Splinter Review |
3.09 KB,
patch
|
mattwillis
:
first-review+
bmo.cec
:
second-review+
|
Details | Diff | Splinter Review |
http://lxr.mozilla.org/mozilla/source/calendar/locales/en-US/chrome/calendar/prefs.dtd#57 contains a #ifdef that excludes a section of the file from the build.
If someone (like me) uses a non-Sunbird calendar-en-US.jar file as the base for his L10n, that section will be missing from the L10n and Sunbird will fail to work.
OTOH, if the #ifdef would just not be there, the entities not used in non-Sunbird would just silently be ignored there.
That's some of the reasons why there is a general guideline of never using preprocessor macros in locale files, except in very special cases, see http://developer.mozilla.org/en/docs/Writing_localizable_code#Guidelines
This ifdef should just be removed, there is no harm in having those strings in the file, even if they might not be used everywhere.
Reporter | ||
Comment 1•19 years ago
|
||
This should work (not tested yet as I don't build Sunbird or Lightning, reviewer please test before granting review), it just removes the #ifdef.
Assignee: nobody → kairo
Status: NEW → ASSIGNED
Attachment #224763 -
Flags: first-review?(mattwillis)
Assignee | ||
Comment 2•19 years ago
|
||
This was fixed with one of the checkins for bug 267981.
-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•19 years ago
|
Attachment #224763 -
Flags: first-review?(mattwillis) → first-review+
Assignee | ||
Comment 3•19 years ago
|
||
Comment on attachment 224763 [details] [diff] [review]
just remove the #ifdef
This is necessary to avoid entity collisions with Thunderbird in Lightning.
Attachment #224763 -
Flags: first-review+ → first-review-
Assignee | ||
Updated•19 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 4•19 years ago
|
||
Upon rereading and testing this, we NEED that ifdef.
I believe the need for this ifdef will disappear after we move to toolkit-based prefs, and after Lightning and Sunbird share some prefs
Since we need the ifdef to prevent the collision, I'm wontfixing this. Like I said, hopefully we'll avoid the need for the ifdef in nuprefs.
-> WONTFIX
Status: REOPENED → RESOLVED
Closed: 19 years ago → 19 years ago
Resolution: --- → WONTFIX
Comment 5•19 years ago
|
||
Iff you really need those #ifdefs, please put them in XML comments?
<!--
#ifndef...
-->
That way the DTD will still be standards compatible and can be read by tools,
and even localized, if the tools pay attention to being a good editor. Some do,
potools may actually handle that well.
Note, one way to circumvent the #ifdef in the localization code, which you
should try to achieve by all means, is to put the #ifdef in the source file
and include a separate DTD by build config containing just these strings.
I didn't fully investigate this, but this seems like a feasible technical
solution.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Comment 6•19 years ago
|
||
Just as a note, the translate toolkit (potools) can handle this file as it is
Of course, that's no reason not to improve the situation...
Assignee | ||
Comment 7•19 years ago
|
||
Since this dtd will be replaced when toolkit-based preferences land, I'm doing the quick and ugly fix for this only. I'll be sure it's done correctly in the new prefs.
Assignee: kairo → mattwillis
Attachment #224763 -
Attachment is obsolete: true
Status: REOPENED → ASSIGNED
Attachment #225026 -
Flags: first-review?
Assignee | ||
Updated•19 years ago
|
Attachment #225026 -
Flags: first-review? → first-review?(kairo)
Assignee | ||
Comment 8•19 years ago
|
||
I checked through all l10n locales having calendar. Only these had been updated to include the #ifdef
Attachment #225027 -
Flags: first-review?(kairo)
Comment 9•19 years ago
|
||
Comment on attachment 225026 [details] [diff] [review]
en-US: places the ifdef in comments
I'll do that review.
Which was the bug where you wanted to do the real fix?
Attachment #225026 -
Flags: first-review?(kairo) → first-review+
Assignee | ||
Comment 10•19 years ago
|
||
Assignee | ||
Comment 11•19 years ago
|
||
Comment on attachment 225027 [details] [diff] [review]
l10n: does the same for the two locales that are up-to-date enough to have the ifdef already
Adding Cedric for the fr review. KaiRo remains for the de review unless he wants to give it to someone else.
Attachment #225027 -
Flags: second-review?(cedric.corazza)
Assignee | ||
Comment 12•19 years ago
|
||
en-US version checked in to MOZILLA_1_8_BRANCH and trunk
Updated•19 years ago
|
Attachment #225027 -
Flags: second-review?(cedric.corazza) → second-review+
Assignee | ||
Comment 13•19 years ago
|
||
fr checked in on MOZILLA_1_8_BRANCH and trunk
Comment 14•19 years ago
|
||
lilmatt, i did the german localization of Sunbird and give you a review+ for the de changes.
Assignee | ||
Comment 15•19 years ago
|
||
Comment on attachment 225027 [details] [diff] [review]
l10n: does the same for the two locales that are up-to-date enough to have the ifdef already
r+ from thorsten
Attachment #225027 -
Flags: first-review?(kairo) → first-review+
Assignee | ||
Comment 16•19 years ago
|
||
de checked in on trunk. It doesn't yet exist on branch.
-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 19 years ago → 19 years ago
Resolution: --- → FIXED
Comment 17•19 years ago
|
||
I just added the commented out #ifdefs to pl in prefs.dtd on trunk and on the branch.
We based pl calendar localization on our Sunbird 0.3a2 localization (which was based on the content of Sundbird 0.3a2 en-US xpi) and updated it to the current state and put in the l10n CVS directory structure, with compare-locales showing no errors.
Is there anything else that compare-locales won't detect that is different in the 'compiled' en-US Calendar localization and we may have missed it this way? Any more #ifdefs? Or whatever?
Assignee | ||
Comment 18•19 years ago
|
||
(In reply to comment #17)
> Is there anything else that compare-locales won't detect that is different in
> the 'compiled' en-US Calendar localization and we may have missed it this way?
> Any more #ifdefs? Or whatever?
Not that I know of.
You need to log in
before you can comment on or make changes to this bug.
Description
•