Closed Bug 418115 Opened 12 years ago Closed 12 years ago

wrong start & end time for new events (new event button on today pane)

Categories

(Calendar :: Lightning Only, defect, minor)

defect
Not set
minor

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: maxxmozilla, Assigned: Fallen)

Details

Attachments

(1 file, 3 obsolete files)

Attached patch eventStart.hour++; (obsolete) — 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?
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 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.
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?
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.
Flags: wanted-calendar0.8? → wanted-calendar0.8+
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)
Assignee: firefox → philipp
Status: ASSIGNED → NEW
OS: Windows 2000 → All
Hardware: PC → All
Status: NEW → ASSIGNED
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.
> >     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...?)
This should be better.
Attachment #303990 - Attachment is obsolete: true
Attachment #304319 - Flags: review?(ssitter)
Attachment #303990 - Flags: review?(ssitter)
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+
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)
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+
Checked in on HEAD and MOZILLA_1_8_BRANCH

-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 0.8
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.