Investigate in-view task UI of tasks with missing date or due date

RESOLVED FIXED in 2.4

Status

defect
RESOLVED FIXED
13 years ago
7 years ago

People

(Reporter: ulf.stroehler, Assigned: bv1578)

Tracking

unspecified

Details

Attachments

(5 attachments, 2 obsolete attachments)

REPRODUCTION:

1. select from Menu: "View/Tasks in View"
2. "File/New Task..."
3. set Title and Due Date
4. save task with OK

Actual Result:

the task is not displayed in the views

Expected Result:

Task should be displayed in the Views
or the Task Dialog should not allow to only set a Due Date without a Start Date
This simple patch allows the calendar to view the tasks in the month views, but not the day/week views.

Still working on that part.
Assignee: nobody → Pidgeot18
confirmed for branch too, not just trunk... 
Version: Trunk → unspecified
(In reply to comment #1)
Joshua, is this just a work-in-progress copy of your patch? Or is this patch finalized? In the latter case you should request review from a developer in charge (see <http://wiki.mozilla.org/Calendar:Module_Ownership>).
It's just a work-in-progress. I now have week but not day view working on my computer; I stopped working on it for a while due to the fact that Lightning renders TB difficult to use on my setup. I will hopefully have something done by the end of the month.
Status: NEW → ASSIGNED
Hardware: PC → All
Joshua, thanks for the help. Let us know if you have any questions or if we can help with anything.  You can ask us interactively on IRC in #calendar (irc.mozilla.org)
This bug does still exists in RC1.
Moreover I have the problem, that I had to toggle the 'completed' flag to force showing of any task.
Duplicate of this bug: 405562
Duplicate of this bug: 411846
isn't this a dupe of bug 405459? 
BTW, no option to set tasks in calendar-view anymore since the task-mode?
> isn't this a dupe of bug 405459? 
No. This bug is about the view issue and bug 405459 is about the ics provider regression issue.
Joshua, are you still working on this issue?
I apologize, but the answer is no. Lightning on debug trunk Thunderbird builds has way too many issues for me to try developing it right now...
Assignee: Pidgeot18 → nobody
Status: ASSIGNED → NEW
Component: Tasks → Calendar Views
QA Contact: tasks → views
how do we want the task with only due date to be shown in day/week view?

one solution that I can see is to have the start date = due date as soon as due date is fixed, kind of what is done when adding a alarm to a task with only due date.

that is causing another problem, since both date are equal the task in view is shown so thin that we cannot select it (but that is probably another bug.
In this scenario a task should be shown as an all-day event:
http://tools.ietf.org/rfcmarkup?doc=draft-ietf-calsify-rfc2445bis-07#section-3.6.2
Examples:  The following is an example of a "VTODO" calendar component that needs to be completed before May 1st, 2007.  On midnight May 1st, 2007 this to-do would be considered overdue.
        BEGIN:VTODO
        UID:20070313T123432Z-456553@example.com
        DTSTAMP:20070313T123432Z
        DUE;VALUE=DATE:20070501
        SUMMARY:Submit Quebec Income Tax Return for 2006
        CLASS:CONFIDENTIAL
        CATEGORIES:FAMILY,FINANCE
        STATUS:NEEDS-ACTION
        END:VTODO
 comment #14)
> In this scenario a task should be shown as an all-day event:
> http://tools.ietf.org/rfcmarkup?doc=draft-ietf-calsify-rfc2445bis-07#section-3.6.2

First I don't understand how you can say that with the link you gave(In reply to

Second, what about event that is due to 6pm, we cannot show it as an all-day event (task??)
From RFC2445:
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.

Of course a task which ends at a certain time shouldn't be active (should be overdue) at that time.
so in sumary:
-in multiweek and month view we just show the task on the end day that it has a end time or not.
-a task that doesn't have start date and end time but have a end date is shown as a "all day task" on that day.
-a task that doesn't have start date but have end time (and day) have to stop at that time... but when are they starting (in term of UI in the view)?

so all problem are solved except the last one and it's the one that I can't figure out as how to show it in the day/week view. I see 2 way and both are problematic in some way:
-the task start at the begening of the day and stop when it is finish. but I wouldn't like to have a block that take almost all the space of a day just because it has to be finish a 6pm.
-we just show a one line block at the finish time but it's could get confusing for the user.
How about to fade out.
I mean: 100% opacity at the finish time (FT) and 0% opacity at FT-2h.

Don't know how difficult this would be and if it fits in the concept, but maybe a possible solution.
well I like that idea it's solve most of the problem I saw we just have to check with the programmers if it's possible to implement but I think it's would be a great way of making it.
I think, a Task with only a due date should be shown as all day on the current till the due date is reached and on the Due date to the given time.
I think it's a little bit confusing too because we change the organization without user action. if the user look at his calendar and see a task for today (on the all day section) it's tell him I've to do that today...now does it have a due time?? he need to check deeper for that info. next, he come back a few hours later and start searching is task that was in the all day section, but it isn't there anymore... he finally find it somewhere in the day calendar not really understanding why it's there now...

I would like to hear about more experimented developer, is the fade-out solution implementable without to much trouble?
please see the discussion in mda:
http://groups.google.com/group/mozilla.dev.apps.calendar/browse_thread/thread/34d3f50b7762003b/6ec5789f1d58a397?#6ec5789f1d58a397 
and have discussions there... Fade-out is a feature which might depend on this bug but certainly isn't the same imho. 
Sadly the discussion about this bug has flawed down....

I think, that the way the tasks are presented in the views is not usable. When I have a task due in a couple of months,  it is displayed every day. This is terrible in all views...

I agree to this comment :
http://groups.google.com/group/mozilla.dev.apps.calendar/msg/4c20d383f33efce5

- Have an entry on the start date
- Have an entry on the due date
- Tasks without dates don't show up in the views

I think this is the easiest solution from an user's point of view.

Hope to get that discussion going again!
(In reply to comment #23)
> Sadly the discussion about this bug has flawed down....
> 

well I was working on summarizing all the idea but I got out of time, I'm now on holidays, so I should finish that up soon and put everything on wiki for easier overview. 
Perhaps tasks without a date should be shown "today", until it is marked completed.

Also, see my suggestions in bug 339955 comment 10.
Duplicate of this bug: 339955
Summary: tasks with only a Due Day set, are not displayed in the views → Investigate in-view task UI of tasks with missing date or due date
My suggestions from bug 339955 comment 10, since it is now a dupe of this one:

Proposed order: (do it like Lotus Organizer - all hail Lotus Organizer)

- order by due date

- then, if no due date, order by start date

- then, if no date at all, order by creation date (or, if that is not possible,
just put them below the other tasks / no date = less urgent)

- put all the tasks above all the events

- sort the soonest due/start/creation dates at top

BTW: I think showing tasks that have a due date that is days later than the
start date on every day in between is distracting and a waste of space. This is
particularly problematic with tasks that run for long times. A solution might
be to make the due date the default selection, and let the user set the alarm
for well in advance. I don't really see much use for a start date anyhow.

BTW2: Perhaps tasks can be further differentiated from events by displaying
them without a colored background box, and, instead, color the task's text in
the calendar's color - or just put a calendar-colored box around the task icon
on the left and leave the text black and without a background color.
(In reply to comment #25)
Duplicate of this bug: 489383
Wow -- This one's been around since 2006?

I just entered Bug 489383, which is related to Bug 479578 and this Bug 349529.

My suggestion? Quit thinking so hard about this one and do what's done as in Bug 479578. 

The application response to that bug is to set the Start date implicitly (greyed) to the Due Date. This solves the problem -- the task becomes visible in all calendar views.

As users, if we're not entering anything in the Start date fields, then we really aren't concerned with that attribute -- our concern is the due date, and our ability to set alarms with respect to the due date, and of course (as is the case here), our ability to see the task on all calendar views.

Hope this helps...
Stefan, I see you've marked my Bug 489383 as duplicate to this Bug 349529.  That is all well, as I expected that action.  I felt it was necessary to submit it anyway, as it took a bit of searching on my part before I came across this report.

You made the comment in Bug 489383, "If you set an alarm related to the due date only the due date is required. There is no need to enforce a start date."

I understand your meaning, but it certainly is a means to facilitate an expected result from the application, based on the way the application [at present] processes certain inputs.

With respect to the current version of Sunbird, if I create a task, set the Due Date, and select one of the preset alarms (which don't state whether they are with respect to event start/end, the application implicitly sets the Start Date to the Due Date.  Is that not enforcement?  I don't see it that way -- the checkbox is checked, and it's greyed out.  I can still override those Start Date values if I so desire.

I say do the same thing here. As soon as one sets the Due Date, implicitly set the Start Date in the same manner. Could that be confusing for some users? Possibly. But we're fast learners, and as soon as we see it on the calendar, I'm pretty sure we'll get it.
Duplicate of this bug: 489952
Be advised that tasks with a start date but no due date do not show up in the day or week view but do show in the multiweek and month view as being on the start date. Let me know if I should enter a new bug for this - it seems pretty closely related.
Regarding comment #33, I'm attaching a patch that fixes the problem for me. Perhaps changing the && to || instead of completely removing the test for the dueDate would fix it for both cases.
with lightning 1.0b2pre I do not see tasks with start but no due date on any calender view any more: I did see them on the start day in lightning 0.9, and that was ok for me as. I used it for todos that had to be postponed for a certain period and it was very handy to see the date the work on them had to be started again. Now I lack any visual reminder ...
With Lightning 1.0b2, I too cannot see tasks in the calendar views, unless they have both a start and a due date. It would be useful to have a way of defining tasks with only a start date (and no time), visible together with all-day events. Currently, I find myself defining such tasks as starting and ending at 8:00, to be able to see them on the week view. Problem is, sometimes two recurring tasks will coincide on the same day, and then they appear too small, side by side...
I have also seen this, (Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.9) Gecko/20100825 Lightning/1.0b2 Thunderbird/3.1.3).  I also do not believe that a Due Date should be required to display the task on the Calendar.  It looks like there has been a large amount of discussion on this topic but no "best" solution.  My suggestion is give the user the option in the Lightning Preferences to:
   * allow display of tasks with out Due Dates as single event on Start day (default), or
   * don't display tasks with out Due Dates or Start dates, or
   * display tasks on each day between Start Date and Due Date, or
   * <any other I have not thought of>

This will give the user the choice on how to display and will cause less confusion when moving from 0.9 to 1.0.

Also looking at http://www.rumblingedge.com/ (don't know that it is the full list, but just a nice one I found) I don't see this as being a stopper for 1.0+, I would call it so, just because functionality (all tasks with out Due dates will not show up in calendar views) changed between 0.9 and 1.0 and will confuse users (as can be seen by the number of duplicate issues reported above, and we don't know the number of people that saw this bug report started adding due dates so don't care any more).  And it probably should not be considered "New" being that it is over 4 years old... ;-)
Suggestion:
- Always put tasks in the "all day" area at the top of the day and week views. Exception: If a task is due "today" *and* if the task has a "time", then either show the task within the timeline in the day and week views *or* put a clock icon on the task in the "all day" area.
- Only show the task "today". Don't show tasks on every day between start and due dates. This needlessly clutters the view. It's the main reason I don't use tasks in Lightning!
- Color the task's text and/or background based on whether the task is "not yet due" (green), "due today" (orange), "overdue" (red), "no due date" (black).
- Make the due date the default entry field (Tasks almost always start "now" - they are not events, and they are definitely not Gantt-Diagrams!).

PS. The 7-year-old *Lotus Organizer* still rules over all! Let's change that.

BTW: Any news on Google offering CalDAV Tasks?
I see that no decision has been taken yet about this bug. What about starting, at least, with showing in the views, at their date-time, the tasks that have only one of the two dates, nothing more nothing less?
In this patch I've considered such tasks like events with zero length, hence in day\week views their vertical size depend on the character size inside the event-box (a single line).
Since they are different from normal tasks with both dates, I've added an icon with a downward arrow that should explain the type (start or end date as shown in the next screenshot). I've also changed the behavior for the drag and drop in day/week view, in particular I've hidden the grip bars on the event-box in order to allow only moving but not "stretching" the task, and changed one of the two drag-labels with a string "Start\Due time".

I'd like a behavior as mentioned in comment #39 about multiple days tasks, but I've seen that is more complicated to implement and, in any case, it would need beforehand a decision about the way it should work.

Philipp, could you give me your opinion about this patch?

I've also loaded two xpi files based on the 21/12/2010 build (Lightning 1.0b3pre) for Windows and Linux (the last one without testing), so I'd be glad if someone wanted to try it (on a new Thunderbird profile):

http://www.mediafire.com/file/2c1bxdghotp3681/lightning1.0b3pre-20101221-Bug349529-Win.xpi 
http://www.mediafire.com/file/o1zx4i8pe9ity32/lightning1.0b3pre-20101221-Bug349529-Linux.xpi (without test)
Attachment #501676 - Flags: feedback?(philipp)
I think, this patch really is a step forward.
Although I agree with Decathlon, that the behaviour mentioned in Comment #39 would be even more desirable, it is way better then the current.
Comment on attachment 501676 [details] [diff] [review]
proposal - only display task without entry OR due date

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

Looks fine to me, I agree we should start out with this. Codewise I'd give r+ with the following comments considered.

::: calendar/base/content/calendar-month-view.xml
@@ +75,5 @@
>                                 class="calendar-item-image"
>                                 xbl:inherits="progress,allday,itemType"/>
> +                    <xul:image anonid="startEnd-icon"
> +                               class="calendar-item-image2"
> +                               xbl:inherits="type"/>

Will there be a situation where both the item icon and the startEnd icon are displayed? If not, then I'd rather just use one xul:image here.

::: calendar/base/content/calendar-multiday-view.xml
@@ +902,3 @@
>                start = start.getInTimezone(self.mTimezone);
>                aEventInfo.layoutStart = start;
> +              let end = item.endDate || (item.dueDate || item.entryDate);

At least logically, no need for extra () here. To make it more concise you may want to use 

let start = item[calGetStartDateProp(item)] || item[calGetEndDateProp(item)] 
and 
let end = item[calGetEndDateProp(item)] || item[calGetStartDateProp(item)];

Please check if that works before doing so.

@@ +1332,5 @@
>                if (i == 0) {
>                    // first shadow
>                    column.fgboxes.dragspacer.setAttribute(aSizeattr, aStart * column.mPixPerMin);
> +                  // for events with the same Start End date, give a minimum size of 2 pixels
> +                  column.fgboxes.dragbox.setAttribute(aSizeattr, (firstShadowSize * column.mPixPerMin) || 2);

Isn't there a min-width/min-height rule that takes care of this? I belìeve what is set here is only the height/width, which is less important than the min-height/min-width on calculation.

::: calendar/locales/en-US/chrome/calendar/calendar.properties
@@ +592,5 @@
>  #    %1$S will be replaced with the date of the due date
>  #    %2$S will be replaced with the time of the due date
>  datetimeIntervalTaskWithoutStartDate=due date %1$S %2$S
>  
> +dragLabelTasksWithOnlyEntryDate=Start time

I think in this case its "Starting time"

@@ +593,5 @@
>  #    %2$S will be replaced with the time of the due date
>  datetimeIntervalTaskWithoutStartDate=due date %1$S %2$S
>  
> +dragLabelTasksWithOnlyEntryDate=Start time
> +dragLabelTasksWithOnlyDueDate=Due time

Hmm, it would be nice to consult a native speaker here, but I believe its not common to say "Due time". More common is "Due at", but that may not fit into the context.
Attachment #501676 - Flags: feedback?(philipp) → feedback+
Assignee: nobody → bv1578
Status: NEW → ASSIGNED
(In reply to Philipp Kewisch [:Fallen] from comment #43)

> >                                 class="calendar-item-image"
> >                                 xbl:inherits="progress,allday,itemType"/>
> > +                    <xul:image anonid="startEnd-icon"
> > +                               class="calendar-item-image2"
> > +                               xbl:inherits="type"/>
> 
> Will there be a situation where both the item icon and the startEnd icon are
> displayed? If not, then I'd rather just use one xul:image here.
>

With these new icons there are six possible situations for todo events:

1 todo
2 todo-completed
3 todo, start
4 todo, end
5 todo-completed, start
6 todo-completed, end

cases with start and end need two icons, hence two images in the box.

It would be possible to use only one image in the event box by using a "day-box-item-image.png" with redundant icons as shown in the screenshot. What's your opinion? 

  
> > +dragLabelTasksWithOnlyEntryDate=Start time
> 
> I think in this case its "Starting time"
> 
  
> > +dragLabelTasksWithOnlyEntryDate=Start time
> > +dragLabelTasksWithOnlyDueDate=Due time
> 
> Hmm, it would be nice to consult a native speaker here, but I believe its
> not common to say "Due time". More common is "Due at", but that may not fit
> into the context.

I think you are right, I simply changed "date" with "time" in the labels "start date" and "due date" that are present in the interface.
Posted patch patch - v1 (obsolete) — Splinter Review
(In reply to Philipp Kewisch [:Fallen] from comment #43)

> Will there be a situation where both the item icon and the startEnd icon are
> displayed? If not, then I'd rather just use one xul:image here.

In this patch I've adopted the solution mentioned in comment 44. Only _one_ image is displayed in the item-box but it includes one or two icons according to the case. The new "day-box-item-image.png" file has redundant icons as showed in the previous screenshot.
Alternatively we could think to different icons for the four new cases.


> At least logically, no need for extra () here. To make it more concise you
> may want to use 
> 
> let start = item[calGetStartDateProp(item)] || item[calGetEndDateProp(item)] 
> and 
> let end = item[calGetEndDateProp(item)] || item[calGetStartDateProp(item)];

If it's not a problem I would prefer to use the other notation. It seems to me simpler (and maybe faster?).


> Isn't there a min-width/min-height rule that takes care of this? I belìeve
> what is set here is only the height/width, which is less important than the
> min-height/min-width on calculation.

I've added the min-width/min-height rule in the "fgdragbox" class (calendar-views.css file) because the events with null duration need a such rule too, otherwise when dragging a such event, only the labels with the time are showed.


> I think in this case its "Starting time"
> 
> Hmm, it would be nice to consult a native speaker here, but I believe its
> not common to say "Due time". More common is "Due at", but that may not fit
> into the context.

Changed both as you suggested while waiting for a native speaker's opinion.


---------------------------------------

While I was testing, a new problem has arisen, caused by the fix for bug 617277, and I'm not completely sure if the fix I've added in this patch doesn't go to break something else in the tasks' filter.
The problem involved tasks with only due date and a _completed_ date set outside the date range of the view inside which the task is showed. E.g.:

- in week-view create a task with only the due date (a day in the week showed
  in the view), save and close.
   -> With the previous patch the task was correctly showed;
- reopen the task and set the status to completed and select a completed date
  in the previous week.
   -> With the previous patch the task disappeared from the week view, but it
      was correctly showed (in the right position) if you switched to
      month/multiweek view (because the new date range included the _completed_
      date).
   
With the fix for bug 617277, tasks without the entry date, are showed only if the completed date (if it exists) is in the date range of the view.
In order to correct this problem, in this patch, I added the changes in calUtils.js and calStorageCalendar.js files (the same files in the fix for bug 617277) and both might change the tasks' filter behavior, that I see is a bit delicate. For this reason I also ask for Matthew Mecca's feedback about this issue because he has done a lot of work on tasks filter.
Attachment #501676 - Attachment is obsolete: true
Attachment #707303 - Flags: review?(philipp)
Attachment #707303 - Flags: feedback?(matthew.mecca)
> Changed both as you suggested while waiting for a native speaker's opinion.

"Starting time" and "Due at" are both good in U.S. English.
Comment on attachment 707303 [details] [diff] [review]
patch - v1

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

As far as I can see these changes won't affect any of the task filters. There is an unintended side effect though - all old completed tasks with a due date but without a start date will always be returned from ranged getItems calls. These are ignored by the views, but for performance I think it would be better to add another constraint on the due date if it exists without a start date.

::: calendar/base/src/calUtils.js
@@ +1109,5 @@
>              // 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.
>              let completedDate = item.completedDate;
> +            if (completedDate && !dueDate) {

I'd suggest something like:

            if (completedDate) {
                let queryStart = ensureDateTime(rangeStart);
                let queryEnd = ensureDateTime(rangeEnd);
                if (dueDate) {
                    dueDate = ensureDateTime(dueDate);
                    return (!queryStart || dueDate.compare(queryStart) >= 0) &&
                           (!queryEnd || dueDate.compare(queryEnd) < 0);
                }
                completedDate = ensureDateTime(completedDate);
                return (!queryStart || completedDate.compare(queryStart) > 0);
            }

::: calendar/providers/storage/calStorageCalendar.js
@@ +1178,5 @@
>                  " ((todo_entry IS NULL) AND " +
> +                "  ((todo_due IS NOT NULL) OR " +
> +                "   (todo_completed IS NULL) OR " +
> +                "   (("+floatingCompleted+" > :range_start + :start_offset) OR " +
> +                "    ("+nonFloatingCompleted+" > :range_start))))) " +

and here I'd suggest instead changing the first part of the where clause as in:

                "SELECT * FROM cal_todos " +
                "WHERE " +
                "(((("+floatingTodoDue+" > :range_start + :start_offset) OR " +
                "   ("+nonFloatingTodoDue+" > :range_start)) AND " +
                "  ((todo_entry IS NULL) OR " +
                "   (("+floatingTodoEntry+" < :range_end + :end_offset) OR " +
                "    ("+nonFloatingTodoEntry+" < :range_end)))) OR " +


I only tested these briefly, so please test if you use this.
Attachment #707303 - Flags: feedback?(matthew.mecca) → feedback+
Comment on attachment 707303 [details] [diff] [review]
patch - v1

(In reply to Decathlon from comment #45)
> If it's not a problem I would prefer to use the other notation. It seems to
> me simpler (and maybe faster?).
Ok, no problem

> Changed both as you suggested while waiting for a native speaker's opinion.
If this remains the only blocker, I'd suggest to just go with what we have now and wait for someone to complain :)

r=philipp for these changes. I'd suggest to incorporate Matthew's suggestions and if needed ask him for review.
Attachment #707303 - Flags: review?(philipp) → review+
(In reply to Philipp Kewisch [:Fallen] from comment #48)
> > Changed both as you suggested while waiting for a native speaker's opinion.
> If this remains the only blocker, I'd suggest to just go with what we have
> now and wait for someone to complain :)
Sorry, I missed comment 46. Going by that of course.
(In reply to Matthew Mecca [:mmecca] from comment #47)

> I'd suggest something like:
> 
>             if (completedDate) {
>                 let queryStart = ensureDateTime(rangeStart);
>                 let queryEnd = ensureDateTime(rangeEnd);
>                 if (dueDate) {
>                     dueDate = ensureDateTime(dueDate);
>                     return (!queryStart || dueDate.compare(queryStart) >= 0)
> &&
>                            (!queryEnd || dueDate.compare(queryEnd) < 0);
>                 }
>                 completedDate = ensureDateTime(completedDate);
>                 return (!queryStart || completedDate.compare(queryStart) >
> 0);
>             }
 
> ::: calendar/providers/storage/calStorageCalendar.js
> and here I'd suggest instead changing the first part of the where clause as
> in:
> 
>                 "SELECT * FROM cal_todos " +
>                 "WHERE " +
>                 "(((("+floatingTodoDue+" > :range_start + :start_offset) OR
> " +
>                 "   ("+nonFloatingTodoDue+" > :range_start)) AND " +
>                 "  ((todo_entry IS NULL) OR " +
>                 "   (("+floatingTodoEntry+" < :range_end + :end_offset) OR "
> +
>                 "    ("+nonFloatingTodoEntry+" < :range_end)))) OR " +
> 
> 
> I only tested these briefly, so please test if you use this.


I've tested your changes and I've had to add the condition "entry IS NULL" also on the second OR condition of the WHERE clause in order to include tasks with due date on the lower limit of the range (due date = range start). So, at the end, the complete WHERE clause should be:

 (due > rangeStart  AND  (entry IS NULL  OR  entry < rangeEnd)) OR
 (due = rangeStart  AND  (entry IS NULL  OR  entry = rangeStart)) OR
 (due IS NULL  AND  (entry >= rangeStart  AND  entry < rangeEnd)) OR
 (entry IS NULL  AND  (completed > rangeStart  OR  completed IS NULL))

please, correct me if I'm wrong.

Instead, the change you've proposed for the file calUtils.js causes a different behavior in the task filter that you can see in task view:
 
 - given a task with e.g.:  no start date
                            due date 24/2/2013
                            completed date 8/1/2013 (d/m/y)
 - selected day in the minimonth: an earlier day than the due date (e.g. today 21/2)
  
   task filter         without patch       with patch + your correction
  ---------------------------------------------------------------------
   ALL                 task visible        task visible        
   COMPLETED           task visible        *task disapears*
   
The "Completed Tasks" filter option calls the checkIfInRange function with rangeEnd equal to midnight of the selected day in the minimonth i.e. 00:00 of the next day (when the selected day is later than today), hence the code returns FALSE with a task like that one mentioned above (completed but with due date later than the selected day in the minimonth). It would return a TRUE if the selected day was later than the due date.

This would change the behavior of the filter, and I'm not sure if this new one is good. The list of completed tasks would depend on the selected day in the minimonth without an explicit indication (the present behavior doesn't depend on the selected day).
What's your opinion?
(In reply to Decathlon from comment #50)
>  (due > rangeStart  AND  (entry IS NULL  OR  entry < rangeEnd)) OR
>  (due = rangeStart  AND  (entry IS NULL  OR  entry = rangeStart)) OR
>  (due IS NULL  AND  (entry >= rangeStart  AND  entry < rangeEnd)) OR
>  (entry IS NULL  AND  (completed > rangeStart  OR  completed IS NULL))
> 
> please, correct me if I'm wrong.

Look right.


> The "Completed Tasks" filter option calls the checkIfInRange function with
> rangeEnd equal to midnight of the selected day in the minimonth i.e. 00:00
> of the next day (when the selected day is later than today), hence the code
> returns FALSE with a task like that one mentioned above (completed but with
> due date later than the selected day in the minimonth). It would return a
> TRUE if the selected day was later than the due date.
> 
> This would change the behavior of the filter, and I'm not sure if this new
> one is good. The list of completed tasks would depend on the selected day in
> the minimonth without an explicit indication (the present behavior doesn't
> depend on the selected day).
> What's your opinion?

Nice catch, we don't want to constrain dueDate by queryEnd to keep in line with the fix for Bug 617277.

Instead we could probably consolidate that whole if (completedDate) { ... } block with something like:

 let queryStart = ensureDateTime(rangeStart);
 dueDate = ensureDateTime(dueDate);
 let completedDate = ensureDateTime(item.completedDate);
 return !completedDate || !queryStart ||
        completedDate.compare(queryStart) > 0 ||
        (dueDate && dueDate.compare(queryStart) >= 0);


The queryStart definition is the same as the one later in the function block so that could probably be consolidated into a single earlier definition also.
Posted patch patch - v2Splinter Review
I've tested the new changes you proposed and it seems there is no difference in the tasks filter behavior compared to the existing one (the filter itself has some aspect that sometimes might leave the user a bit puzzled, but I've already seen there are bugs that take care of that).

Matthew, I ask you for review for the part related to the tasks filter if you want to take a look (and maybe give it a try) before pushing the patch.
Attachment #707303 - Attachment is obsolete: true
Attachment #725644 - Flags: review?(matthew.mecca)
Comment on attachment 725644 [details] [diff] [review]
patch - v2

Looks good. r=mmecca
Attachment #725644 - Flags: review?(matthew.mecca) → review+
Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/dd80dc338a4c

FIXED.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.4
You need to log in before you can comment on or make changes to this bug.