Closed
Bug 443722
Opened 16 years ago
Closed 16 years ago
Add possiblity to change elements order in Edit Recurrence window
Categories
(Calendar :: General, defect)
Calendar
General
Tracking
(Not tracked)
VERIFIED
FIXED
0.9
People
(Reporter: hubert+bmo, Assigned: hubert+bmo)
Details
Attachments
(4 files, 3 obsolete files)
6.41 KB,
patch
|
Fallen
:
review+
|
Details | Diff | Splinter Review |
3.86 KB,
patch
|
hubert+bmo
:
review+
|
Details | Diff | Splinter Review |
24.00 KB,
image/png
|
Details | |
35.51 KB,
image/png
|
Details |
There are languages (bug 352513 comment c20) which require different order for dropdown lists. An example: Recurrence pattern -> Repeat annually in English: Every [ Fourth v] [ Friday v] of [ June v] In Lithuanian they need something like this: Every [ June v] [ Fourth v] [ Friday v] I'll try to fix it.
Assignee | ||
Updated•16 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Updated•16 years ago
|
Assignee: nobody → hubert
Status: ASSIGNED → NEW
Assignee | ||
Updated•16 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•16 years ago
|
||
Attachment #328263 -
Flags: review?
Assignee | ||
Updated•16 years ago
|
Attachment #328263 -
Flags: review? → review?(mvl)
Assignee | ||
Comment 2•16 years ago
|
||
Some small changes.
Attachment #328263 -
Attachment is obsolete: true
Attachment #328265 -
Flags: review?(mvl)
Attachment #328263 -
Flags: review?(mvl)
Assignee | ||
Updated•16 years ago
|
Flags: wanted-calendar0.9?
Updated•16 years ago
|
OS: Linux → All
Hardware: PC → All
Version: Trunk → unspecified
Updated•16 years ago
|
Flags: wanted-calendar0.9? → wanted-calendar0.9+
Assignee | ||
Updated•16 years ago
|
Attachment #328265 -
Flags: review?(michael.buettner)
Assignee | ||
Updated•16 years ago
|
Attachment #328265 -
Flags: review?(ctalbert)
Comment 3•16 years ago
|
||
Comment on attachment 328265 [details] [diff] [review] Patch 0.2 mvl, mickey and ctalbert are currently not actively doing reviews. For the dialog topics you probably want to ask philipp or berend, see <http://wiki.mozilla.org/Calendar:Module_Ownership>
Attachment #328265 -
Flags: review?(philipp)
Attachment #328265 -
Flags: review?(mvl)
Attachment #328265 -
Flags: review?(michael.buettner)
Attachment #328265 -
Flags: review?(ctalbert)
Comment 4•16 years ago
|
||
>+function monthlyWidgetsOrder() {
>+function yearlyWidgetsOrder1() {
>+function yearlyWidgetsOrder2() {
These functions are all quite similar, it would be nice to consolidate them into one function.
r=me with functions consolidated
Since our string freeze is coming up, I've created the patch for you. If you don't agree, please feel free to correct me and I'll check in a correction.
Attachment #328265 -
Attachment is obsolete: true
Attachment #329509 -
Flags: review+
Attachment #328265 -
Flags: review?(philipp)
Comment 5•16 years ago
|
||
Checked in on HEAD and MOZILLA_1_8_BRANCH -> FIXED
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → 0.9
Comment 6•16 years ago
|
||
Is there any reason why the string "of" is included as %3$S, and not just "of" in yearlyOrder2 ?
Comment 7•16 years ago
|
||
Yes. These strings are not real strings to translate, they just determine the order of elements which is changed when the dialog is loaded. The way this was implemented, it doesn't make sense to use hardcoded strings there.
Comment 8•16 years ago
|
||
> # Edit recurrence window -> Recurrence pattern -> Repeat montly
> # %1$S - ordinal, %2$S - month
> monthlyOrder=%1$S %2$S
%2$S is not month, but weekday?
Comment 9•16 years ago
|
||
> // This will place the node at the end
> element.parentNode.appendChild(element);
As for yearlyOrder2, this does not work correctly
because the parent of "yearly-weekday" and the parent of "yearly-month-rule" are not same.
Assignee | ||
Comment 10•16 years ago
|
||
(In reply to comment #8) > > # Edit recurrence window -> Recurrence pattern -> Repeat montly > > # %1$S - ordinal, %2$S - month > > monthlyOrder=%1$S %2$S > > %2$S is not month, but weekday? Yes, you are right. My fault. +# Edit recurrence window -> Recurrence pattern -> Repeat yearly +# %1$S - ordinal, %2$S - month +yearlyOrder=%1$S %2$S And here we have rather "day of month", not ordinal.
Comment 11•16 years ago
|
||
(In reply to comment #9) > > // This will place the node at the end > > element.parentNode.appendChild(element); > > As for yearlyOrder2, this does not work correctly > because the parent of "yearly-weekday" and the parent of "yearly-month-rule" > are not same. > I now understand what that part of Hubert's code was doing. Sorry for bashing it. I'll post a fix later today.
Updated•16 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 12•16 years ago
|
||
(In reply to comment #11) > > I'll post a fix later today. BTW - please fix my wrong localization notes (described in comment 9 and 10).
Assignee | ||
Comment 14•16 years ago
|
||
Comment on attachment 329671 [details] [diff] [review] patch 0.3.1 r=hubert
Attachment #329671 -
Flags: review?(hubert) → review+
Assignee | ||
Updated•16 years ago
|
Keywords: checkin-needed
Comment 15•16 years ago
|
||
(In reply to comment #13) > This patch should do it. If yearlyOrder2=%4$S %2$S %3$S %1$S, Every [ month ] [ weekday ] of [ ordinal ] is expected, but actually Every [ weekday ] [ month ] of [ ordinal ]
Assignee | ||
Comment 16•16 years ago
|
||
(In reply to comment #15) > (In reply to comment #13) > > This patch should do it. > > If yearlyOrder2=%4$S %2$S %3$S %1$S, > > Every [ month ] [ weekday ] > of [ ordinal ] > > is expected, but actually > > Every [ weekday ] [ month ] > of [ ordinal ] > I was checking this configuration and it was OK. Let me check again.
Assignee | ||
Comment 17•16 years ago
|
||
Comment on attachment 329671 [details] [diff] [review] patch 0.3.1 Thanks Atsushi, I was wrong.
Attachment #329671 -
Flags: review+ → review-
Assignee | ||
Updated•16 years ago
|
Keywords: checkin-needed
Comment 18•16 years ago
|
||
Sorry, didn't notice. Taking out the check to see if the element is already at the right place seems to do it. I've tested the following combinations: 1 2 3 4 2 4 3 1 2 4 1 3 4 2 3 1 4 3 2 1
Attachment #329797 -
Flags: review?(hubert)
Updated•16 years ago
|
Attachment #329671 -
Attachment is obsolete: true
Assignee | ||
Comment 19•16 years ago
|
||
Comment on attachment 329797 [details] [diff] [review] patch 0.3.2 r=hubert I've checked also 4 different configurations. Looks OK. Now I remember - I had similar problem with yearlyOrder2 - this is why I was removing all elements.
Attachment #329797 -
Flags: review?(hubert) → review+
Comment 20•16 years ago
|
||
1 4 3 2 (used in Lithuanian) doesn't seem to work either.
Assignee | ||
Comment 21•16 years ago
|
||
Rimas: WFM - please look at the screenshot.
Comment 22•16 years ago
|
||
(In reply to comment #21) > Created an attachment (id=329828) [details] > Edit recurrence window in Lithuanian > > Rimas: WFM - please look at the screenshot. Yes, this is correct. Possibly another bug, but the captions of the minimonths below in the same dialog should also be localizable. This also applies to the calendar widget in the Date tab in the left pane of the main window. There's a monthInYear entity defined in calendar.properties that could be used for this, but not necessarily.
Comment 23•16 years ago
|
||
Checked in on HEAD and MOZILLA_1_8_BRANCH -> FIXED
Status: REOPENED → RESOLVED
Closed: 16 years ago → 16 years ago
Resolution: --- → FIXED
Comment 24•16 years ago
|
||
Hubert, check this out please. I just downloaded the latest Tinderbox build from ftp://ftp.mozilla.org/pub/mozilla.org/calendar/sunbird/tinderbox-builds/latest-mozilla1.8-l10n/ , but it still manifests the ordering problem. I even extracted sun-dialog-event-dialog.properties to make sure the order is correct in there, and it is correct actually. Any ideas?
Comment 25•16 years ago
|
||
(In reply to comment #24) > I just downloaded the latest Tinderbox build Its build ID is 2008071520. Please wait the build after 2008071612.
Assignee | ||
Comment 26•16 years ago
|
||
I've check latest Linux build. WFM.
Comment 27•16 years ago
|
||
Thanks to both of you. I'll check again later then. :)
Comment 28•16 years ago
|
||
Works for me too now (build 2008071620), thanks!
Status: RESOLVED → VERIFIED
Comment 29•16 years ago
|
||
Comment on attachment 328263 [details] [diff] [review] Patch >Index: mozilla/calendar/locales/en-US/chrome/prototypes/sun-calendar-event-dialog.properties >+# LOCALIZATION NOTE: You can change the order of below params >+# Edit recurrence window -> Recurrence pattern -> Repeat montly >+# %1$S - ordinal, %2$S - month >+monthlyOrder=%1$S %2$S (very late comment, sorry). UIntil now, we tries to use the OS formatting for dates and times. But this patch seems like a switch away from that. That seems inconsistent to me. Is this really what is wanted? Should everything else also be changed to use date format from l10n, instead of the OS?
Comment 30•16 years ago
|
||
(In reply to comment #29) > (very late comment, sorry). UIntil now, we tries to use the OS formatting for > dates and times. But this patch seems like a switch away from that. That seems > inconsistent to me. Is this really what is wanted? Should everything else also > be changed to use date format from l10n, instead of the OS? I asked a similar question in some other bug. The answer is that not all Operating Systems are localized into all locales that Mozilla apps support. And even when they are, you can't always expect the user to use OS localized into his preferred language.
You need to log in
before you can comment on or make changes to this bug.
Description
•