Closed
Bug 418115
Opened 16 years ago
Closed 16 years ago
wrong start & end time for new events (new event button on today pane)
Categories
(Calendar :: Lightning Only, defect)
Calendar
Lightning Only
Tracking
(Not tracked)
VERIFIED
FIXED
0.8
People
(Reporter: maxxmozilla, Assigned: Fallen)
Details
Attachments
(1 file, 3 obsolete files)
2.45 KB,
patch
|
ssitter
:
review+
|
Details | Diff | Splinter Review |
New event start time shows current full hour instead of next full hour.
calendar-item-editing.js:
> // The time for the event should default to the next full hour
Tested on Lightning 0.8pre (build 2008021620)
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1.9) Gecko/20071031
Thunderbird/2.0.0.9 ID:2007103104
Attachment #303898 -
Flags: review?(daniel.boelzle)
Flags: wanted-calendar0.8?
Assignee | ||
Comment 1•16 years ago
|
||
Comment on attachment 303898 [details] [diff] [review] eventStart.hour++; Since daniel is on vacation, taking over review. Thanks for catching this, r=philipp
Attachment #303898 -
Flags: review?(daniel.boelzle) → review+
Comment 2•16 years ago
|
||
Comment on attachment 303898 [details] [diff] [review] eventStart.hour++; > var eventEnd = eventStart.clone(); > eventEnd.hour++; > createEventWithDialog(getSelectedCalendar(), eventStart, eventEnd) This looks like an regression. Or is the corresponding bug still open? No end date should be pasted to createEventWithDialog() to create an event with the users preferred default event length.
Assignee | ||
Comment 3•16 years ago
|
||
I was just about to comment something similar. I think it would be best if createEventWithDialog automatically set up the eventStart and eventEnd if it is null, or automatically set up eventEnd if only eventStart is specified. Przemyslaw, could you take care of doing this and making sure that everything calling createEventWithDialog calls correctly?
Comment 4•16 years ago
|
||
I was referring to Bug 390300 in Comment #2. If no start date is passed to createEventWithDialog() it will use the day selected in the main calendar view instead. The end date is calculated based on the user preference calendar.event.defaultlength. In case of the Today Pane I think it's OK to pass in today as start date but omit the end date because there is no visible relation to the day selected in the main calendar view.
Updated•16 years ago
|
Flags: wanted-calendar0.8? → wanted-calendar0.8+
Assignee | ||
Comment 5•16 years ago
|
||
This patch removes eventEnd, leaving the next hour part to createEventWithDialog(). It also removes an agenda function that is not used anywhere in the codebase.
Attachment #303898 -
Attachment is obsolete: true
Attachment #303990 -
Flags: review?(ssitter)
Updated•16 years ago
|
Assignee: firefox → philipp
Status: ASSIGNED → NEW
OS: Windows 2000 → All
Hardware: PC → All
Updated•16 years ago
|
Status: NEW → ASSIGNED
Comment 6•16 years ago
|
||
The patch looks good but I still have the feeling the New Event logic is wrong:
>+ // Create a new event "today". Setting isDate = true
>+ // automatically makes the start time be the next full hour.
> var eventStart = now();
> eventStart.day = this.today.start.day;
From testing it seems that this.today.start corresponds to the date that is selected in the Agenda (and might be different from today by the way). Pressing the New Event button creates an event in the current month and year but with the selected day. For example if 07-Mar-2009 is selected the event will be created for 07-Feb-2008. In my opinion this doesn't make sense.
Either the event should be created always for today or for the date that is displayed in the Agenda. I would prefer the second option.
Reporter | ||
Comment 7•16 years ago
|
||
> > eventEnd.hour++;
doh! I should have noticed this but I did forget about the preference...
I agree with Stefan last comment and I would propose to add:
eventStart.month = this.today.start.month;
eventStart.year = this.today.start.year;
to the patch, with these changes we could fix 3 bugs at one time
(wondering if date selected in the Agenda should also affect proposed dates for tasks created / modified there...?)
Assignee | ||
Comment 8•16 years ago
|
||
This should be better.
Attachment #303990 -
Attachment is obsolete: true
Attachment #304319 -
Flags: review?(ssitter)
Attachment #303990 -
Flags: review?(ssitter)
Comment 9•16 years ago
|
||
Comment on attachment 304319 [details] [diff] [review] Get rid of eventEnd and fix eventStart - v2 In my opinin the comment should be corrected like 'create new event on the date selected in the agenda ...', otherwise r=ssitter
Attachment #304319 -
Flags: review?(ssitter) → review+
Reporter | ||
Comment 10•16 years ago
|
||
Adjusting bug description based on latest patch.
Summary: wrong start time for new events (new event button on today pane) → wrong start & end time for new events (new event button on today pane)
Updated•16 years ago
|
Keywords: checkin-needed
Comment 11•16 years ago
|
||
Phillipp's patch ready for checkin with code comment adjusted according to Comment #9; carry over r=ssitter
Attachment #304319 -
Attachment is obsolete: true
Attachment #305549 -
Flags: review+
Comment 12•16 years ago
|
||
Checked in on HEAD and MOZILLA_1_8_BRANCH -> FIXED
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 0.8
Comment 13•16 years ago
|
||
Checked in nightly build 2008030318 -> task is fixed and verified.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•