220075 - Make task start date/due date optional.

Status: RESOLVED FIXED

(Calendar :: General, enhancement)

Description

[lonewolf32us]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4) Gecko/20030624
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4) Gecko/20030624

In many cases I just want to keep track of tasks I need to do.  These tasks do
not have a due date, nor are they at specific times, which the events are good
for.  Something like, "Make sure to fix Bug X when you have time." would be a
good example.  It is nothing that anyone is waiting for, it just needs to get
done eventually.
But in Calendar, if I leave that field empty, it makes the due date today, which
is not correct, and it mixes in with my other tasks that are actually due today.
 I would like to see the start date/due date be optional.

Reproducible: Always

Steps to Reproduce:
1. Create a new task
2. Set the due date field to [empty]
3. Click OK

Actual Results:  
Upon reopening the task, the due date is set to today, and the next date it is
"overdue" (colored red) which is incorrect.

Expected Results:  
Left the due date empty.

Comment 1

[Chris Charabaruk]

Agreed. I even have an idea of how the UI should be, I'll photoshop it tonight 
and attach it tomorrow.

Comment 2

[Simon Tyler]

If you are subscribed to a remote calendar and the VTODO imported does not 
contain a DTSTART and DTDUE element then the display and editing of the task 
does not work properly. Items like the summary and description are not shown on 
the edit window. Adding a DTSTARt and DTDUE resolves this!

Simon

Comment 3

[gekacheka]

RFC2445,sec4.6.2 says

   A "VTODO" calendar component without the "DTSTART" and "DUE" (or
   "DURATION") properties specifies a to-do that will be associated with
   each successive calendar date, until it is completed.

So undated todos should appear sorted with today's todos when sorted by start or
due date.

Start and due dates can be optional: 
  in todo dialog, just include a checkbox in front (like completed date).
  in tables, list blank if there is no date.

Comment 4

[gekacheka]

Attachment: toDoDialog.xul patch: add checkbox before start picker and due picker

adds checkbox between label and start datetimepicker, between label and due
datetimepicker, similar to completed date checkbox.

Comment 5

[gekacheka]

Attachment: toDoDialog.js patch: update to check for NODATE values; enable/disable start & due pickers on checkbox action

Add const kNODATE, the milliseconds value returned for the due date from a todo
object parsed from an .ics file VTODO which has no DUE field.  Thus VTODOs with
no DUE or START date field will correctly be interpreted as have no due or
start date.

loadCalendarEventDialog:  Check for NODATE value when initializing start date
and due date.  If value is NODATE, then corresponding picker is disabled but
initialized with current datetime (in case user decides to enabled it),
otherwise picker is enabled and initialized to date value.  

onOKCommand:  if Start date picker is enabled, then set the start date of the
TODO object to the picker value, otherwise set it to the NODATE value.	Ditto
for due date picker.

checkSetTimeDate:  only need to check that start date <= due date when both
start date and due date are enabled, otherwise no error.

add method onDateTimeCheckbox:	enables or disables the corresponding
datepicker, then calls updateOKButton to check for errors and enable/disable OK
button.

Comment 6

[gekacheka]

Attachment: unifinderToDo.js patch: handle NODATE values

formatUnifinderToDoDateTime:  if value is NODATE, return "" instead.

ToDoProgressAtom:  check for NODATE value before accessing each date.

toDoTreeView.getCellText:  
  date values:	call formatUnifinderToDoDateTime
  todo title:  Fix minor bug: Use "untitled" for null as well as "" titles.

dateToMilliseconds:
  use kNODATE const instead of literal.

patch is diff from results of patch to bug 250307.

Comment 7

[gekacheka]

Attachment: calendar.js patch: newToDoCommand: initialize to NODATE start, due dates

Initialize new TODOs with NODATE to disable initial start date and due date
(The dialog will initialize datetimes to now if user choose to enable them.)

Comment 8

[gekacheka]

Tested on Moz1.7 and sunbird (1.8).

Bug 250307 fixes problem where only due dates, not due times were shown in todo
list.

