Fix RecurrenceRule::timesInInterval() when parsing VTIMEZONE RRULE components

Authored by djarvie on Apr 10 2016, 12:20 PM.


Group Reviewers

Currently, when parsing a VTIMEZONE's RRULE component in ICalTimeZones, RecurrenceRule::timesInInterval() returns no values. This is due to KDateTime::isValid() wrongly returning invalid for an instance which is specified in ClockTime, if the date/time is invalid in LocalTime. This in turn is due to QDateTime being set to Qt::LocalTime, and unlike in Qt4, QDateTime in Qt5 uses the local time zone to validate the date/time. For ClockTime, there is no associated time zone, so it should ignore the local time zone when validating.

In this particular case, the time being validated is the time in the previous daylight saving or standard time at which the transition to the next daylight saving/standard time takes place. For a transition to daylight saving, this is by definition invalid in the local time zone, and in any case it could be for any time zone (not the local one).

Test Plan

New alarms in KAlarm were an hour out without this fix (see After applying the fix, alarm times are correct.

Diff Detail

R82 KDE PIM Application Libraries
Lint Skipped
Unit Tests Skipped
djarvie retitled this revision from to Fix RecurrenceRule::timesInInterval() when parsing VTIMEZONE RRULE components.Apr 10 2016, 12:20 PM
djarvie updated this object.
djarvie edited the test plan for this revision. (Show Details)
djarvie added a reviewer: KDE PIM.
djarvie set the repository for this revision to R82 KDE PIM Application Libraries.
djarvie added a project: KDE PIM.
knauss added a subscriber: knauss.Apr 10 2016, 7:03 PM

looks fine, please apply it to 16.04 branch. And add the BUG line to the commit message to close the bug.

If you have any questions don't hesitate to ask.

Merged commit to master. Can't see how to mark this review as closed.

Well currently phabricator is not listening to the correct repository, thats why it can't closed automatically. I requested the addition now.

In meanwhile you can close it your self - you find an option, for closing it in the action drop down menu.

I can't find an action drop-down menu!

knauss accepted this revision.Apr 13 2016, 12:54 AM
knauss added a reviewer: knauss.

you see the comment field right :D and above this there is this Action and right of it there is a list of possible actions.

This revision is now accepted and ready to land.Apr 13 2016, 12:54 AM

Perhaps as the maintainer you have that option - I couldn't see a close option.

knauss edited edge metadata.Apr 13 2016, 10:06 PM
knauss changed the repository for this revision from R82 KDE PIM Application Libraries to R172 KCalendar Core.

So now kcalcore was added to phabricator, so it should close automatically :D

aacid closed this revision.Feb 27 2017, 12:12 AM