mozilla-thunderbird locates 2/6/2008 as last week in 4/6/2008

[Dexo]

Binary package hint: mozilla-thunderbird

Hi,

Thunderbird version 2.0.0.14 (20080502)
Folowing this menus (Translating from my menus localiced in ES_es)
View => Order => Desc.
View => Order => Group

Today is wensday 04/jun/2008, and monday 04/jun/2008 appears in last week.

ProblemType: Bug
Architecture: i386
Date: Wed Jun 4 14:46:46 2008
Dependencies:

DistroRelease: Ubuntu 7.10
NonfreeKernelModules: vmnet vmmon
Package: mozilla-thunderbird None
PackageArchitecture: all
SourcePackage: thunderbird
Uname: Linux Pesc11-128 2.6.22-14-generic #1 SMP Tue Feb 12 07:42:25 UTC 2008 i686 GNU/Linux

[Dexo (interruptux)]

Oops!

I mean:

Today is wensday 04/jun/2008, and monday 02/jun/2008 appears in last week.

[John Vivirito (gnomefreak)]

Dexo wrote:
> Oops!
>
> I mean:
>
> Today is wensday 04/jun/2008, and monday 02/jun/2008 appears in last
> week.
>
I figured that is what you meant.
Is TB the only app that has this issue?
Where are you seeing this date?

--
Sincerely Yours,
     John Vivirito

https://launchpad.net/~gnomefreak
https://wiki.ubuntu.com/JohnVivirito
Linux User# 414246

[Dexo (interruptux)]

> Is TB the only app that has this issue?
It's my only mail client.
And calendars here starts at monday, and the rest (kde apps) behaves normally and respect that.

> Where are you seeing this date?
In the date group header.
That is in the mail list window, that is sorted and grouped by date.

[John Vivirito (gnomefreak)]

Dexo wrote:
>> Is TB the only app that has this issue?
> It's my only mail client.
> And calendars here starts at monday, and the rest (kde apps) behaves normally and respect that.
>
>> Where are you seeing this date?
> In the date group header.
> That is in the mail list window, that is sorted and grouped by date.
>
You mean the date on the right of every piece of mail? Can you please
attach a screenshot of the issue to this bug report please.

--
Sincerely Yours,
     John Vivirito

https://launchpad.net/~gnomefreak
https://wiki.ubuntu.com/JohnVivirito
Linux User# 414246

[Dexo (interruptux)]

# Today is
$ date --rfc-3339=date
2008-06-07
---------
Snapshot dates
"La semana pasada" Group (= Last week. Translated)
05/06/08 (= 2008-06-05)
05/06/08 (= 2008-06-05)
"Hace dos semanas" Group (= Two weeks ago)
29/05/08 (= 2008-05-29)
---------
# Actually 2008-06-05 is in the same week and 2008-05-29 in last week
$ cal -m 06 2008
     June 2008
Mo Tu We Th Fr Sa Su
                   1
 2 3 4 [5] 6 [7] 8
 9 10 11 12 13 14 15
16 17 18 19 20 21 22
23 24 25 26 27 28 29
30
$ cal -m 05 2008
      May 2008
Mo Tu We Th Fr Sa Su
          1 2 3 4
 5 6 7 8 9 10 11
12 13 14 15 16 17 18
19 20 21 22 23 24 25
26 27 28[29]30 31

# I don't have an example but it don't respect Monday as the first
# day of the week and puts 2008-06-01 and 2008-06-05 in the same week group

[Joel Goguen (jgoguen)]

Thank you for taking the time to report this bug and helping to make Ubuntu better. You reported this bug a while ago and there hasn't been any activity in it recently. We were wondering if this is still an issue for you. Can you try with the latest Ubuntu release? Thanks in advance.

[David Fernández (davidf-76)]

This bug exists yet, and it's the same in Windows XP.
I have Ubuntu Intrepid Ibex and Thunderbird 2.0.0.19 (20090105).

I've attached an image to see it clearly (it's in spanish: 'Hoy' means 'today', 'La semana pasada' means 'Last week' and 'Hace dos semanas' means 'Two weeks ago'). Today is March 13th, 2009. And March 10th (Monday) is in the last week and March 5th is in two weeks ago (and it was Thursday, have passed one week only!)

[Joel Goguen (jgoguen)]

Confirming this based on multiple reports, and Mozilla 315114 (https://bugzilla.mozilla.org/show_bug.cgi?id=315114).

[BenoitL (benoit-leseul)]

As discussed on the l10n newsgroup, the "Last Week" sorting group label in the message list is misleading. It doesn't mean the last calendar week but the last seven days.
http://groups.google.com/group/mozilla.dev.l10n/browse_thread/thread/c597551173c2703

The label could be changed to "Last seven days", or the grouping could be made on calendar weeks instead. John Wilcock proposed to split that in "Earlier this week" and "Last week" based on calendar weeks.

[Rsx11m-pub (rsx11m-pub)]

Benoit, since you filed that for Thunderbird and not SeaMonkey, it is covered
by bug 315114 already. Thus, I'd suggest that you either retarget the bug for
SeaMonkey as its counterpart or resolve as duplicate of the Thunderbird bug.

[Bugzilla-babylonsounds (bugzilla-babylonsounds)]

(In reply to comment #1)
> Benoit, since you filed that for Thunderbird and not SeaMonkey, it is covered
> by bug 315114 already.

I don't think that this bug is a dupe of bug 315114. Basically I consider the proposal in bug 315114 as very unintuitive and therefore as WONTFIX.

[Bugzilla-babylonsounds (bugzilla-babylonsounds)]

*** Bug 315114 has been marked as a duplicate of this bug. ***

[Rsx11m-pub (rsx11m-pub)]

Well, at least you seem to agree that both bugs cover the same topic... ;-)

[Mkmelin+mozilla (mkmelin+mozilla)]

As I wrote in bug 315114 comment 12, at least to me the current groups aren't that useful, maybe we could come up with better ones.

[John-tradoc (john-tradoc)]

I'd suggest that many people split up recent past time using a calendar-week paradigm:

I'd suggest the following categories, similar to those proposed in bug 315114:
• Today
• Yesterday
• Earlier this week (i.e. sometime between the beginning of the current calendar week and the day before yesterday). This category obviously would only exist if today is at least the third day of the week, bearing in mind that different locales start the week on different days. I think this would be more useful than a day-by-day view for earlier than today and yesterday, as by the time we get to, say, Friday, people no longer remember which day at the beginning of the week a message was received.
• Last week (i.e. the previous calendar week, not the current usage)

For older messages I'm not sure which categories would be best, not even how useful it is to continue defining categories. One could argue for "Earlier this month" (where applicable) and "Last month", for example, or use month names in the same way as Firefox history - but IMO we don't want too fine a granularity here either.

[Francesco-lodolo (francesco-lodolo)]

(In reply to comment #6)

> • Earlier this week (i.e. sometime between the beginning of the current
> calendar week and the day before yesterday).

I'm afraid that the resulting localized string would be awfully long for many languages.

[Bugzilla2007 (bugzilla2007)]

(In reply to comment #7)
> (In reply to comment #6)
>
> > • Earlier this week (i.e. sometime between the beginning of the current
> > calendar week and the day before yesterday).
> I'm afraid that the resulting localized string would be awfully long for many
> languages.

Which wouldn't hurt much as there's space enough on the group header.

[Micah Gersten (micahg)]

Upstream marked as duplicate.

[Tom Corcoran (boardtc)]

How about "Last 7 days", that's 12 characters in English, shorter than the next label "Two Weeks ago" (14 characters). Regardless this is a quick change and hopefully it can be rolled into the next release.

[John-tradoc (john-tradoc)]

"Last 7 days" would indeed make the current groupings unambiguous (in English at least) and might be a desirable quick fix, but this bug (and to an even greater extent its duplicate bug 315114) has focused on alternative and hopefully more intuitive groupings based on calendar weeks.

[Luc-mousseau (luc-mousseau)]

My vote would be to make the groupings work logically by calendar week/month, ie: "last week" is the week previous to this one based on the user's selection in Views > "Start the week on".

I also wouldn't mind seeing John's suggestion of "Earlier this week" (or maybe just "This Week" for a shorter string), etc. It should really be based on the largest logical calendar block though, not just on number of days as it currently is.

..Although I wonder why it couldn't be more granular for old email, if done by breaking out more headings under "Old Mail"? ie (hopefully this displays):

[-] Old Mail
 |- [+] 2008
 |- [+] 2009
 |- [-] 2010
     |- [+] January
     |- [+] February
...etc...

[Marcel-frightanic (marcel-frightanic)]

I stumbled upon this because I moaned about this in the TB support group: http://groups.google.com/group/mozilla.support.thunderbird/browse_thread/thread/668d78f2962ab1cb

I quite like John Wilcock's suggestion in Comment 6.

[BenoitL (benoit-leseul)]

*** Bug 591345 has been marked as a duplicate of this bug. ***

[Schoeni (schoeni)]

*** Bug 627655 has been marked as a duplicate of this bug. ***

[John-tradoc (john-tradoc)]

Looking back at this relatively old bug, I wonder whether the lack of agreement on categories (and hence the lack of progress) might be at least partly because the preferred categorisation varies from locale to locale. None of those who complain about the existing categories seem to be English-language users, and that can't be entirely a coincidence.

It would be perfectly possible in theory to define a structure whereby different locales can choose their own categorisation depending whether the mindset in their locale conceptualises a "week" as last-7-consecutive-days or as a calendar week.

I've no idea, though, whether the current i18n/l10n structures allow for a choice like this. Maybe this would need to be a (hidden?) preference, with corresponding UI strings to be translated for both paradigms.

[Bienvenu (bienvenu)]

(In reply to comment #15)

> I've no idea, though, whether the current i18n/l10n structures allow for a
> choice like this. Maybe this would need to be a (hidden?) preference, with
> corresponding UI strings to be translated for both paradigms.

l10n has no control over this, so it would have to be a pref the localizations could set. But I'm pretty sure I've heard english-language user complain about this, if not in bugzilla, at least in Get Satisfaction issues.

[Schoeni (schoeni)]

It should be simple to define a logical structure.
A time period has a clearly defined start and end like a day that starts at 0:00 and ends on 23:59:59.999 (shortly before 0:00). If we would classify a time by hours as example 12:21 it would be "this hour", everithing in the range of 11:00...11:59 would be "one hour before" and then "earlier this day".
The same scheme applies to days. If we define the start of the week at Monday(like defined in Lightning) then we could have Thursday as "Today", Wednesday as "Yesterday", Monday and Tuesday as "Earlier this week". With the use of "Last week" we could solve any ordering related problem. If today would be Monday then Monday is "Today", Sunday is "Yesterday", Satterday is in the group "Last week". Everything before the last week could go to "Older Messages".
Sure we could do the same with months or even years ... but this could be overdrawn.

[Rsx11m-pub (rsx11m-pub)]

*** Bug 941197 has been marked as a duplicate of this bug. ***

[Neal-o (neal-o)]

Created attachment 8337301
fix bucket groups to be based off of a calendar week concept

Attached is a first attempt patch to address this bug.
It aligns the buckets > yesterday to week boundaries, and adds a lastWeek bucket.

I need information on where to get the beginning of calendar week localization runtime information

Also, I did the English localization, but none of the others. I would need help with that.

This needs testers to validate that I didn't break something else.

[Jim (squibblyflabbetydoo)]

Comment on attachment 8337301
fix bucket groups to be based off of a calendar week concept

Review of attachment 8337301:
-----------------------------------------------------------------

This isn't a full review; just a quick pass. David Bienvenu is unlikely to be super-response, since I believe he works at Google now. I've redirected the review to someone who might be a better pick (sorry if I redirected wrong!).

::: mail/locales/en-US/chrome/messenger/messenger.properties
@@ +178,3 @@
> lastWeek=Last Week
> twoWeeksAgo=Two Weeks Ago
> older=Older

We might need to change the names of these strings to force localizers to update them; after this patch, lastWeek doesn't refer to the same thing. A localizer might have translated that to something like "Within the last 7 days", which is accurate for how the code works pre-patch, but would be wrong post-patch.

::: mailnews/base/src/nsMsgGroupView.cpp
@@ -135,5 @@
> - int64_t GMTLocalTimeShift = currentExplodedTime.tm_params.tp_gmt_offset +
> - currentExplodedTime.tm_params.tp_dst_offset;
> - GMTLocalTimeShift *= PR_USEC_PER_SEC;
> - currentTime += GMTLocalTimeShift;
> - dateOfMsg += GMTLocalTimeShift;

Why'd you remove the time-shifting? Isn't that important, since we want to figure out when midnight is in local time, not GMT?

@@ +135,5 @@
> + // the localization for first day of calendar
> + int64_t todayMidnight = currentTime - currentTime % PR_USEC_PER_DAY;
> + int64_t yesterday = todayMidnight - PR_USEC_PER_DAY;
> + int64_t thisWeek = todayMidnight - (PR_USEC_PER_DAY *
> + (currentExplodedTime.tm_wday + 0));

Why the "+ 0" here?

@@ +137,5 @@
> + int64_t yesterday = todayMidnight - PR_USEC_PER_DAY;
> + int64_t thisWeek = todayMidnight - (PR_USEC_PER_DAY *
> + (currentExplodedTime.tm_wday + 0));
> + int64_t lastWeek = thisWeek - (PR_USEC_PER_DAY * 7);
> + int64_t twoWeeks = lastWeek - (PR_USEC_PER_DAY * 7);

Nit: I'd call this lastTwoWeeks to be clearer.

@@ +778,5 @@
> if (m_kOldMailString.IsEmpty())
> m_kOldMailString.Adopt(GetString(NS_LITERAL_STRING("older").get()));
> aValue.Assign(m_kOldMailString);
> break;
> + case Invalid:

I don't think we really need to add this here; the default case will catch it.

::: mailnews/base/src/nsMsgGroupView.h
@@ +46,5 @@
> virtual void InternalClose();
> nsMsgGroupThread *AddHdrToThread(nsIMsgDBHdr *msgHdr, bool *pNewThread);
> virtual nsresult HashHdr(nsIMsgDBHdr *msgHdr, nsString& aHashKey);
> +
> + enum AgeBucket_t { Invalid, Today, Yesterday, ThisWeek, LastWeek, TwoWeeksAgo, Older };

I think it would make more sense to put this at the top of the protected: list, and also to put each value on its own line.

I'm not sure what Mozilla's standard for naming enums is; I don't think it's Foo_t though.

[Jim (squibblyflabbetydoo)]

Oh, I should explain more about localization: you're not responsible for updating any of the strings except for en-US. Dedicated localizers will come in and update them for all the other locales. However, if we change the meaning of any string in the code, we generally need to change the name of the string too, since that forces the localizers to update the string. If the string name stays the same, they usually won't notice the change*.

* Yeah, our tools could use improvement in this department...

[Neal-o (neal-o)]

Thanks for the comments.

(In reply to Jim Porter (:squib) from comment #20)
> Comment on attachment 8337301
> fix bucket groups to be based off of a calendar week concept
>
> Review of attachment 8337301:
> -----------------------------------------------------------------
>
> This isn't a full review; just a quick pass. David Bienvenu is unlikely to
> be super-response, since I believe he works at Google now. I've redirected
> the review to someone who might be a better pick (sorry if I redirected
> wrong!).

Thanks for pointing it to anyone who'll respond.

>
> ::: mail/locales/en-US/chrome/messenger/messenger.properties
> @@ +178,3 @@
> > lastWeek=Last Week
> > twoWeeksAgo=Two Weeks Ago
> > older=Older
>
> We might need to change the names of these strings to force localizers to
> update them; after this patch, lastWeek doesn't refer to the same thing. A
> localizer might have translated that to something like "Within the last 7
> days", which is accurate for how the code works pre-patch, but would be
> wrong post-patch.

noted. suggestions ?

>
> ::: mailnews/base/src/nsMsgGroupView.cpp
> @@ -135,5 @@
> > - int64_t GMTLocalTimeShift = currentExplodedTime.tm_params.tp_gmt_offset +
> > - currentExplodedTime.tm_params.tp_dst_offset;
> > - GMTLocalTimeShift *= PR_USEC_PER_SEC;
> > - currentTime += GMTLocalTimeShift;
> > - dateOfMsg += GMTLocalTimeShift;
>
> Why'd you remove the time-shifting? Isn't that important, since we want to
> figure out when midnight is in local time, not GMT?

It didn't seem to work correctly.
With it, I had messages that were very near to the bucket boundaries, that sometimes, were on the wrong side of the bucket boundary.

Commented out, all my messages seemed to align correctly at the boundaries.

>
> @@ +135,5 @@
> > + // the localization for first day of calendar
> > + int64_t todayMidnight = currentTime - currentTime % PR_USEC_PER_DAY;
> > + int64_t yesterday = todayMidnight - PR_USEC_PER_DAY;
> > + int64_t thisWeek = todayMidnight - (PR_USEC_PER_DAY *
> > + (currentExplodedTime.tm_wday + 0));
>
> Why the "+ 0" here?

You missed the line above. It's a place holder for the start of calendar week localization offset.

>
> @@ +137,5 @@
> > + int64_t yesterday = todayMidnight - PR_USEC_PER_DAY;
> > + int64_t thisWeek = todayMidnight - (PR_USEC_PER_DAY *
> > + (currentExplodedTime.tm_wday + 0));
> > + int64_t lastWeek = thisWeek - (PR_USEC_PER_DAY * 7);
> > + int64_t twoWeeks = lastWeek - (PR_USEC_PER_DAY * 7);
>
> Nit: I'd call this lastTwoWeeks to be clearer.

Noted.

>
> @@ +778,5 @@
> > if (m_kOldMailString.IsEmpty())
> > m_kOldMailString.Adopt(GetString(NS_LITERAL_STRING("older").get()));
> > aValue.Assign(m_kOldMailString);
> > break;
> > + case Invalid:
>
> I don't think we really need to add this here; the default case will catch
> it.

Yeah, it was a Nit on my part, default aside, I recalled that some compilers complain if every instance of an enum is not handled, so I was being pro-active. :)

>
> ::: mailnews/base...

[Neal-o (neal-o)]

Yeah, uncle, after further testing, the time zone adjustments go back in...

[Neal-o (neal-o)]

Comment on attachment 8337301
fix bucket groups to be based off of a calendar week concept

I'm attaching an updated patch to address review comments by :squib

[Neal-o (neal-o)]

Created attachment 8337339
fix bucket groups to be based off of a calendar week concept

[Jim (squibblyflabbetydoo)]

Ok, I looked into the issue of how to determine the first day of the week. There's a localization file here, used for the datepicker[1] and there's also a pref in Lightning[2]. I'm pretty sure most OSes also specify this somewhere, but we don't detect that. I'm CCing one of the Lightning devs so that we can discuss the correct solution here. It might be useful to add support to Gecko to pull this in from the OS; that could be useful for HTML5 forms as well.

[1] http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/locales/en-US/chrome/global/datetimepicker.dtd
[2] http://mxr.mozilla.org/comm-central/source/calendar/locales/en-US/lightning-l10n.js#9

[Neal-o (neal-o)]

ha, that is funny!
I've been hunting through the UI and code base to find this info, and I literally just finished a build with preliminary support to pull in the calendar.week.start preference, after which I started it up to find a new email with your last comment.

I do think that getting this from the OS with and override from the app is correct.
To further complicate things, on OS X, I do believe it should be initially influenced by the Calendar app settings, then by thunderbird mechanisms.

Thanks for staying with me.

[Mkmelin+mozilla (mkmelin+mozilla)]

(In reply to Jim Porter (:squib) from comment #26)
> It might be useful to add
> support to Gecko to pull this in from the OS; that could be useful for HTML5 forms as well.

Absolutely! It's increasingly annoying to get wrong start of week. Bug 164495 for calendar.

[Philipp-bugzilla (philipp-bugzilla)]

Indeed, there is bug 164495 for calendar. What you want to fix first is bug 333938 though. This will allow Lightning/Thunderbird to access the locale info. I'm not sure the first day of the week is available on all platforms, but from skimming that bug it seems windows has an API call for it and linux has nl_localeinfo. This could be available on mac too.

You can of course check if the Lightning pref exists and is set to ease integration for Lightning. The better solution would be to default to checking the OS and making the code extensible enough so that Lightning can hook in and change the behavior. If you have time to do the Lightning patch too that would be super :)

[Neil-httl (neil-httl)]

Comment on attachment 8337339
fix bucket groups to be based off of a calendar week concept

Sorry for the delay. The code seems reasonable but this needs to have ui-review before I can review it properly.

>+ // TODO - in the calculation for thisWeek, the + 0 should be based of off
>+ // the localization for first day of calendar
If only it were that easy...

As a side note I'd like to compare to Firefox's History sidebar which uses the following groupings:
> Today
> Yesterday
> Last 7 days
> This month (if today is the 8th or later)
> 5 entries, one for each previous month
> Older than 6 months

Out of interest, Outlook seems to be use the following groupings:
> Today
> Yesterday
> 5 entries, one for each previous day
> Last Week (7-13 days ago)
> Two Weeks Ago (14-20 days ago)
> Three Weeks Ago (21-27 days ago)
> Earlier This Month
> Last Month
> Older
What I don't know is whether these groups are based on the day of the week or fixed numbers of days.

[Neal-o (neal-o)]

Created attachment 8355581
lastWeek2.diff

fix bucket groups to be based off of a calendar week concept.
pull firstDayOfWeek bias from lightning prefs if available.

[Jim (squibblyflabbetydoo)]

I think before we do much with this bug, we either need to fix bug 333938 or at least move the "calendar.week.start" pref to Thunderbird. If we do the latter, we should write some migration code so that we copy over the old pref value to the new pref.

[Neal-o (neal-o)]

This bug has stalled because, I seem not to be able to get any real direct help, without infinite referrals, for 333938.
So, I'll opt for and take a stab at moving the pref, and hopefully make some headway.

[Jim (squibblyflabbetydoo)]

Yeah, let's do the pref. That should be a reasonable workaround, and when bug 333938 is fixed, we can just switch over to that and everyone's happy.

[Jim (squibblyflabbetydoo)]

Comment on attachment 8355581
lastWeek2.diff

r- for now since we want to use a pref to handle the start-of-week.

[Zhangchaowang (zhangchaowang)]

Can this be pushed to upstream repository? I'm quite surprised to learn that it has not been addressed for almost 5 years.

(In reply to Jim Porter (:squib) from comment #35)
> Comment on attachment 8355581
> lastWeek2.diff
>
> r- for now since we want to use a pref to handle the start-of-week.

[Karl-rossing (karl-rossing)]

What we are seeing with TB 31.4.0 is this:

Today is Friday but "Last week" starts 7 days ago. That label should be "This Week".
Today is Friday but "Last 2 week" starts 14 days ago. That label should be "Last Week".

Willing to test fix when a fix is available.

[Paul White (paulw2u)]

Upstream bug closed "RESOLVED FIXED" on 2016-09-09
Display periods changed. "Last week" removed.
Fix targetted for Thunderbird 51
Tested ok using Ubuntu 18.04 and Thunderbird 60.2.1
Closing by marking "Fix released"