[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-bugs-dist
Subject:    [Bug 179076] kmail aggregration current activity, unknown date
From:       John Layt <john () layt ! net>
Date:       2010-01-05 11:42:48
Message-ID: 20100105114248.147CF2F726 () immanuel ! kde ! org
[Download RAW message or body]

https://bugs.kde.org/show_bug.cgi?id=179076


John Layt <john@layt.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |john@layt.net




--- Comment #5 from John Layt <john layt net>  2010-01-05 12:42:43 ---
In kdepim/messagelist/core/model.cpp you have: 

       // within this month
1369         int weekStartOffset = KGlobal::locale()->workingWeekStartDay() -
1;
1370         int todayWeekNumber = mTodayDate.addDays( -weekStartOffset
).weekNumber();
1371         int dateWeekNumber = dDate.addDays( -weekStartOffset
).weekNumber();
1372         if ( dateWeekNumber == todayWeekNumber )
1373         {
1374           // within this week
1375           groupLabel = KGlobal::locale()->calendar()->weekDayName( dDate
);
1376         } else {
1377           // previous weeks
1378           int weekDiff = todayWeekNumber - dateWeekNumber;
1379           switch( weekDiff )
1380           {
1381             case 1:
1382               groupLabel = mCachedLastWeekLabel;
1383             break;
1384             case 2:
1385               groupLabel = mCachedTwoWeeksAgoLabel;
1386             break;
1387             case 3:
1388               groupLabel = mCachedThreeWeeksAgoLabel;
1389             break;
1390             case 4:
1391               groupLabel = mCachedFourWeeksAgoLabel;
1392             break;
1393             case 5:
1394               groupLabel = mCachedFiveWeeksAgoLabel;
1395             break;
1396             default:
1397               groupLabel = mCachedUnknownLabel; // should never happen
1398             break;
1399           }
1400         }

The default value is where I would guess the 'Unknown' comes from, because the
code assumes that any 2 days in the same month will have the same or
consecutive week numbers up to 5 weeks apart, i.e. traditional week numbering
starting from 1st Jan.  This is not the case as weekNumber() returns the ISO
week number (see http://en.wikipedia.org/wiki/ISO_week_date, yes we need to
rename the api).  For example the week of 28 Dec 2009 to 3 Jan 2010 is week 53
of 2009, so Sun 3 Jan 2010 is week 53/2009 and Mon 4 Jan 2010 is week 1/2010,
so line 1378 will return 1 - 53 = -52 and so get Unknown.

The correct way to calculate this would be something like:

int weekDiff;
if (todayWeekNumber < dateWeekNumber) {
    weekDiff = todayWeekNumber - dateWeekNumber;
} else {
    weekDiff = KLocale::locale()->Calendar()->weeksInYear(dDate) +
todayWeekNumber - dateWeekNumber;
}

This assumes dDate is always <= today, if not the case then I can propose an
alternative.

Three other points I notice that may need fixing.  

First, I'm not sure about the whole adjusting for workingWeekStartDay() thing,
I think I see why you are doing it but I'm not sure it will work with ISO Week
Numbers, I need to think some more.

Secondly, I think lines 1306 - 1320 are OK, other than my point about
workingWeekStartDay().

Thirdly, and this definitely needs fixing, is you mix using QDate methods like
month() and day() and KCalendarSystem methods like formatDate() and
monthName(), which will produced inconsistent results if the user has selected
a non-Gregorian calendar system.  You need to always use the
KLocale::locale()->Calendar() methods (including weekNumber, year, month, day,
addDays, etc) to ensure consistent and localised results.  I'd be happy to help
with a patch for this.

-- 
Configure bugmail: https://bugs.kde.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.
[prev in list] [next in list] [prev in thread] [next in thread]