807956 - Event window: Early end date warning locks event edit window

Status: RESOLVED FIXED

(Calendar :: Lightning Only, defect)

Description

[jrf.maillist]

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:16.0) Gecko/20100101 Firefox/16.0
Build ID: 20121024073032

Steps to reproduce:

Trying to enter a new event from a converted task (not sure if the latter is relevant)


Actual results:

When the window opened there were start and end times/dates already entered by default. First I changed the start date, since it's the top item, of course. However, as soon as I then clicked on the end date field to change it, the warning popped up "The end date you entered occurs before the start date". This warning box takes focus and the only active control is its OK button.  However when you press the OK button it just reappears and won't go away.  The only way to get rid of it is to close and reopen Thunderbird.


Expected results:

It's okay for the warning box to pop up once, but then it should close when OK is clicked and not reappear.  Better would be that any checks for errors in event window are only done when saving it, rather than checking for errors on a partially completed window - of course there are going to be errors at this stage.

Comment 1

[Decathlon]

Attachment: Patch only for the wrong task-to-event conversion

I can confirm this issue about the bad behavior of the warning dialog when the New Event dialog has been filled out with an end date earlier than the start date and this could happen e.g. by loading a not well formatted event in an ics file. So the issue has to be handled in a general way.
Keeping in count the comments e.g in bug 410427 comment 21, bug 702613 comment 4, bug 350463 comment 4 ... a decision should be taken about a fix that correct only the warning dialog or a different approach.


Though, there is also a problem when converting a task into an event as initially reported by jrf.maillist.
It's reproducible always by converting a task with only a *due date in the past* (yesterday or earlier if you convert the task from any view or in task mode, or a date earlier than the date selected in the today-pane if you convert the task from there).
A such task, when converted, creates an even with an end date earlier than the start date because the latter gets the default start value (today, or the date in the today-pane) since it doesn't exist in the original task. This will cause the bug when trying to modify the end date.
The patch solves only this case and could be applied quickly.

Comment 2

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 677996 [details] [diff] [review]
Patch only for the wrong task-to-event conversion

Review of attachment 677996 [details] [diff] [review]:
-----------------------------------------------------------------

r=philipp

::: calendar/base/content/calendar-dnd-listener.js
@@ +140,5 @@
>  
>          // Dates and alarms
>          item.startDate = aTask.entryDate;
>          if (!item.startDate) {
> +            if (!aTask.dueDate) {

If you change the order of the if/else part, then you save one ! operation. JS might optimize this away, but just in case :)

@@ +141,5 @@
>          // Dates and alarms
>          item.startDate = aTask.entryDate;
>          if (!item.startDate) {
> +            if (!aTask.dueDate) {
> +                item.startDate = getDefaultStartDate();

Please use cal.getDefaultStartDate() instead.

Comment 3

[Decathlon]

Attachment: Patch only for the wrong task-to-event conversion - v2

Patch with fixes requested in previous comment.

Pushed to comm-central changeset 0d15ba91f5a9

Comment 4

[Decathlon]

Attachment: Patch for the wrong behavior of the warning dialog

With the actual warning dialog implementation, the wrong behavior foremost depends on the datetimepicker binding which fires the "change" event even with actions that don't change the date-time. For example, clicking on the drop-down button fires the "change" event twice; the timepicker fires "change" (again twice) by simply losing the focus;

The "change" event causes the check of the end date that, if wrong, is being substituted by the previous date (even if it was wrong), afterward the warning dialog appears.

In a case like this bug, the datetimepicker is pre-filed with a wrong end date, so the warning dialog appears just after a click on the datepicker's drop down button and before the user can select a date. In the case of the timepicker (when the date is wrong), closing the warning dialog moves away the the focus and hence a new warning dialog immediately appears so Thunderbird get stuck.

This patch changes datetimepickers behavior in order to generate only one "change" event and only if a date/time has changed. It also disables the Save and Save&Close commands when the warning dialog is showed. 

Since the previous patch for this bug has been checked in, to try this one we have to load events with wrong end dates like the one in the calendar attacked in the following comment. The event doesn't appear in the views, but it can be found in the unifinder.

Comment 5

[Decathlon]

Attachment: calendar with one event with wrong end date

Comment 6

[Decathlon]

Forgot to say that this patch needs patch for bug 410427 being applied before.

Comment 7

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 696901 [details] [diff] [review]
Patch for the wrong behavior of the warning dialog

Review of attachment 696901 [details] [diff] [review]:
-----------------------------------------------------------------

r=philipp

::: calendar/base/content/dialogs/calendar-event-dialog.js
@@ +647,5 @@
>          gWarning = true;
> +        let callback = function func() {
> +            Services.prompt.alert(null,
> +                                  document.title,
> +                                  calGetString("calendar", "warningEndBeforeStart"));

cal.calGetString()


we should really rename that function some day... :)

Comment 8

[Decathlon]

(In reply to Philipp Kewisch [:Fallen] from comment #7)

> cal.calGetString()

Changed in the pushed patch.

Pushed to comm-central http://hg.mozilla.org/comm-central/rev/db8918e79907

FIXED