Closed Bug 298348 Opened 15 years ago Closed 14 years ago
Lightning needs to be converted to DTDs for localization
64.80 KB, patch
|Details | Diff | Splinter Review|
We currently have no l10n story; we don't even really pretend.
15 years ago
This doesn't block 0.8.
No longer blocks: lightning-0.1
I'm willing to do this.
Assignee: shaver → bugzilla
QA Contact: shaver → nobody
Just for the record: I don't really know how to localize the stuff in http://lxr.mozilla.org/mozilla/source/calendar/base/content/calendar-month-view.xml#382 http://lxr.mozilla.org/mozilla/source/calendar/base/content/calendar-multiday-view.xml#2132 so the patches, I will attach here, will not cover this stuff.
Ok, here's the patch for the calendar/base part. Three additional notes: 1. The patch contains all the changes to calendar.dtd that are also in the first patch. 2. It also contains a change in resources/content/eventDialog.xul. If that change is okay, it will probably eliminate some logic in eventDialog.js. But since I'm not a qualified JS coder, I didn't mess with that file. 3. This patch is without the whitespace changes I applied to calendar.dtd for better readability. I will attach a patch for checkin purposes, which has all the whitespace changes.
Don't do whitespace changes in .dtd files, at least not without talking to mostafah. iirc, he uses some scripts, and they don't like random whitespace changes. And if you want such major changes, do it in a different bug, to keep cvs blame a bit sane.
Attachment #198914 - Flags: second-review?(mvl) → second-review-
Comment on attachment 198491 [details] [diff] [review] calendar/lightning part - patch v2 Looks good; just a few minor tweaks required. >Index: calendar/resources/locale/en-US/calendar.dtd >=================================================================== >RCS file: /cvsroot/mozilla/calendar/resources/locale/en-US/calendar.dtd,v >retrieving revision 1.111 >diff -u -6 -r1.111 calendar.dtd >--- calendar/resources/locale/en-US/calendar.dtd 22 Sep 2005 01:51:04 -0000 1.111 >+++ calendar/resources/locale/en-US/calendar.dtd 4 Oct 2005 20:03:14 -0000 >@@ -263,13 +263,15 @@ > <!ENTITY calendar.gototoday.button.tooltip "Go to today" > > <!ENTITY calendar.choosedate.button.tooltip "Choose date to go to" > > > <!ENTITY calendar.dayview.button.tooltip "Switch to day view" > > <!ENTITY calendar.weekview.button.tooltip "Switch to week view" > > <!ENTITY calendar.monthview.button.tooltip "Switch to month view" > >-<!ENTITY calendar.multiweekview.button.tooltip "Switch to multiweek view" > >+<!ENTITY calendar.multiweekview.button.tooltip "Switch to multiweek view" > As mvl said, please check with Mostafa about whitespace changes here (or just get rid of the whitespace change). >Index: calendar/lightning/locale/lightning.dtd >=================================================================== >RCS file: /cvsroot/mozilla/calendar/lightning/locale/lightning.dtd,v >retrieving revision 1.1 >diff -u -6 -r1.1 lightning.dtd >--- calendar/lightning/locale/lightning.dtd 15 Feb 2005 21:45:45 -0000 1.1 >+++ calendar/lightning/locale/lightning.dtd 4 Oct 2005 20:03:28 -0000 >@@ -1,2 +1,27 @@ > <!-- Tools menu --> > <!ENTITY lightning.taskLabel "Lightning"> >+ >+<!-- Agenda Tree View --> >+<!ENTITY agenda.treeview.label "View:"> >+<!ENTITY agenda.treeview.menu.label "View"> >+<!ENTITY agenda.treeview.all.label "All"> >+<!ENTITY agenda.treeview.events.label "Events"> >+<!ENTITY agenda.treeview.tasks.label "Tasks"> >+<!ENTITY agenda.treeview.reminders.label "Reminders"> >+<!ENTITY agenda.treeview.customize.label "Customize..."> >+<!ENTITY agenda.treeview.item.label "Item"> >+<!ENTITY agenda.treeview.label "Customize your views!"> agenda.treeview.label has already been used; I'm betting this needs a different ENTITY name. Also, can you drop the exclamation point in the text? The XUL overlay has the same problem with the re-use of the ENTITY name. Instead of "messenger.", can we use "lightning." as a prefix? >+<!-- Messenger Sidebar --> >+<!ENTITY messenger.toolbar.dayview.label "Day View"> >+<!ENTITY messenger.toolbar.weekview.label "Week View"> >+<!ENTITY messenger.toolbar.monthview.label "Month View"> I bet that comment wants to read "Toolbar" instead of "Sidebar". Thanks for the patch!
Attachment #198491 - Flags: first-review?(dmose) → first-review-
New patch with review comments addressed. Additionally I added license headers to all the files I touched.
Patch with review comments addressed. I only moved the hardcoded strings to entities in calendar.dtd. Don't blame me if it's not pretty :-) I chose to leave resources/content/eventDialog.xul alone. And like the lightning patch I added a license header to files I touched, when there was none.
Comment on attachment 199655 [details] [diff] [review] calendar/lightning part - patch v3 >+<!ENTITY agenda.treeview.description "Customize your views"> How about making this just "Customize views"? Using a personal pronoun in the UI sounds kind of unusual. Sorry for not noticing this last time. Change that, and you've got r=dmose.
Attachment #199655 - Flags: first-review?(dmose) → first-review+
Comment on attachment 199688 [details] [diff] [review] calendar/base part - patch v2 r=dmose. Note that I didn't review the text tweaks themselves closely; it makes sense to me to land this without really worry ing about those, and let folks file new bugs on them if there really are issues.
Attachment #199688 - Flags: first-review?(dmose) → first-review+
Comment on attachment 199688 [details] [diff] [review] calendar/base part - patch v2 Looks good. I trust dmose's review. I didn't check everything, so i can't say i reviewed it. Lets say I ok-ed it :)
Patch with last review comment from dmose addressed. I'm carrying over the review+ from the v3 patch.
Attachment #200428 - Flags: first-review+
*** Bug 313759 has been marked as a duplicate of this bug. ***
Attachment #199655 - Attachment is obsolete: true
Unfortunately, there was a bit of a mixup here: I somehow was confused and thought you had checkin privs. As a result, I didn't realize you were expecting me to land them until Joey pinged me yesterday. So I just tried applying these patches to my local tree and found a couple of issues: a) The base patch doesn't actually apply because the tree has changed out from under it. Can you generate a new patch for that one? b) The lightning patch has multiple XUL files with JS/C/C++-style header boilerplate added, which means that the XML parser can't parse them at all, so it needs a new version as well. In the future, if you could test patches before requesting review, that would be much appreciated. Anyway, thanks again for the patch, and sorry for the delay.
(In reply to comment #18) > Unfortunately, there was a bit of a mixup here: I somehow was confused and > thought you had checkin privs. You aren't the first to think that :-) > a) The base patch doesn't actually apply because the tree has changed out from > under it. Can you generate a new patch for that one? Expect a new patch on Friday or Saturday. > b) The lightning patch has multiple XUL files with JS/C/C++-style header > boilerplate added Ups, sorry for that. I somehow mixed up the commenting style. I'll post a new patch on Friday or Saturday. > In the future, if you could test patches before requesting review, that would > be much appreciated. I just tested the base patch and didn't bother to test the lighning patch. My fault, won't happen again. Sorry for the hassle. > Anyway, thanks again for the patch, and sorry for the delay. No problem. You're much faster than other reviewers in other parts of the tree.
Attaching an updated patch from Bug 313759 as per sipaqs request.
Attachment #200871 - Flags: first-review?(bugzilla)
Comment on attachment 200871 [details] [diff] [review] calendar-alarm-dialog.xul hardcode-stuff >Index: mozilla/calendar/resources/locale/en-US/calendar.dtd >=================================================================== > >+<!ENTITY calendar.alarm.dismiss.label "Dismiss" > >+<!ENTITY calendar.alarm.dismissall.label "Dismiss All" > > <!ENTITY calendar.alarm.snooze.label "Snooze" > >-<!ENTITY calendar.alarm.title.label "Calendar Alarm" > >+<!ENTITY calendar.alarm.title.label "Calendar Alarm" > The calendar.dtd portion of your patch still contains string changes, which apply only to calendar-alarm-widget.xml, which should not be covered by this patch. Also don't do random whitespace changes (see comment 8 and comment 10). Please post an updated patch.
Attachment #200871 - Flags: first-review?(bugzilla) → first-review-
New version reflecting sipaqs comments. Sorry about the whitespace stuff, however, I thought it was prefered to have the lines aligned.
Comment on attachment 200888 [details] [diff] [review] calendar-alarm-dialog.xul hardcode-stuff V2 >Index: mozilla/calendar/resources/locale/en-US/calendar.dtd >=================================================================== > > <!-- Calendar Alarm Dialog --> > <!ENTITY calendar.alarm.acknowledge.label "Acknowledge" > > <!ENTITY calendar.alarm.editevent.label "Edit Event" > >-<!ENTITY calendar.alarm.acknowledgeall.label "Acknowledge All Alarms" > >+<!ENTITY calendar.alarm.dismissall.label "Dismiss All" > > <!ENTITY calendar.alarm.snooze.label "Snooze" > > <!ENTITY calendar.alarm.title.label "Calendar Alarm" > Better! But please leave in calendar.alarm.acknowledgeall.label for now. We should wait with cleanup work until after 0.3a1 and also this bug is not the right place for cleanup work. Cleanup work should happen in a separate bug.
Attachment #200888 - Flags: first-review?(bugzilla) → first-review-
There you go. This one should do it! =) (I had no idea about the cleanup-in-separate-bug stuff, sorry about that)
Comment on attachment 200890 [details] [diff] [review] calendar-alarm-dialog.xul hardcode-stuff V3 r=sipaq
Attachment #200890 - Flags: first-review?(bugzilla) → first-review+
Ok, here's the final patch for checkin purposes, which applies to the current state of the trunk. It combines the lightning patch, the base patch and the additional patch for calendar-alarm-dialog.xul from Robin Edrenius. I've changed all the C++ comments to XML style comments. I added myself to the contributors section where I thought it would be warranted (exception: calendar-alarm-dialog.xul, where I added Robin).
In mozilla/calendar/resources/content/calendarProperties.xul the string "Read-only" is hard-coded too. Is this the proper bug to solve this problem? Or do we need a new one?
(In reply to comment #27) > In mozilla/calendar/resources/content/calendarProperties.xul the string > "Read-only" is hard-coded too. Is this the proper bug to solve this problem? Or > do we need a new one? Stefan, this is not a Lightning issue. Please file a new bug.
Patch checked in; thanks for the work, guys!
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Why you guys don't follow the source-l10n path like Fx and Tb?
(In reply to comment #31) > Why you guys don't follow the source-l10n path like Fx and Tb? Gandalf, please see bug 281935 and bug 267981 for our ongoing work on that issue. This bug here does in no way relate to that issue.
This has caused bug 317119
You need to log in before you can comment on or make changes to this bug.