328197 - Working with Todo- and Agenda-tabpage and recurring Task kills whole calendar

Status: VERIFIED FIXED

(Calendar :: Lightning Only, defect)

Description

[Andreas Treumann]

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

see 'Steps to Reproduce'


Reproducible: Always

Steps to Reproduce:
1. create a task with a recurrency, e.g 'Repeat for 5 occurences'
2. check the contol box of this task at the 'ToDo'-tabpage
3. go to the 'Agenda'-tabpage and change the view to 'Tasks'
4. go back to the 'ToDo'-tabpage und uncheck the box
5. now go back to the 'Agenda'-tabpage, try to change the view -> nothing happens
6. refresh the calendar view -> all events and task are gone.

After a application restart the 'Calendars'-tabpage is empty, too.

Actual Results:  
Steps above kills all Calendars (dataloss)


Mayby this bug should be added to Bug 326352 (Lightning 0.1 release notes)

Comment 1

[Andreas Treumann]

I checked this at win2k and Linux (Suse 10.0)

Comment 2

[Stefan Sitter [:ssitter]]

Unfortunately confirmed on Windows XP with branch builds. JavaScript console is full of errors (some of them multiple times):

----------------------------------------------------------------------
Error: [Exception... "Component returned failure code: 0x80004003 (NS_ERROR_INVALID_POINTER) [calIRecurrenceItem.getOccurrences]"  nsresult: "0x80004003 (NS_ERROR_INVALID_POINTER)"  location: "JS frame :: file:///<profile>/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/components/calRecurrenceInfo.js :: anonymous :: line 388"  data: no]
----------------------------------------------------------------------
Error: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [mozIStorageStatementWrapper.row]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: file:///<profile>/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/components/calStorageCalendar.js :: anonymous :: line 748"  data: no]
----------------------------------------------------------------------
Error: flushItem DB error: database table is locked
Source File: file:///<profile>/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/components/calStorageCalendar.js
Line: 1475
----------------------------------------------------------------------
Error: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [mozIStorageConnection.rollbackTransaction]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: file:///<profile>/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/components/calStorageCalendar.js :: anonymous :: line 1476"  data: no]
----------------------------------------------------------------------
Error: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [mozIStorageStatementWrapper.step]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: file:///<profile>/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/components/calStorageCalendar.js :: anonymous :: line 1270"  data: no]
----------------------------------------------------------------------

Comment 3

[Stefan Sitter [:ssitter]]

Order of events/errors:

After closing the New Task dialog for reccuring task:
--> (NS_ERROR_INVALID_POINTER) [calIRecurrenceItem.getOccurrences]

After checking the check box for this task in Todo tab:
--> (NS_ERROR_INVALID_POINTER) [calIRecurrenceItem.getOccurrences]

After selecting 'Tasks' filter in agenda tab:
--> (NS_ERROR_FAILURE) [mozIStorageStatementWrapper.row]

After unchecking the check box for this task in Todo tab:
--> flushItem DB error: database table is locked
--> (NS_ERROR_FAILURE) [mozIStorageConnection.rollbackTransaction]

Comment 4

[Daniel Boelzle [:dbo]]

Attachment: typo in storage provider, readonly attribute

patch fixes:
- missing readonly tag for calIItemBase::recurrenceStartDate
- typo in storage provider

IMO the following is still open:
- the todo dialog has to require a "Date" to allow a "Repeat"
- the agenda view shows only tasks with due date, is this intended/sensible?
- I see sometimes doubled entries

I have no time left anymore this evening...

Comment 5

[Daniel Boelzle [:dbo]]

Attachment: typo in storage provider, readonly attribute (checked in)

patch fixes:
- missing readonly tag for calIItemBase::recurrenceStartDate
- typo in storage provider

IMO the following is still open:
- the todo dialog has to require a "Date" to allow a "Repeat"
- the agenda view shows only tasks with due date, is this intended/sensible?
- I see sometimes doubled entries

I have no time left anymore this evening...

Comment 6

[Michael Büttner (no reviews TFN)]

Attachment: patch disables recurring todos (checked in)

as discussed on IRC disabling recurring todo's for the 0.1 lightning release is a reasonable way to work around this issue.

Comment 7

[Dan Mosedale (:dmosedale, :dmose)]

Both patches have been checked in; thanks guys!  Since weve disabled recurrence UI for todos for the moment, this no longer blocks 0.1.  Marking for release-noting.

Comment 8

[Michael Büttner (no reviews TFN)]

reassigned.

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

