There are several things which cause calendar_ical to fail validation at http://severinghaus.org/projects/icv/ As a result of this several clients refuse/fail to parse the iCalendar file.
The attached patch fixes the following issues:
* Line endings must be \r\n (as per s4.1, folding still needs work)
* UID is more likely to be globally unique (as per s4.8.4.7)
* UID is more in keeping with the spirit of RFC822
* UID doesn't change between renderings
* DTSTAMP is now UTC, relies on fixes in date contained in #760284: template_preprocess_date_vcalendar needs to be changed to help calendar_ical be more RFC 2445 compliant (as per s4.8.7.2)
If this patch is applied #668336: iCal feeds do not pass ical validation and #757498: iCal events export incorrectly, switch to use UTC (Z) timezone can probably be closed.
Comment | File | Size | Author |
---|---|---|---|
#30 | calendar-760316-rfc-fixes-30.diff | 4.49 KB | arithmetric |
#26 | calendar-760316-rfc-fixes-26.diff | 3.86 KB | skwashd |
#16 | calpopup-empty.png | 12.81 KB | domesticat |
#9 | calendar - Copy.ics_.txt | 3.46 KB | Macronomicus |
#7 | calendar - Copy.ics_.txt | 3.44 KB | Macronomicus |
Comments
Comment #1
domesticat CreditAttribution: domesticat commentedFYI: I followed your instructions in #760284: template_preprocess_date_vcalendar needs to be changed to help calendar_ical be more RFC 2445 compliant that said to apply that patch before this one. The patches are partial dupes - both contain the same modifications to the Calendar module. Anyone trying to apply the patch in this thread after the one in #760284: template_preprocess_date_vcalendar needs to be changed to help calendar_ical be more RFC 2445 compliant will see errors. You'll probably want to separate the patches properly: one for code in the Date module, and one for Calendar.
I manually applied the patches, and determined they aren't getting my iCal feed completely compliant, but it's getting closer. I ran my .ics file through http://severinghaus.org/projects/icv/ and got the following error:
Fewer errors than before, though! Marking 'needs work' to get the duplication resolved.
Comment #2
skwashd CreditAttribution: skwashd commentedTracked down the problem with the patches. I posted the wrong patch on #760284: template_preprocess_date_vcalendar needs to be changed to help calendar_ical be more RFC 2445 compliant. I've fixed that.
The validation error you are seeing is triggered by the recurring rule handling. I don't use recurring events, so I didn't look at it when creating the original patch. A quick look at the date and calendar code suggests that the problem is in calendar_ical. The recurring rules are generated properly in the date_repeat module, but calendar_ical is adding RRULE; at the start of the line which is breaking things. Try changing lines 41-43 in calendar/calendar_ical/calendar-view-ical.tpl.php to read:
If that passes the validator I'll reroll the patch with that fix included.
Putting back to needs review as the patch does what it was originally intended to do.
Comment #3
domesticat CreditAttribution: domesticat commentedAha! MUCH better. I tried the patches together, including the suggestion for RRULE change, and my feed finally validates. Google Calendar still isn't displaying the events, which I need to check on, but this is a promising start.
Comment #4
domesticat CreditAttribution: domesticat commentedGrr. On the other hand, http://icalvalid.cloudapp.net vomits profusely. Lots of 'The time zone with the TZID 'America/Chicago' was not found.'
Comment #5
skwashd CreditAttribution: skwashd commentedI read more of RFC 5545 and paid attention to s3.2.19. What exciting reading RFCs are.
Based on my reading of that, we need to define VTIMEZONEs for each unique TZID in the generated iCalendar file. That will involve some work and some code refactoring, so I propose that we fork that off to another issue and deal with it there.
I will try to reroll this with the RRULE fix included over the weekend.
As for google calendar not displaying the entries, try deleting and readding the feed.
Comment #6
skwashd CreditAttribution: skwashd commentedI read the RFC in more detail this morning when I wasn't so tired. I also read RFC 2445. What a great way to start the weekend.
The cloudapp.net validator still isn't happy. I noticed in both RFCs the TZID is presented as [continent]-[location] as well as the more common [continent]/[location] format. Changing the / to - did nothing - except slow things down. At this point I am inclined to claim the cloudapp validator is buggy when it comes to TZID validation.
The DTSTART and DTEND values were incorrectly formatted, I've fixed that. I also added the fix for recurring events I proposed at #2. Attached is a rerolled patch and will do the same for one attached to #760284: template_preprocess_date_vcalendar needs to be changed to help calendar_ical be more RFC 2445 compliant.
I've tested this using Mozilla Lightning, Google Calendar and Novell Evolution and all work. The servinghaus.org validator is happy too.
If there are any other bugs, please attach a iCalendar file so I can check the output against the code.
For now I think we are good to go once someone checks the rerolled patch.
Comment #7
Macronomicus CreditAttribution: Macronomicus commentedI applied your patch since I kept getting errors when importing the ics into google calendar ...still its not working and im stumped.
Took me a bit to fix my ics file because it was empty; now the data is in there but google no likey, man I was really hoping your patch would fix it... perhaps there is something wrong in my view though. Ive spend a bunch of time getting beautytips to work with my calendar and this little ics bugger is killing me! lol
Ive attached the ics and the view, what have I gone and screwed up? ...or is this a bug?
I have gone through all the TROUBLESHOOTING bits KarenS outlined... any help is greatly appreciated.
When importing the ics Google responds ..
Im running...
Comment #8
Macronomicus CreditAttribution: Macronomicus commentedI ran it through that validator you posted in the first post and got this
EDIT: Woops ... forgot to apply http://drupal.org/node/760284#comment-2828614
Comment #9
Macronomicus CreditAttribution: Macronomicus commentedOk well its working now .. I just have to get all that html markup out of it .. the validator is throwing "Illegal property" errors and google is displaying the markup instead of just the text.
Comment #10
Macronomicus CreditAttribution: Macronomicus commentedwoops I was using custom markup to make beautytips work in my other displays .. forgot to remove the [markup] when I did an override of the fields in the ical display... then just turned on strip html for the body and location fields.
All is well now .. I hope this gets committed since google wont work without it ... at least not for me.
Comment #12
skwashd CreditAttribution: skwashd commented@macrocosm could you please post a comment on #760284: template_preprocess_date_vcalendar needs to be changed to help calendar_ical be more RFC 2445 compliant too please so the 2 can be committed in a coordinated way. Sorry for the hassle, but I had to split the patches up as the problems exist in 2 different modules.
Comment #13
Macronomicus CreditAttribution: Macronomicus commentedwill do .. I did notice one thing though ... seems thunderbird lightning doesnt like the ical file but thats better for another issue. This combo is the only thing that made google happy, thanks a ton!
Cheers!
Comment #14
skwashd CreditAttribution: skwashd commentedI tested here with thunderbird 3 and lightning on ubuntu 10.04 and it seemed to work ok. I did keep on deleting it and readding it until I got a working version during development. If it doesn't work after deleting it can you post (or email me) a url for checking?
Comment #15
Macronomicus CreditAttribution: Macronomicus commentedWhen importing the ical I get the following
Im running...
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1.9) Gecko/20100317 Lightning/1.0b1 Thunderbird/3.0.4
However .. as I was writing this I tested something else .. I created another calendar inside lightning because I remembered that my main calendar in there was one connected to gmail. That was the prob because as soon as I used a diff cal it worked as expected! yay!
Im feeling the Drupal calendar love!
Comment #16
domesticat CreditAttribution: domesticat commented@skwashd - I'm seeing something strange that only appears when this patch is applied. The calendar popup is empty. I'm attaching a screenshot to give you an idea of what it looks like. The problem goes away as soon as I reverse the patch and clear the cache.
Caveat in the other direction: we do have some custom theming going on in node-event.tpl.php.
Comment #17
skwashd CreditAttribution: skwashd commented@domesticat can you please post the node-event.tpl.php and node type export.
Comment #18
penguin25 CreditAttribution: penguin25 commentedIs there a reason why the patch in comment 6 changes some, but not all, line endings in the feed to be CRLF (\r\n) instead of LF (\n)? Reading the spec says that CRLF is the correct line ending, but I can't see why this hasn't been changed everywhere.
After applying the patch, Apple's iCal app stopped displaying the content of my feeds. This was traced this to the mixed line endings - iCal will only display a feed if every line ends in CRLF, or if every line ends in LF. When I subsequently changed calendar-view-ical.tpl.php to print \r\n at the end of every line, the feed displayed in iCal again.
Comment #19
skwashd CreditAttribution: skwashd commented@penguin25 which lines don't have CRLF line endings?
Comment #20
penguin25 CreditAttribution: penguin25 commentedEvery line from the initial BEGIN:VCALENDAR up to and including the PRODID: line, and then all BEGIN:VEVENT and END:VEVENT lines, and finally the END:VCALENDAR line.
Basically, all the lines that the patch doesn't change.
Comment #21
skwashd CreditAttribution: skwashd commented@penguin25 It passed the validator. i will look at rerolling the patch in the coming days.
Comment #22
SyneX CreditAttribution: SyneX commentedThank you for the patch. Now it's working for me in Google calendar.
Comment #23
raintonr CreditAttribution: raintonr commented+1 Subscribing
Comment #24
KarenS CreditAttribution: KarenS commentedSo the linked issue on the Date issue queue says the patch is ready to be committed and this one says patch needs work, but the note says both need to go in together. If this one is not ready, that one is not ready.
I tried and failed to figure out what the patch actually is or whether it is working. #22 says 'it is working', but *what* is working? This issue is not clear at all.
Comment #25
schultetwin CreditAttribution: schultetwin commentedBy "it works" I assume Synex means the google calendar now correctly imports the ical feed. Before these two patches are applied, at least for me, the ical feed would not import all events into google calendar (usually stopping at the latest event created, but I didn't look into it that much.) As it turns out, the ical feed did not validate at http://severinghaus.org/projects/icv/. After both patches are applied, the feed validates at the previous website, and imports into google calendar without problem.
I can also confirm that the ical feed still loads correctly in evolution and sunbird. Also, it does brake the Calendar Popup module. Lastly, I believe this brakes the import to iCal. I'm not positive, because I tested it on an iPod Touch, but the import didn't work there.
Comment #26
skwashd CreditAttribution: skwashd commented@KarenS What @schultetwin said. There were a bunch of problems with the iCal feed from the calendar module. This patch fixes them.
Here is a list of issues which this and the Date patch ( #760284 ):
* Feed now validates using http://severinghaus.org/projects/icv/
* Feed works with Mozilla Lightning, Novell Evolution
* Line endings are now \r\n (as per s4.1, folding still needs work)
* UID is more likely to be globally unique (as per s4.8.4.7)
* UID is more in keeping with the spirit of RFC822
* UID doesn't change between renderings - stops clients getting confused'
* DTSTAMP is now UTC which seems the simplest solution
* Strings no longer contain escaped html entities
I rolled all this into a single patch as it is all related to the module being RFC compliant.
This version of the patch also does the following:
* The jcalendar popup regression reported by @schultetwin is now fixed, I'd never used the module before
* Line ending problems as reported by @penguin25 have been fixed. I don't own a mac/i* to test this.
* Date patch has been rerolled to ensure it is current
Let me know if you need more information.
Comment #27
schultetwin CreditAttribution: schultetwin commented@skwashd Thanks a lot. I can confirm that, at least on the ipod touch, the ical feed now imports correctly. Thanks.
Comment #28
skwashd CreditAttribution: skwashd commented@schultetwin Thanks for the feedback. Is it worthy of a RTBC status? :)
Comment #29
schultetwin CreditAttribution: schultetwin commentedNot sure. Two people isn't much of a test base. However, I will say that the ical feed now works flawlessly for me. It's great. Thanks again for the patch.
Comment #30
arithmetric CreditAttribution: arithmetric commentedI've been testing this patch (and the related one for the Date module) to fix a problem with Outlook clients listing events an hour off. These patches did not solve the problem until I added the changes suggested in #757498: iCal events export incorrectly, switch to use UTC (Z) timezone for the Calendar module.
I'm attaching an updated version of the patch from this issue that integrates the changes from the other issue, including changing DTSTART and DTEND to using a UTC date value (like DTSTAMP). Also, I added a \r\n after the PRODID line to fix a validation error detected by http://icalvalid.cloudapp.net
Note: This patch show many lines from calendar-view-ical.tpl.php as changed because I ensured that the non-PHP lines uses \r\n for breaks.
Comment #31
IcyAndy CreditAttribution: IcyAndy commented- Patch applies to latest CSV code as per 14.10.2010 (Version 6.x-2)
- Also applied patch #760284: template_preprocess_date_vcalendar needs to be changed to help calendar_ical be more RFC 2445 compliant
- Checked line ending changes (CR LF) with RFC 5545. Care must be taken when editing .tpl files in future to preserve CR LF endings
- Checked UID change with RFC 5585, 3.8.4.7
- Patch does not deal with line folding as per RFC 5545, 3.1. As this is a "SHOULD" requirement it is not strictly necessary.
Tested with Thunderbird (lightning extension) and http://severinghaus.org/projects/icv/ but not with http://icalvalid.cloudapp.net as the validator is not reachable most of the time (at least for me, though other cloudapp.net sites work).
Recurring events have been tested only with one test case but this patch does not change handling of recurring events and therefore should be fine.
Possible caveat: Format of UID of events changed. When upgrading this might break existing ical feed imports (external site or application importing calendar ical feed) which rely on this. Recommended to delete imported events first (completely remove imported feed) and then import the ical feed from scratch.
Patch #30 includes #757498: iCal events export incorrectly, switch to use UTC (Z) timezone which can be marked duplicate when this patch is applied.
Comment #32
keopxThe popup no support multi-language. The popup link to the node id but this url path change to default language and no support navigation language.
View this post to solve: http://drupal.org/node/427388
After applying the patch I've seen the error and I think the error was in the patch
Comment #33
izmeez CreditAttribution: izmeez commentedsubscribe
Comment #34
ekes CreditAttribution: ekes commentedUsed patch #30 as with #31 along with #760284: template_preprocess_date_vcalendar needs to be changed to help calendar_ical be more RFC 2445 compliant #15.
Validating on http://icalvalid.cloudapp.net as well. Patch makes sense. Agree #31 RTBC.
Comment #35
KarenS CreditAttribution: KarenS commentedI'm trying to figure out the reason behind this part in the patch:
The use of ':' as a splitter for the parts of the date is required for other things in the date module to work, so this change will break other things. Without any explanation for why it was changed, I can't address a way to fix this without breaking other things.
I'm going to apply the rest of this patch, leaving out the part that alters the splitter and let people sort out what still needs to be changed and why.
Comment #36
KarenS CreditAttribution: KarenS commentedWe also can't do the following:
That only works if you're using PHP 5.2 or higher and not everyone is. We have to use the wrapper function, date_set_timezone(), for compatibility with the older versions.
Comment #37
ekes CreditAttribution: ekes commented#35 I think is just following RECOMMENDED sections of the RFC. The UID can include ':' as it's a valid part of TEXT; but it could be problematic because parsers sometimes make invalid assumptions about the : separator similar (but not the same) as for example http://drupal.org/node/282521#comment-2950950 ;-)
Comment #38
KarenS CreditAttribution: KarenS commentedCommitted this much. If there are still problems please start a new issue with that information. This thread is getting pretty long and confusing.