From e3a728d6377857dc2b0f6dfb58e71f097a635cf2 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 25 Jul 2018 20:16:44 +0200 Subject: [PATCH 1/5] Assert on invalid date in wxCalendarCtrl::SetDate() in all ports This was done in wxMSW and wxQt but not in the native GTK+ nor the generic version, even though they still asserted when actually trying to use the invalid parameter later. Make things more clear and consistent by asserting immediately and also document the behaviour more clearly. --- interface/wx/calctrl.h | 3 ++- src/generic/calctrlg.cpp | 2 ++ src/gtk/calctrl.cpp | 4 +++- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/interface/wx/calctrl.h b/interface/wx/calctrl.h index 40bd51493d..a756e458e3 100644 --- a/interface/wx/calctrl.h +++ b/interface/wx/calctrl.h @@ -469,7 +469,8 @@ public: The @a date parameter must be valid and in the currently valid range as set by SetDateRange(), otherwise the current date is not changed and - the function returns @false. + the function returns @false and, additionally, triggers an assertion + failure if the date is invalid. */ virtual bool SetDate(const wxDateTime& date); diff --git a/src/generic/calctrlg.cpp b/src/generic/calctrlg.cpp index 58875a3066..3c5b8ce4e9 100644 --- a/src/generic/calctrlg.cpp +++ b/src/generic/calctrlg.cpp @@ -434,6 +434,8 @@ bool wxGenericCalendarCtrl::EnableMonthChange(bool enable) bool wxGenericCalendarCtrl::SetDate(const wxDateTime& date) { + wxCHECK_MSG( date.IsValid(), false, "invalid date" ); + bool retval = true; bool sameMonth = m_date.GetMonth() == date.GetMonth(), diff --git a/src/gtk/calctrl.cpp b/src/gtk/calctrl.cpp index 221fa53ef5..fbeb93edf8 100644 --- a/src/gtk/calctrl.cpp +++ b/src/gtk/calctrl.cpp @@ -198,7 +198,9 @@ bool wxGtkCalendarCtrl::EnableMonthChange(bool enable) bool wxGtkCalendarCtrl::SetDate(const wxDateTime& date) { - if ( date.IsValid() && !IsInValidRange(date) ) + wxCHECK_MSG( date.IsValid(), false, "invalid date" ); + + if ( !IsInValidRange(date) ) return false; g_signal_handlers_block_by_func(m_widget, From 9293634af148aea2cb6dfb11bd981b95f204d07e Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 30 Jul 2018 21:48:48 +0200 Subject: [PATCH 2/5] Avoid assertion failure when typing into generic wxDatePickerCtrl Don't call wxCalendarCtrl::SetDate() if the current date is invalid, this would just result in an assertion failure. --- src/generic/datectlg.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/generic/datectlg.cpp b/src/generic/datectlg.cpp index d5abccfd3a..f2a13d143a 100644 --- a/src/generic/datectlg.cpp +++ b/src/generic/datectlg.cpp @@ -192,7 +192,8 @@ private: if ( (dt.IsValid() && (!dtOld.IsValid() || dt != dtOld)) || (!dt.IsValid() && dtOld.IsValid()) ) { - SetDate(dt); + if ( dt.IsValid() ) + SetDate(dt); SendDateEvent(dt); } } From b8401d2fb592b93cc16b54d3c1dab03f5a447a98 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 30 Jul 2018 22:03:21 +0200 Subject: [PATCH 3/5] Fix return value of wxCalendarComboPopup::ParseDateTime() Don't pretend that empty string represents a valid date, this made no sense and resulted in unwanted events with an invalid date on clearing the text part of the generic wxDatePickerCtrl. --- src/generic/datectlg.cpp | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/generic/datectlg.cpp b/src/generic/datectlg.cpp index f2a13d143a..94f5906fed 100644 --- a/src/generic/datectlg.cpp +++ b/src/generic/datectlg.cpp @@ -125,12 +125,9 @@ public: { wxASSERT(pDt); - if ( !s.empty() ) - { - pDt->ParseFormat(s, m_format); - if ( !pDt->IsValid() ) - return false; - } + pDt->ParseFormat(s, m_format); + if ( !pDt->IsValid() ) + return false; return true; } @@ -253,7 +250,7 @@ private: virtual void SetStringValue(const wxString& s) wxOVERRIDE { wxDateTime dt; - if ( !s.empty() && ParseDateTime(s, &dt) ) + if ( ParseDateTime(s, &dt) ) SetDate(dt); //else: keep the old value } From d406f23e6787240cafc36b091fb6e5fbe2381e0b Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 30 Jul 2018 22:05:32 +0200 Subject: [PATCH 4/5] Fix behaviour on focus loss in generic wxDatePickerCtrl Don't send any events if the date is invalid when leaving the control. Also, for the controls without wxDP_ALLOWNONE style, ensure that we revert to the valid date we had before when this happens, as such controls must, by definition, always have a valid sate. Note that there already was a check for wxDP_ALLOWNONE, added back in aae5ec8bbe0b14bd2f43708186428e3c09d10739 (and cherry-picked to master as d6781628fda216a01fa99ae3e49805b1a98b6ef2), but it seemed to be reversed, as it only set the date to invalid if the control did not have this style. --- src/generic/datectlg.cpp | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/src/generic/datectlg.cpp b/src/generic/datectlg.cpp index 94f5906fed..f50a1bba65 100644 --- a/src/generic/datectlg.cpp +++ b/src/generic/datectlg.cpp @@ -180,18 +180,33 @@ private: dt = dtOld; } - m_combo->SetText(GetStringValueFor(dt)); - - if ( !dt.IsValid() && HasDPFlag(wxDP_ALLOWNONE) ) - return; - - // notify that we had to change the date after validation - if ( (dt.IsValid() && (!dtOld.IsValid() || dt != dtOld)) || - (!dt.IsValid() && dtOld.IsValid()) ) + if ( dt.IsValid() ) { - if ( dt.IsValid() ) - SetDate(dt); - SendDateEvent(dt); + // Set it at wxCalendarCtrl level. + SetDate(dt); + + // And show it in the text field. + m_combo->SetText(GetStringValue()); + + // And if the date has really changed, send an event about it. + if ( dt != dtOld ) + SendDateEvent(dt); + } + else // Invalid date currently entered. + { + if ( HasDPFlag(wxDP_ALLOWNONE) ) + { + // Clear the text part to indicate that the date is invalid. + // Would it be better to indicate this in some more clear way, + // e.g. maybe by using "[none]" or something like this? + m_combo->SetText(wxString()); + } + else + { + // Restore the original value, as we can't have invalid value + // in this control. + m_combo->SetText(GetStringValue()); + } } } From d7351536c1a1f1eb3c46966e7baad386f2965861 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 30 Jul 2018 22:32:29 +0200 Subject: [PATCH 5/5] Change the data in generic wxDatePickerCtrl immediately Accept the new date typed into the text control immediately if it's valid. This is more consistent with the native MSW control behaviour and avoids semi-duplicated events on text change and then on focus loss that occurred before. --- src/generic/datectlg.cpp | 39 ++++++++++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/src/generic/datectlg.cpp b/src/generic/datectlg.cpp index f50a1bba65..5ddadc3933 100644 --- a/src/generic/datectlg.cpp +++ b/src/generic/datectlg.cpp @@ -121,6 +121,34 @@ public: return m_combo->GetTextCtrl()->IsEmpty(); } + // This is public because it is used by wxDatePickerCtrlGeneric itself to + // change the date when the text control field changes. The reason it's + // done there and not in this class itself is mostly historic. + void ChangeDateAndNotifyIfValid() + { + wxDateTime dt; + if ( !ParseDateTime(m_combo->GetValue(), &dt) ) + { + // The user must be in the process of updating the date, don't do + // anything -- we'll take care of ensuring it's valid on focus loss + // later. + return; + } + + if ( dt == GetDate() ) + { + // No need to send event if the date hasn't changed. + return; + } + + // We change the date immediately, as it's more consistent with the + // native MSW version and avoids another event on focus loss. + SetDate(dt); + + SendDateEvent(dt); + } + +private: bool ParseDateTime(const wxString& s, wxDateTime* pDt) { wxASSERT(pDt); @@ -144,8 +172,6 @@ public: datePicker->GetEventHandler()->ProcessEvent(event); } -private: - void OnCalKey(wxKeyEvent & ev) { if (ev.GetKeyCode() == WXK_ESCAPE && !ev.HasModifiers()) @@ -488,13 +514,8 @@ void wxDatePickerCtrlGeneric::OnText(wxCommandEvent &ev) ev.SetId(GetId()); GetParent()->GetEventHandler()->ProcessEvent(ev); - // We'll create an additional event if the date is valid. - // If the date isn't valid, the user's probably in the middle of typing - wxDateTime dt; - if ( !m_popup || !m_popup->ParseDateTime(m_combo->GetValue(), &dt) ) - return; - - m_popup->SendDateEvent(dt); + if ( m_popup ) + m_popup->ChangeDateAndNotifyIfValid(); } #endif // wxUSE_DATEPICKCTRL