Fix keyboard editting of start/end times of VTODOs
ClosedPublic

Authored by dkurz on Oct 10 2018, 7:46 PM.

Details

Summary

Editting start/end times of VTODOs using only the keyboard did not
trigger a check whether the VTODO is dirty. If other fields were updated
along with start/end times, everything was fine, but only editting those
times was simply ignored. The patch causes a dirty check for keyboard
edits of the corresponding KTimeComboBoxes.

BUG: 391769
FIXED-IN: 5.9.3

Test Plan

I changed start and end times of a test VTODO. Without the patch,
the change was not saved when hitting OK, nor did I hit my dirty-check
breakpoint. With the patch, everything works as expected: Keyboard editting
results in an updated VTODO, as verified via the Month View tooltip and by
opening the IncidenceEditor again for that VTODO. VEVENTs do not have this
problem even without the patch.

Diff Detail

Repository
R78 PIM: Incidence Editor
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
dkurz created this revision.Oct 10 2018, 7:46 PM
Restricted Application added a project: KDE PIM. · View Herald TranscriptOct 10 2018, 7:46 PM
Restricted Application added a subscriber: kde-pim. · View Herald Transcript
dkurz requested review of this revision.Oct 10 2018, 7:46 PM
winterz accepted this revision.Oct 10 2018, 8:12 PM
This revision is now accepted and ready to land.Oct 10 2018, 8:12 PM
void KTimeComboBox::timeChanged (const QTime & time) [signal]
    Signal if the time has been changed either manually by the user or programatically.

void KTimeComboBox::timeEdited (const QTime & time) [signal]
    Signal if the time is being manually edited by the user.

to me that reads as if timeEdited isn't needed if you are using timeChanged. but .. heck, what do I know?

dkurz added a comment.Oct 10 2018, 8:27 PM
void KTimeComboBox::timeChanged (const QTime & time) [signal]
    Signal if the time has been changed either manually by the user or programatically.
 
void KTimeComboBox::timeEdited (const QTime & time) [signal]
    Signal if the time is being manually edited by the user.

to me that reads as if timeEdited isn't needed if you are using timeChanged. but .. heck, what do I know?

That's exactly what caused the bug I mentioned: timeChanged only fires for changes via mouse and up/down arrow. The function that sets up signal-slot connections for VEVENTs instead of VTODOs even has comments that point out this problem (see line 552).

dkurz added a comment.Oct 10 2018, 8:29 PM

Err... can I already push this to Applications/18.08? We are currently in the short period between 18.08.2 tagging and release. No idea if pushing now would have any consequences.

I'll just wait till tomorrow to be sure.

This revision was automatically updated to reflect the committed changes.