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

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

Details

Reviewers
knauss
Group Reviewers
KDE PIM
Summary

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 https://bugs.kde.org/show_bug.cgi?id=360674). After applying the fix, alarm times are correct.

Diff Detail

Repository
R82 KDE PIM Application Libraries
Lint
Lint Skipped
Unit
Unit Tests Skipped
djarvie updated this revision to Diff 3256.Apr 10 2016, 12:20 PM
djarvie retitled this revision from to Fix RecurrenceRule::timesInInterval() when parsing VTIMEZONE RRULE components.
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