Bug 250824 fixes problem where checkbox only enables/disables timepicker part
and not datepicker part of datetimepickers.

Suggested patch order and tests:
  toDoDialog.xul:  
    adds checkboxes in front of start & due dates in todo dialog
  toDoDialog.js:
    each checkbox enable/disable corresponding date and time picker.
    a todo saved with disabled date still has date disabled when edited.
  unifinder.js:
    todos with disabled start & due dates display with blank dates in list.
  calendar.js:
    new todo is created with start and due dates disabled.

Comment 9

[gekacheka]

Attachment: unifinderToDo.js patch: handle NODATE values

See comment #6.  Previous was diff without patch from bug 250307.

Comment 10

[Mostafa Hosseini]

Patches checked in.
Will leave bug open for:
- backend to remove DTSTART and DTDUE if they are not set
- frontend to disable features that depend on an existing start date ( e.g.
recurrence )

Comment 11

[gekacheka]

Attachment: unifinderToDo.js.patch: remove duplicate formatUnifinderToDoDateTime()

to remove strict javascript warning:

Warning: redeclaration of function formatUnifinderToDoDateTime
Source File: chrome://calendar/content/unifinderToDo.js
Line: 260

Patch removes second definition, uses its body as first definition (next to
similar functions).

Comment 12

[Mostafa Hosseini]

Attachment 155142 [details] [diff] checked in. Thanks

Comment 13

[gekacheka]

Because start checkbox and repeat checkbox are on separate tabs, the interaction
may not be apparent, and requires a warning message.

Design options for start checkbox / repeat checkbox interaction:

A. If start checkbox is unchecked, disable repeat checkbox, and leave ok enabled
even if repeat was checked.

Problem A1: it is not clear why repeat checkbox is disabled.

Problem A2: if someone sets start, then sets a repeat, then unchecks start date
(maybe shifting it to due date), they may not realize that the repeat has been
disabled (data loss).

B. If start checkbox is unchecked, disable repeat checkbox with a warning
message explaining start is required, and leave ok enabled even if repeat was
checked.  (Fixes problem A1 by adding warning message.)

Problem B1:  other warning messages only occur when ok is disabled, so having a
warning message that is irrelevant to ok is confusing, especially if there is
another problem which disabled the ok.

C. If start date is unchecked, repeat is disabled, and if repeat was checked
warning is shown and ok becomes disabled.  (Fixes A2 by disabling ok so user
must fix warning before saving todo.  Fixes B2 since warning appears only when
ok disabled.)

Problem C1:  cannot simply uncheck repeat to fix problem, because it is
disabled. (Have to enable start, disable repeat, then disable start.)

D. If start date is unchecked and repeat is checked, warning is shown and ok
disabled.  (Fixes C1 because repeat checkbox is never disabled.)

Comment 14

[gekacheka]

Attachment: toDoDialog.xul patch: add label for recurrence start date warning (checked in)

Adds label for start date warning, which appears only if repeat is checked and
start date is not checked.

Comment 15

[gekacheka]

Attachment: toDoDialog.js patch: control visibility of repeatstart date warning (checked in)

[loadCalendarEventDialog strict nits:
 remove "var recurEndDate ..." because it is not used (redeclared later)
 add "var" to declare "ExceptionTime"]

checkSetRecur modified: 
* calls setRecurStartError if repeat checkbox is checked but start date is not
checked, and 
* calls setRecurUntilError if until date is before due date.   

checkSetRecur is called by updateOkButton, so it is called whenever either
checkbox changes state, and when either due or end date is changed.

Comment 16

[gekacheka]

Tested on Mozilla 1.7.2, Mozilla 1.8a2, Firefox 0.9.3

Test:

New Task dialog (file menu)

Task tab:
  Leave start date unchecked to start

Recurrence tab:
  check repeat-every (to checked)
    Warning should appear (problem A1 addressed)
      "start date required for recurrence"
    Ok button should disable
  uncheck repeat-every
    Warning should disappear (problem B1 addressed)
    Ok button should enable (problem C1 addressed)

