Comment 109 for bug 307680

(From update of attachment 404433)
> function minimonthPick(aNewDate) {
>- if (isSunbird() || gCurrentMode == "calendar") {
>+ if (isSunbird() || gCurrentMode == "calendar" || gCurrentMode == "task") {
Go ahead and use cal.isSunbird() instead, here and elsewhere.

>+ // prevent toggling completed status for parent items of repeating tasks
>+ if (!task || task.recurrenceInfo)
> return;
While you're here, please add {}, even for one-line if's

> onAddItem: function tTO_onAddItem(aItem) {
...
>+ let occs;
>+ if (this.binding.mFilter.endDate) {
>+ occs = aItem.getOccurrencesBetween(this.binding.mFilter.startDate,
>+ this.binding.mFilter.endDate,
>+ {});
Lets be more robust here and also check if the start date isn't null.

>+ }
>+ for each (let occ in occs) {
>+ this.binding.mTreeView.removeItem(occ);
>+ }
If you like:
  occs.forEach(this.binding.mTreeView.removeItem, this.binding.mTreeView);

>
>- if (savedThis.mFilter.startDate && savedThis.mFilter.endDate) {
>+ if (savedThis.mFilter.endDate) {
> filter |= aCalendar.ITEM_FILTER_CLASS_OCCURRENCES;
> }
Same comment as before, this time I have the feeling you did it for a reason :-) Why only check for end date here? Can't we be more robust and also check for the start date?

>+ let oneDay = createDuration();
Please prefix everything you use from calUtils.js with "cal.", we are transitioning to the use of the calUtils.jsm module. If you get an error that it is not defined, add

Components.utils.import("resource://calendar/modules/calUtils.jsm");

in the constructor.

>+ // add listener to update the date filters
>+ getViewDeck().addEventListener("dayselect", updateCalendarToDoUnifinder, false);
>+
> function getDatesForFilter(aFilter) {
>- var EndDate = createDateTime();
>- var StartDate = createDateTime();
>- var Duration = createDuration();
>+ var endDate = createDateTime();
>+ var startDate = createDateTime();
>+ var duration = createDuration();
> var oneDay = createDuration();
Go ahead and change all var's to let's in this function.

>+.calendar-task-tree > treechildren::-moz-tree-cell-text(repeating) {
>+ color: blue;
>+}
The css style rules seem the same between windows and mac. Please put the rules in the respective files in base/themes/common. If there is no file for the task tree, then go ahead and create one, including it via @import (similar to how the other common files are included)

> if (itemReturnOccurrences && item.recurrenceInfo) {
>+ let startDate = aRangeStart;
>+ if (!aRangeStart && isToDo(item))
>+ startDate = item.entryDate;
Please use {} for this if statement. If you like, please also change var to let.

The code looks fine, r- just to get a new patch with nits fixed. Waiting for ui-review now, I'll be testing it in the meanwhile.