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

VERIFIED FIXED

Status

--
critical
VERIFIED FIXED
13 years ago
12 years ago

People

(Reporter: andreas.treumann, Assigned: michael.buettner)

Tracking

({dataloss})

Trunk
x86
All
dataloss

Details

Attachments

(2 attachments, 4 obsolete attachments)

(Reporter)

Description

13 years ago
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)
(Reporter)

Comment 1

13 years ago
I checked this at win2k and Linux (Suse 10.0)
OS: Windows 2000 → All
Version: unspecified → Trunk
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]
----------------------------------------------------------------------
Status: UNCONFIRMED → NEW
Ever confirmed: true
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]

Updated

13 years ago
Blocks: 298366
Keywords: dataloss

Comment 4

13 years ago
Created attachment 212787 [details] [diff] [review]
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

13 years ago
Created attachment 212789 [details] [diff] [review]
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...

Updated

13 years ago
Attachment #212787 - Attachment is obsolete: true
(Assignee)

Comment 6

13 years ago
Created attachment 213036 [details] [diff] [review]
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.

Updated

13 years ago
Attachment #213036 - Attachment description: patch disables recurring todos → patch disables recurring todos (checked in)
Attachment #213036 - Attachment is obsolete: true

Updated

13 years ago
Attachment #212789 - Attachment description: typo in storage provider, readonly attribute → typo in storage provider, readonly attribute (checked in)
Attachment #212789 - Attachment is obsolete: true
Attachment #212789 - Flags: first-review+

Comment 7

13 years ago
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.
No longer blocks: 298366
Whiteboard: [cal relnote]
(Assignee)

Comment 8

12 years ago
reassigned.
Assignee: nobody → michael.buettner

Comment 9

12 years ago
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.
(Assignee)

Comment 10

12 years ago
Created attachment 225247 [details] [diff] [review]
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).
Attachment #225247 - Flags: first-review?(jminta)

Comment 11

12 years ago
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

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

See previous comment
Attachment #225247 - Flags: first-review?(jminta) → first-review-
(Assignee)

Comment 13

12 years ago
Created attachment 225557 [details] [diff] [review]
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.
Attachment #225247 - Attachment is obsolete: true
Attachment #225557 - Flags: first-review?(jminta)

Comment 14

12 years ago
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
Attachment #225557 - Flags: first-review?(jminta) → first-review+
(Assignee)

Comment 15

12 years ago
patch checked in.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED

Comment 16

12 years ago
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 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

12 years ago
Reopening to deal with the 2 previous comments.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 19

12 years ago
Created attachment 226127 [details] [diff] [review]
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.
Attachment #226127 - Flags: first-review?(jminta)

Comment 20

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

r=jminta
Attachment #226127 - Flags: first-review?(jminta) → first-review+
(Assignee)

Comment 21

12 years ago
patch checked in.
Status: REOPENED → RESOLVED
Last Resolved: 12 years ago12 years ago
Resolution: --- → FIXED
Whiteboard: [cal relnote]
(Reporter)

Comment 22

12 years ago
Checked again in latest nighly build, task is fixed.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.