Task tab:
  Click start date checkbox to checked

Recurrence tab:
  check repeat-every (to checked)
    Warning should not appear
    Ok button should stay enabled
  uncheck repeat-every
    Warning should not appear
    Ok button should stay enabled
  Check repeat-every (to checked, prepare for next step)

Task tab:
  Uncheck start date checkbox
    Ok button should become disabled (problem A2 addressed)
  Check start date checkbox
    Ok button should become enabled

Comment 17

[gekacheka]

Attachment: eventDialog.js patch: apply parallel cleanups like todoDialog.js patch (checked in)

[loadCalendarEventDialog strict nits: 
 * remove "var recurEndDate = ..." as it is not used (redeclared later)
 * Add "var" to declare "ExceptionTime"]
[onOkCommand:
 * remove out-of-place comment "check that the repeat end time is later..."]

checkSetRecur: clarify code as in toDoDialog.js, and add comments about special
case for same year,month,date.

Comment 18

[gekacheka]

Attachment: calendar.dtd pach: add newtodo.repeatstart.warning (all locales), rewords newevent.recurend.warning in en-US (checked in)

Comment 19

[Mostafa Hosseini]

All new patches checked in. Thanks

Comment 20

[gekacheka]

Bug 263539 takes care of disabling alarms in the todo dialog when no dates.

Comment 21

[gekacheka]

Attachment: toDoDialog.js patch: use todo.due.clear() (checked in)

To omit start/due times for ical output, omitted dates need to be set to the
icaltime_null_time value, which does not correspond to any millisecond time,
but can be set by calling the oeIDateTime.clear() method.
http://lxr.mozilla.org/mozilla/source/calendar/libxpical/oeDateTimeImpl.cpp#202



NEEDED:  oeIDateTime.isSet() method, I think something like

NS_IMETHODIMP oeDateTimeImpl::GetIsSet(PRBool *retval)
{
     *retval = ! icaltime_is_null_time(m_datetime);
     return NS_OK;
}

oeIICal.idl oeIDateTime needs something like

   readonly attribute boolean isSet;


Currently using the NODATE workaround, but the NODATE hack for detecting a
cleared date is broken because the oeDateTime.getTime() returns the msecs
adjusted to the local timezone offset.	Adding a counter-adjustment for the
current timezone offset just makes it more of a kludge (should be recalculated
each time in case timezone offset changes due to travel, or due to begin or end
of daylight/summer time).  Adding something like this isSet() method is much
tidier and makes the code easier to understand.

Comment 22

[gekacheka]

Attachment: oeDateTime patch: add isSet attribute ( checked in )

Proposed addition in form of a patch.

Comment 23

[Mostafa Hosseini]

Comment on attachment 162945 [details] [diff] [review]
oeDateTime patch:  add isSet attribute ( checked in )

Patch checked in. Thanks

Comment 24

[Mostafa Hosseini]

Comment on attachment 161714 [details] [diff] [review]
toDoDialog.js patch:  use todo.due.clear() (checked in)

Patch checked in. Thanks

Comment 25

[gekacheka]

Attachment: patch to use oeICalDateTime.isSet/.clear() instead of kNODATE, oeICalDateTime_isSet

calendar.js
  remove k_NO_DATE (no longer used, it was not timezone-general)
dateutils.js
  remove oeICalDateTime_isSet (no longer used or needed, 
    it was workaround until oeICalDateTime.isSet was implemented)
eventDialog.js
  on loading, test for event.recurEnd.isSet before using date, else use now.
  on storing, if recurEnd not used, then event.recurEnd.clear()
mouseoverPreviews.js
  replace oeICalDateTime_isSet(oedate) with oedate.isSet
toDoDialog.js
  remove kNODATE (no longer used, it was not timezone-general)
  moved gStartDate and gDueDate next to gDuration
  replace:  event.start/due.getTime() != kNODATE 
     with:  event.start/due && event.start/due.isSet
  on loading, test completed.isSet, recurEnd.isSet before using, else use now.
  on storing, clear recurEnd if not used (completed is already cleared).
