When a task is marked completed by clicking the check box image in the task tree, the display does not update until after the modify transaction is processed. This behavior is different from "real" check boxes that usually update immediately when clicked. Updating the check box sooner will also help to notify the user that the task is being updated, especially with a slower transaction. AFAIK there is no way to force the display to update immediately, but if we first update the tree row, then we can postpone the actual modify transaction until after the display has been updated.

Allows the updateItem function to take a callback modifier function as an optional argument. If used, a clone of the item is created, and the modifier function is called on the clone to do the modification. The item is first updated in the tree, and the doTransaction call is postponed using setTimeout to allow the display to update first. A delay of 100ms was the shortest timeout interval that consistently caused the display to update first.

Comment on attachment 607816 [details] [diff] [review] Fix v1 Review of attachment 607816 [details] [diff] [review]: ----------------------------------------------------------------- r- just to resolve the below comments. I haven't actually tested the patch, but I see some potential issues. ::: calendar/base/content/calendar-task-tree.xml @@ +530,5 @@ > + // replace the item in the tree. The modify transaction will be postponed to allow > + // the tree view to update first. > + updateItem: function tTV_updateItem(aItem, aModifier) { > + if (aItem.parentItem.hashId in this.binding.mPendingUpdates) { > + return false; What happens if: 1. User checks task completed on a slow calendar 2. User decides otherwise and unchecks the task again 3. The operation completes on the calendar, so the onModifyItem will check the task? The user will likely be confused now and scream dataloss. @@ +545,5 @@ > + let modifyListener = { > + binding: this.binding, > + onOperationComplete: function(aCalendar, aStatus, aOperationType, aId, aDetail) { > + if (aItem.parentItem.hashId in this.binding.mPendingUpdates) { > + delete this.binding.mPendingUpdates[aItem.parentItem.hashId]; If you do let binding = this.binding; on toplevel of updateItem(), then you don't have to save the binding on the modifyListener, but can just reference the binding var. It will also shorten the line length a bit. @@ +551,5 @@ > + } > + }; > + > + function doModifyTransaction() { > + doTransaction('modify', newItem, newItem.calendar, aItem, modifyListener); let doModifyTransaction = doTransaction.bind(null, "modify", newItem, newItem.calendar, aItem, modifyListener); Consider breaking the line at something around 80 chars. @@ +558,5 @@ > + aModifier(newItem); > + > + if (index > -1 && (newItem.hashId == aItem.hashId)) { > + // update the item in the tree > + this.binding.mTaskArray[index] = newItem; How does this interact with the onModifyItem observer? Lets say: 1. User clicks checkbox on a task on a slow calendar 2. Task updates in the list, user doubleclicks on task 3. The onModifyItem returns and does some changes to the item - for example, some providers might change a sequence number or so 4. User wants to save the task from the dialog 5. The user might get a conflict dialog from the provider, since the sequence number is not updated.

patch needs followup to r-