Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix VTIMEZONE DTSTART format and TZOFFSETFROM calculation #58

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

icvalyssa
Copy link

@icvalyssa icvalyssa commented Oct 9, 2020

Fix for Issue #53

  • Remove inline TZID from VTIMEZONE -> DTSTART (regardless of dateTimeFormat) for compatibility with Google and adherence to https://tools.ietf.org/html/rfc5545#page-66. The inline TZID for the VEVENT DTSTART/DTEND remains intact and dependent on dateTimeFormat.
    Old: DTSTART;TZID=+00:00:20201101T090000
    Fixed: DTSTART:20201101T090000
  • Set VTIMEZONE -> DTSTART to local time as stated in https://tools.ietf.org/html/rfc5545#page-66 ("DTSTART in this usage must be specified as a date with a local time value"). Since getTransitions() returns UTC times, use the previous offset to calculate local time.
    Old: 20201101T090000 (Los Angeles 2am transition time, UTC)
    Fixed: 20201101T020000 (Los Angeles 2am transition time, local)
  • Update the VTIMEZONE -> TZOFFSETFROM calculation to look back more than 1 hour to get the previous timezone offset. In some cases, the previous offset was not different from the new offset, possibly due to the 100-second look back putting it in the same offset due to the transition.

Old:

BEGIN:VTIMEZONE
TZID:Australia/Sydney
BEGIN:STANDARD
DTSTART:20200405T020000
TZOFFSETTO:+1000
TZOFFSETFROM:+1000

Fixed:

BEGIN:VTIMEZONE
TZID:Australia/Sydney
BEGIN:STANDARD
DTSTART:20200405T030000
TZOFFSETTO:+1000
TZOFFSETFROM:+1100

@jasvrcek
Copy link
Owner

jasvrcek commented Oct 9, 2020

@icvalyssa this is great, thank you! Can you please update the .ics example files in /tests to reflect your format change, so the tests can pass? See https://travis-ci.org/github/jasvrcek/ICS/jobs/734157770.

@dereuromark
Copy link
Contributor

Is this fix still relevant?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants