Fix handling of day-of-week in QDateTimeParser and QDateTimeEdit

QDTP's absoluteMax(), setDigit() and getDigit() simply treated
day-of-week as synonym for day-of-month.
Consequently, QDTE::stepBy() did the same.

This meant that wrapping happened at the month boundary, so would jump
within the week if it wrapped around, otherwise the up/down arrow
would "jam" at a particular day of the week when further steps would
leave the month. Instead, when wrapping, wrap round the week while
still moving the day-of-month to match, jumping back or forward a week
to stay within the month on hitting a month boundary; otherwise, stop
backwards stepping on hitting the locale-specific day of the week, or
forward stepping when the step would be to or past this first day.
Fixed various bugs found in the course of testing this.

[ChangeLog][QtWidgets][QDateTimeEdit] Corrected handling of weekdays.
Previously, changes to the week-day were simply changes to the day of
the month. Weekday fields are now handled as such: changes to them do
change the day of the month, but a change that would step past the end
(or start) of the month is adjusted to the relevant day of the nearest
week within the month. When wrapping is disabled, the locale-specific
first and last days of the week are the bounds. Formats which specify
day of week but not day of month will now preserve day of week when
changing month or year, selecting the nearest day of month that
matches.

Change-Id: I7868b000fea7a4bc17a1b5687c44bcd56d42ae90
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Andrei Golubev <andrei.golubev@qt.io>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
This commit is contained in:
Edward Welbourne 2020-08-28 09:23:13 +02:00
parent b0a9825edb
commit d093ec8d03
3 changed files with 138 additions and 40 deletions

View File