unifinderToDo.js
  remove kNODATE (no longer used, it was not timezone-general)
  replace:  oeICalDateTime.getTime() != kNODATE 
     with:  oeICalDateTime.isSet
  ToDoProgressAtom:  rewritten to use oeICalDateTime.isSet
  dateToMilliseconds:  rewritten to use oeICalDateTime.isSet

Comment 26

[gekacheka]

Bug 268167 created for subproblem: cannot store a task with a start date and no
due date (due date is incorrectly set to start date).

Comment 27

[gekacheka]

Comment on attachment 164935 [details] [diff] [review]
patch to use oeICalDateTime.isSet/.clear() instead of kNODATE, oeICalDateTime_isSet

(Patch in attachment #164935 [details] [diff] [review] is independent of bug 268167.)

Tested as follows:  (#5,#6 are visible changes, depend on step #1.)

INITIALIZE OS TIMEZONE
1. Set OS timezone to offset DIFFERENT FROM Eastern US/Eastern Canada,
   say, Central US/Canada.  (Old kNODATE code in task table worked
   only in eastern time zone, so use another timezone to verify fix).
2. Restart platform (Moz/TB), then start Calendar.
INITIALIZE TASKS
3. Create test tasks:
  a. Create a task "Filled dates", and provide a start
     date, due date, and completed date, all set to times yesterday.
     Click OK to save.
  b. Create a task "No dates" (disabled start date, due
     date, and completed date).  Click OK to save.
TASK TABLE
4. In task table, enable columns for Start Date, Due Date, Completed (Date)
   a. dates for "Filled dates" are filled.
   b. dates in task table columns for "No Date" are empty.
   Before this fix, "No dates" task appeared RED (overdue), and task
   table columns for Start Date, Due Date, Completed for task "No
   dates" were NOT empty (set to date long ago, in year -1).  (On
   windows the date is out of range for the OS date formatter, so only
   the time of 00:00 appears)
TASK DIALOG
5. Open tasks again
   a. Double click "Filled dates" task.
      * Start date is checked and set to time yesterday.
      * Due date is checked and set to time yesterday.
      * Completed date is enabled and set to time yesterday.
      * Recur end date is disabled, but initialized to end date.
      Close dialog.
   b. Double click "No Dates" task.
      * Start Date is not checked, and disabled date is now.
      * Due date is not checked, and disabled date is now.
      * Completed date is disabled and set to now.
      * Recur end date is disabled, but set to now.
      Close dialog.
   Before this fix, Start date and Due date for "No Dates" would be
   checked on and set to dates in year -1.  (On windows, the dates are
   out of range for the OS date formatter, so today's date appears).  
MOUSEOVER PREVIEW
6. In task table.  Behaves same both before and after this change.
   * Hover Mouse over task "Filled dates".  All three date headers appear.
   * Hover mouse over task "No dates".	No date headers appear.
INITIALIZE EVENTS
7. Create test events
  a. Create an event with title "Recurring event", provide a
     start date and end date sometime two days' ago, and repeats
     every 1 week until yesterday's date.  Click OK to save.
  b. Create an event with title "Non-recurring event", provide a
     start date and end date yesterday.  Click OK to save.
EVENT DIALOG
8. Open events again.  Behaves the same before and after this change.
  a. Double click "Recurring event"
     * Repeat until date is set to yesterday.
  b. Double click "Non-recurring event"
     * Repeat until date is disabled and set to yesterday.
RESET OS TIMEZONE
9. Set timezone back to local timezone.

Comment 28

[Mostafa Hosseini]

Comment on attachment 164935 [details] [diff] [review]
patch to use oeICalDateTime.isSet/.clear() instead of kNODATE, oeICalDateTime_isSet

Checked into trunk and 0.2 branch. Thanks.

Comment 29

[gekacheka]

Still waiting for bug 268167.

Comment 30

[gekacheka]

Looks fixed.