When we turn recurring tasks back on, it's worth checking that they don't still have bug 335643 when using the ICS provider.  A quick glance at calRecurrenceInfo.js suggests that it uses the same logic for events and tasks, which I suspect may cause trouble.

Comment 10

[Michael Büttner (no reviews TFN)]

Attachment: turn on recurring tasks v1

this patch turns recurring tasks back on. in addition to the previously missing support in the dialog it also contains the necessary modifications to the backend stuff (calRecurrenceInfo::onStartDateChange and calRecurrenceInfo::calculateDates).

Comment 11

[Joey Minta]

Comment on attachment 225247 [details] [diff] [review]
turn on recurring tasks v1

This patch ends up disabling the 'Set Pattern' button for new events, which is wrong.

Also, all of the other 'invalid values' cases in the dialog get warnings posted, rather than blocking the user from an action (such as removing the entry date).  I think, for consistency, we should continue with that idea.

+                                         
+                                         // bail out if the item doesn't specify an end date.
+                                         if (!dtend) {
+                                            return;
+                                         }
+ 
Don't we want to check for DUE, since tasks will never have a DTEND?  Perhaps this is why, with this patch, a repeating daily task still only shows up on the first day in the month view.

Comment 12

[Joey Minta]

Comment on attachment 225247 [details] [diff] [review]
turn on recurring tasks v1

See previous comment

Comment 13

[Michael Büttner (no reviews TFN)]

Attachment: turn on recurring tasks v2

> This patch ends up disabling the 'Set Pattern' button for new events, which
> is wrong.
ups, sorry for that.

> Also, all of the other 'invalid values' cases in the dialog get warnings
> posted, rather than blocking the user from an action (such as removing the
> entry date).  I think, for consistency, we should continue with that idea.
I don't like the warnings at all, but instead of spinning off another ui related discussion I just added the appropriate warning text.

> Don't we want to check for DUE, since tasks will never have a DTEND?  Perhaps
> this is why, with this patch, a repeating daily task still only shows up on
> the first day in the month view.
You're right that we should check for DUE here, but it has nothing to do with the issue you mentioned. getOccurrenceFor() had a similar problem which has been addressed in this new iteration as well.

Comment 14

[Joey Minta]

Comment on attachment 225557 [details] [diff] [review]
turn on recurring tasks v2

+    if(!enableAccept) {
nit: space before (

I don't get the warning if I check the Repeat checkbox right away after opening the dialog.  I think you need an updateAccept() call for the checkbox too.

+    get duration() {
+        return this.entryDate.subtractDate(this.dueDate);
+    },
+
This is going to throw if either entryDate or dueDate don't exist.  I tend to feel like it should just return null in that case, but either way we need to make sure that behavior is documented in the IDL.

r=jminta with those

Comment 15

[Michael Büttner (no reviews TFN)]

patch checked in.

Comment 16

[Joey Minta]

Stupid, stupid bugzilla.  I had another review comment here that somehow got lost.  If I check 'Repeats' right away after opening the dialog, I don't get the warning message.  I think we need an updateAccept() in the checkbox's oncommand.

Comment 17

[Stefan Sitter [:ssitter]]

Comment on attachment 225557 [details] [diff] [review]
turn on recurring tasks v2

>+++ mozilla/calendar/base/src/calTodo.js	2006-06-13 17:33:35.890625000 +0200
>+    get duration() {
>+        return this.entryDate.subtractDate(this.dueDate);
>+    },

I think this should be dueDate - entryDate (endDate - startDate in IDL comment) instead. This mix-up would also explain the following behavior I see:

When editing a single occurrence from a repeating task that is scheduled from 12:00 to 15:00 the task dialog displays 12:00 to 09:00.
When editing a single occurrence from a repeating task that is scheduled from 12:00 to 18:00 the task dialog displays 12:00 to 06:00.

Comment 18

[Joey Minta]

Reopening to deal with the 2 previous comments.

Comment 19

[Michael Büttner (no reviews TFN)]

Attachment: add-on patch v1

Joey, your comment regarding the missing updateAccept() call wasn't missing, see your comment #14. Thanks for pointing this out, I added the missing call and checked the patch in, see http://lxr.mozilla.org/seamonkey/source/calendar/base/content/calendar-event-dialog.js#660

Stefan, thanks for the hint, you're totally right.

Comment 20

[Joey Minta]

Comment on attachment 226127 [details] [diff] [review]
add-on patch v1

r=jminta

Comment 21

[Michael Büttner (no reviews TFN)]

patch checked in.

Comment 22

[Andreas Treumann]

Checked again in latest nighly build, task is fixed.