@ -96,7 +96,7 @@ int QDateTimeParser::getDigit(const QDateTime &t, int index) const
case MonthSection: return t.date().month(calendar);
case DaySection: return t.date().day(calendar);
case DayOfWeekSectionShort:
case DayOfWeekSectionLong: return t.date().day(calendar);
case DayOfWeekSectionLong: return calendar.dayOfWeek(t.date());
case AmPmSection: return t.time().hour() > 11 ? 1 : 0;
default: break;
@ -107,6 +107,34 @@ int QDateTimeParser::getDigit(const QDateTime &t, int index) const
return -1;
}
/*!
\internal
Difference between two days of the week.
Returns a difference in the range from -3 through +3, so that steps by small
numbers of days move us through the month in the same direction as through
the week.
*/
static int dayOfWeekDiff(int sought, int held)
{
const int diff = sought - held;
return diff < -3 ? diff + 7 : diff > 3 ? diff - 7 : diff;
}
static bool preferDayOfWeek(const QList<QDateTimeParser::SectionNode> &nodes)
{
// True precisely if there is a day-of-week field but no day-of-month field.
bool result = false;
for (const auto &node : nodes) {
if (node.type & QDateTimeParser::DaySection)
return false;
if (node.type & QDateTimeParser::DayOfWeekSectionMask)
result = true;
}
return result;
}
/*!
\internal
Sets a digit in a datetime. E.g.
@ -127,9 +155,12 @@ bool QDateTimeParser::setDigit(QDateTime &v, int index, int newVal) const
return false;
}
QCalendar::YearMonthDay date = calendar.partsFromDate(v.date());
const QDate oldDate = v.date();
QCalendar::YearMonthDay date = calendar.partsFromDate(oldDate);
if (!date.isValid())
return false;
int weekDay = calendar.dayOfWeek(oldDate);
enum { NoFix, MonthDay, WeekDay } fixDay = NoFix;
const QTime time = v.time();
int hour = time.hour();
@ -150,8 +181,6 @@ bool QDateTimeParser::setDigit(QDateTime &v, int index, int newVal) const
case YearSection: date.year = newVal; break;
case MonthSection: date.month = newVal; break;
case DaySection:
case DayOfWeekSectionShort:
case DayOfWeekSectionLong:
if (newVal > 31) {
// have to keep legacy behavior. setting the
// date to 32 should return false. Setting it
@ -159,6 +188,15 @@ bool QDateTimeParser::setDigit(QDateTime &v, int index, int newVal) const
return false;
}
date.day = newVal;
fixDay = MonthDay;
break;
case DayOfWeekSectionShort:
case DayOfWeekSectionLong:
if (newVal > 7 || newVal <= 0)
return false;
date.day += dayOfWeekDiff(newVal, weekDay);
weekDay = newVal;
fixDay = WeekDay;
break;
case TimeZoneSection:
if (newVal < absoluteMin(index) || newVal > absoluteMax(index))
@ -176,9 +214,27 @@ bool QDateTimeParser::setDigit(QDateTime &v, int index, int newVal) const
if (!(node.type & DaySectionMask)) {
if (date.day < cachedDay)
date.day = cachedDay;
fixDay = MonthDay;
if (weekDay > 0 && weekDay <= 7 && preferDayOfWeek(sectionNodes)) {
const int max = calendar.daysInMonth(date.month, date.year);
if (max > 0 && date.day > max)
date.day = max;
const int newDoW = calendar.dayOfWeek(calendar.dateFromParts(date));
if (newDoW > 0 && newDoW <= 7)
date.day += dayOfWeekDiff(weekDay, newDoW);
fixDay = WeekDay;
}
}
if (fixDay != NoFix) {
const int max = calendar.daysInMonth(date.month, date.year);
if (date.day > max)
date.day = max;
// max > 0 precisely if the year does have such a month
if (max > 0 && date.day > max)
date.day = fixDay == WeekDay ? date.day - 7 : max;
else if (date.day < 1)
date.day = fixDay == WeekDay ? date.day + 7 : 1;
Q_ASSERT(fixDay != WeekDay
|| calendar.dayOfWeek(calendar.dateFromParts(date)) == weekDay);
}
const QDate newDate = calendar.dateFromParts(date);
@ -231,9 +287,10 @@ int QDateTimeParser::absoluteMax(int s, const QDateTime &cur) const
case MonthSection:
return calendar.maximumMonthsInYear();
case DaySection:
return cur.isValid() ? cur.date().daysInMonth(calendar) : calendar.maximumDaysInMonth();
case DayOfWeekSectionShort:
case DayOfWeekSectionLong:
return cur.isValid() ? cur.date().daysInMonth(calendar) : calendar.maximumDaysInMonth();
return 7;
case AmPmSection:
return 1;
default:
@ -927,19 +984,21 @@ QDateTimeParser::parseSection(const QDateTime &currentValue, int sectionIndex, i
/*!
\internal
Returns a day-number, in the same month as \a rough and as close to \a rough's
day number as is valid, that \a calendar puts on the day of the week indicated
by \a weekDay.
Returns the day-number of a day, as close as possible to the given \a day, in
the specified \a month of \a year for the given \a calendar, that falls on the
day of the week indicated by \a weekDay.
*/
static int weekDayWithinMonth(QCalendar calendar, QDate rough, int weekDay)
static int weekDayWithinMonth(QCalendar calendar, int year, int month, int day, int weekDay)
{
// TODO: can we adapt this to cope gracefully with intercallary days (day of
// week > 7) without making it slower for more widely-used calendars ?
int day = rough.day(calendar) + weekDay - calendar.dayOfWeek(rough);
const int maxDay = calendar.daysInMonth(month, year); // 0 if no such month
day = maxDay > 1 ? qBound(1, day, maxDay) : qMax(1, day);
day += dayOfWeekDiff(weekDay, calendar.dayOfWeek(QDate(year, month, day, calendar)));
if (day <= 0)
return day + 7;
if (day > rough.daysInMonth(calendar))
if (maxDay > 0 && day > maxDay)
return day - 7;
return day;
}
@ -1027,7 +1086,7 @@ static QDate actualDate(QDateTimeParser::Sections known, const QCalendar &calend
if ((known & QDateTimeParser::DaySection) == 0) {
// Relatively easy to fix.
day = weekDayWithinMonth(calendar, actual, dayofweek);
day = weekDayWithinMonth(calendar, year, month, day, dayofweek);
actual = QDate(year, month, day, calendar);
return actual;
}
@ -1300,24 +1359,26 @@ QDateTimeParser::scanString(const QDateTime &defaultValue, bool fixup) const
}
}
const auto fieldType = sectionType(currentSectionIndex);
const QDate date(year, month, day, calendar);
if (dayofweek != calendar.dayOfWeek(date)
if ((!date.isValid() || dayofweek != calendar.dayOfWeek(date))
&& state == Acceptable && isSet & DayOfWeekSectionMask) {
if (isSet & DaySection)
conflicts = true;
const SectionNode &sn = sectionNode(currentSectionIndex);
if (sn.type & DayOfWeekSectionMask || currentSectionIndex == -1) {
// dayofweek should be preferred
day = weekDayWithinMonth(calendar, date, dayofweek);
// Change to day of week should adjust day of month;
// when day of month isn't set, so should change to year or month.
if (currentSectionIndex == -1 || fieldType & DayOfWeekSectionMask
|| (!conflicts && (fieldType & (YearSectionMask | MonthSection)))) {
day = weekDayWithinMonth(calendar, year, month, day, dayofweek);
QDTPDEBUG << year << month << day << dayofweek
<< calendar.dayOfWeek(QDate(year, month, day, calendar));
}
}
bool needfixday = false;
if (sectionType(currentSectionIndex) & DaySectionMask) {
if (fieldType & DaySectionMask) {
cachedDay = day;
} else if (cachedDay > day) {
} else if (cachedDay > day && !(isSet & DayOfWeekSectionMask && state == Acceptable)) {
day = cachedDay;
needfixday = true;
}

View File

@ -2101,15 +2101,45 @@ QDateTime QDateTimeEditPrivate::stepBy(int sectionIndex, int steps, bool test) c
v = q->dateTimeFromText(str);
int val = getDigit(v, sectionIndex);
val += steps;
const int min = absoluteMin(sectionIndex);
const int max = absoluteMax(sectionIndex, value.toDateTime());
if (val < min) {
val = (wrapping ? max - (min - val) + 1 : min);
} else if (val > max) {
val = (wrapping ? min + val - max - 1 : max);
if (sn.type & DayOfWeekSectionMask) {
// Must take locale's first day of week into account when *not*
// wrapping; min and max don't help us.
#ifndef QT_ALWAYS_WRAP_WEEKDAY // (documentation, not an actual define)
if (!wrapping) {
/* It's not clear this is ever really a desirable behavior.
It refuses to step backwards from the first day of the week or
forwards from the day before, only allowing day-of-week stepping
from start to end of one week. That's strictly what non-wrapping
behavior must surely mean, when put in locale-neutral terms.
It is, however, likely that users would prefer the "more natural"
behavior of cycling through the week.
*/
const int first = int(locale().firstDayOfWeek()); // Mon = 1 through 7 = Sun
val = qBound(val < first ? first - 7 : first,
val + steps,
val < first ? first - 1 : first + 6);
} else
#endif
{
val += steps;
}
// Restore to range from 1 through 7:
val = val % 7;
if (val <= 0)
val += 7;
} else {
val += steps;
const int span = max - min + 1;
if (val < min)
val = wrapping ? val + span : min;
else if (val > max)
val = wrapping ? val - span : max;
}
const int oldDay = v.date().day(calendar);
@ -2132,7 +2162,8 @@ QDateTime QDateTimeEditPrivate::stepBy(int sectionIndex, int steps, bool test) c
const QDateTime minimumDateTime = minimum.toDateTime();
const QDateTime maximumDateTime = maximum.toDateTime();
// changing one section should only modify that section, if possible
if (sn.type != AmPmSection && (v < minimumDateTime || v > maximumDateTime)) {
if (sn.type != AmPmSection && !(sn.type & DayOfWeekSectionMask)
&& (v < minimumDateTime || v > maximumDateTime)) {
const int localmin = getDigit(minimumDateTime, sectionIndex);
const int localmax = getDigit(maximumDateTime, sectionIndex);

View File

@ -4009,20 +4009,23 @@ void tst_QDateTimeEdit::dateEditCorrectSectionSize_data()
KeyPairList shortAndLongNameIssueKeypresses;
shortAndLongNameIssueKeypresses << key(Qt::Key_Tab) << key(Qt::Key_3) << key(Qt::Key_1) << key(Qt::Key_Up);
// When day-of-week is specified, rather than day-of-month, changing month
// cares more about preserving day-of-week than day-of-month, so Jan/31 ->
// Feb picks 28th even in a leap year, as that's exactly four weeks later.
QTest::newRow("no fixday, leap, yy/M/dddd")
<< ozzy << y2kStart << QString::fromLatin1("yy/M/dddd")
<< threeDigitDayIssueKeypresses_DayName
<< QDate(2000, 2, 29) << QString::fromLatin1("00/2/Tuesday");
<< QDate(2000, 2, 28) << QString::fromLatin1("00/2/Monday");
QTest::newRow("no fixday, leap, yy/M/ddd")
<< ozzy << y2kStart << QString::fromLatin1("yy/M/ddd")
<< threeDigitDayIssueKeypresses_DayName
<< QDate(2000, 2, 29) << QString::fromLatin1("00/2/Tue");
<< QDate(2000, 2, 28) << QString::fromLatin1("00/2/Mon");
QTest::newRow("no fixday, leap, yy/MM/dddd")
<< ozzy << y2kStart << QString::fromLatin1("yy/MM/dddd")
<< threeDigitDayIssueKeypresses_DayName
<< QDate(2000, 2, 29) << QString::fromLatin1("00/02/Tuesday");
<< QDate(2000, 2, 28) << QString::fromLatin1("00/02/Monday");
QTest::newRow("fixday, leap, yy/MM/dd")
<< ozzy << y2kStart << QString::fromLatin1("yy/MM/dd")
@ -4072,12 +4075,12 @@ void tst_QDateTimeEdit::dateEditCorrectSectionSize_data()
QTest::newRow("no fixday, leap, yyyy/M/dddd")
<< ozzy << y2kStart << QString::fromLatin1("yyyy/M/dddd")
<< threeDigitDayIssueKeypresses_DayName
<< QDate(2000, 2, 29) << QString::fromLatin1("2000/2/Tuesday");
<< QDate(2000, 2, 28) << QString::fromLatin1("2000/2/Monday");
QTest::newRow("no fixday, leap, yyyy/MM/dddd")
<< ozzy << y2kStart << QString::fromLatin1("yyyy/MM/dddd")
<< threeDigitDayIssueKeypresses_DayName
<< QDate(2000, 2, 29) << QString::fromLatin1("2000/02/Tuesday");
<< QDate(2000, 2, 28) << QString::fromLatin1("2000/02/Monday");
QTest::newRow("fixday, leap, yyyy/dd/MM")
<< ozzy << y2kStart << QString::fromLatin1("yyyy/dd/MM")
@ -4112,12 +4115,12 @@ void tst_QDateTimeEdit::dateEditCorrectSectionSize_data()
QTest::newRow("fixday, leap, yyyy/dddd/M")
<< ozzy << y2kStart << QString::fromLatin1("yyyy/dddd/M")
<< threeDigitDayIssueKeypresses_DayName_YearDayMonth
<< QDate(2000, 2, 29) << QString::fromLatin1("2000/Tuesday/2");
<< QDate(2000, 2, 28) << QString::fromLatin1("2000/Monday/2");
QTest::newRow("fixday, leap, yyyy/dddd/MM")
<< ozzy << y2kStart << QString::fromLatin1("yyyy/dddd/MM")
<< threeDigitDayIssueKeypresses_DayName_YearDayMonth
<< QDate(2000, 2, 29) << QString::fromLatin1("2000/Tuesday/02");
<< QDate(2000, 2, 28) << QString::fromLatin1("2000/Monday/02");
QTest::newRow("fixday, leap, d/M/yyyy")
<< ozzy << y2kStart << QString::fromLatin1("d/M/yyyy")
@ -4137,7 +4140,7 @@ void tst_QDateTimeEdit::dateEditCorrectSectionSize_data()
QTest::newRow("fixday, leap, dddd/MM/yyyy")
<< ozzy << y2kStart << QString::fromLatin1("dddd/MM/yyyy")
<< threeDigitDayIssueKeypresses_DayName_DayMonthYear
<< QDate(2000, 2, 29) << QString::fromLatin1("Tuesday/02/2000");
<< QDate(2000, 2, 28) << QString::fromLatin1("Monday/02/2000");
QTest::newRow("fixday, leap, d/yy/M")
<< ozzy << y2kStart << QString::fromLatin1("d/yy/M")
@ -4172,12 +4175,12 @@ void tst_QDateTimeEdit::dateEditCorrectSectionSize_data()
QTest::newRow("fixday, leap, dddd/yy/M")
<< ozzy << y2kStart << QString::fromLatin1("dddd/yy/M")
<< threeDigitDayIssueKeypresses_DayName_DayYearMonth
<< QDate(2000, 2, 29) << QString::fromLatin1("Tuesday/00/2");
<< QDate(2000, 2, 28) << QString::fromLatin1("Monday/00/2");
QTest::newRow("fixday, leap, dddd/yy/MM")
<< ozzy << y2kStart << QString::fromLatin1("dddd/yy/MM")
<< threeDigitDayIssueKeypresses_DayName_DayYearMonth
<< QDate(2000, 2, 29) << QString::fromLatin1("Tuesday/00/02");
<< QDate(2000, 2, 28) << QString::fromLatin1("Monday/00/02");
QTest::newRow("fixday, leap, M/d/yy")
<< ozzy << y2kStart << QString::fromLatin1("M/d/yy")
@ -4197,7 +4200,7 @@ void tst_QDateTimeEdit::dateEditCorrectSectionSize_data()
QTest::newRow("fixday, leap, M/dddd/yyyy")
<< ozzy << y2kStart << QString::fromLatin1("M/dddd/yyyy")
<< threeDigitDayIssueKeypresses_DayName_MonthDayYear
<< QDate(2000, 2, 29) << QString::fromLatin1("2/Tuesday/2000");
<< QDate(2000, 2, 28) << QString::fromLatin1("2/Monday/2000");
QTest::newRow("fixday, leap, MM/dd/yyyy")
<< ozzy << y2kStart << QString::fromLatin1("MM/dd/yyyy")
@ -4207,7 +4210,7 @@ void tst_QDateTimeEdit::dateEditCorrectSectionSize_data()
QTest::newRow("fixday, leap, MM/dddd/yyyy")
<< ozzy << y2kStart << QString::fromLatin1("MM/dddd/yyyy")
<< threeDigitDayIssueKeypresses_DayName_MonthDayYear
<< QDate(2000, 2, 29) << QString::fromLatin1("02/Tuesday/2000");
<< QDate(2000, 2, 28) << QString::fromLatin1("02/Monday/2000");
QTest::newRow("fixday, leap, M/yyyy/dd")
<< ozzy << y2kStart << QString::fromLatin1("M/yyyy/dd")
@ -4275,6 +4278,9 @@ void tst_QDateTimeEdit::dateEditCorrectSectionSize()
edit.setDisplayFormat(displayFormat);
edit.show();
edit.setFocus();
// Day-of-week tests rely on advance through week advancing the
// day-of-month, so not stopping at the locale's first day of the week:
edit.setWrapping(true);
// For some reason, we need to set the selected section for the dd/MM/yyyy tests,
// otherwise the 3 is inserted at the front of 01/01/2000 (301/01/2000), instead of the
// selected text being replaced. This is not an issue for the yyyy/MM/dd format though...