Closed Bug 329737 Opened 14 years ago Closed 14 years ago
Script Error: "trigger Prop has no properties"
Works for me. This test event has been created using Sunbird 0.2. I can import this event without problems into Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060307 Mozilla Sunbird/0.3a1+. Alarm is properly scheduled.
Works for me. This test event has been created using Sunbird 0.3a1. I can import this event without problems into Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060307 Mozilla Sunbird/0.3a1+. Alarm is properly scheduled.
The two previous testcases do not contain the problematic VALARM section. Not all files generated with old calendar builds have this problem but some do. I haven't tried to reproduce the problem using an old build, I have just found the offending parts in some old files generated by several of my colleages. This specific testcase is made in Notepad because I didn't want to upload an authentic 225 KB calendar. But the offending part in the testcase is copied from an authentic calendar file, and the error message is the same when importing the testcase and the authentic file.
Cant parse as DURATION value in TRIGGER property. Removing entire property: -PT-45M That says that the trigger wasn't null. Rather, it was a completely invalid duration, namely one with 2 negative signs in it. Would it be possible to create this in older versions by putting a negative number in for the offset? Also, can you try to say more specifically which old version output this alarm? How far back on the version chain are we talking?
(In reply to comment #4) > Cant parse as DURATION value in TRIGGER property. Removing entire > property: -PT-45M > > That says that the trigger wasn't null. Yes, appearently the old calendar build failed to parse the DURATION property and instead wrote this informative error message to the output file. And when a new calendar build tries to parse a file containing this error message, it barfs about triggerProp being null. > Also, can you try to say more specifically which old version output this > alarm? How far back on the version chain are we talking? Several builds have been in use in the office. May best guess is either Sunbird 0.2 or the calendar plug-in for Thunderbird found in calendar_windows_20050111.xpi. AFAICS the version/build id isn't written to the ICS file.
OK, here's what I've found from more testing. It's possible to create this sort of illegal duration in Sunbird 0.2 by putting in a negative number in the alarm offset slot of the event dialog. However, even 0.2 freaks out with this data, converting it to some seemingly random large number. I'm somewhat inclined to say that there's nothing we can do here, because (1) Hacking libical to parse this duration is dangerous (2) Even Sunbird 0.2 wasn't "compatible" with the data. (3) Trigger is required for alarms (see below). On the other hand, it may be somewhat possible to hack in a fix for just the VALARM component. The problem is that the TRIGGER is the most important part of an alarm. Without it, we have absolutely no idea when the alarm should fire. I'm open to suggestions as to what we ought to substitute if no trigger is defined. My inclination, however, is that this error is an exactly correct response to a completely invalid alarm. (Note that it doesn't contain a DESCRIPTION or an ACTION either!)
> I'm open to suggestions as to what we ought to substitute if no trigger > is defined. I don't know what the general approach is when an ICS file contains an error. Rejecting the whole file because of an error in one entry is suboptimal IMO. I agree that trying to parse the bogus value is overkill. I suggest that the malformed VALARM entry should just be silently ignored.
This patch makes it so that on the off-chance we encounter an alarm that is invalid like this, we report the error to the console, but will keep parsing the rest of the file.
Assignee: nobody → jminta
Status: NEW → ASSIGNED
Attachment #216819 - Flags: first-review?(mvl)
Comment on attachment 216819 [details] [diff] [review] report error, but keep parsing Can you add some information to the errormessage, which tells the user which event failed? (the ID will do, I think) r=mvl with that.
Attachment #216819 - Flags: first-review?(mvl) → first-review+
Patch checked in with additional reporting of item's id.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.