293742 - incorrect grouping of message in group view & incorrect day selection if the short date display option is selected

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Petres, Zoltan]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.7) Gecko/20050414 Firefox/1.0.3
Build Identifier: version 1.0.2 (20050317)

All the dates of the messages are displayed in local time, but Thunderbird
calculates the time according to GMT. If someone stays in a country where the
time difference is big (eg. Japan is GMT+0900) then it can happen that some
messages time is shown as yesterday, but grouped is the last week section.

The same error is in the date show case, if we select that we want to see the
day and the time (eg. Thu 12.20 PM) instead of the full date, then the end of
the week calculated inproperly.

When we calculate the last midnight in sec, last week's midnight in sec etc.
first we have to modify the received GMT time to the actual time zone.

Reproducible: Always

Steps to Reproduce:
Get messages, select grouping. If you are lucky, then you can see. See pic attached.
Actual Results:  
Incorrect grouping. The third message is in wrong place.

Expected Results:  
Correct grouping. Third message is in yesterday section.

Comment 1

[Petres, Zoltan]

Attachment: Bad grouping example

Comment 2

[Petres, Zoltan]

Attachment: Patch for the Short Date for the last week problem

Comment 3

[Petres, Zoltan]

Attachment: Patch for the GroupView problem

Comment 4

[David :Bienvenu]

thx a lot for looking into this - can you attach -uw diffs so I can see the context?

Comment 5

[Petres, Zoltan]

Attachment: Patch for the Short Date for the last week problem (more verbose)

Comment 6

[Petres, Zoltan]

Attachment: Patch for the GroupView problem (more verbose)

Comment 7

[David :Bienvenu]

Comment on attachment 183288 [details] [diff] [review]
Patch for the Short Date for the last week problem (more verbose)

looks right to me. I'll run with this patch.

Comment 8

[David :Bienvenu]

Comment on attachment 183290 [details] [diff] [review]
Patch for the GroupView problem (more verbose)

oops, I believe we need to use the LL macros here, and in the previous patch.

Comment 9

[David :Bienvenu]

Comment on attachment 183288 [details] [diff] [review]
Patch for the Short Date for the last week problem (more verbose)

minusing - need the LL macros here too (or use nsInt64) Do you want to try
this, Zolton, or do you want me to?

Comment 10

[Petres, Zoltan]

(In reply to comment #9)
> (From update of attachment 183288 [details] [diff] [review] [edit])
> minusing - need the LL macros here too (or use nsInt64) Do you want to try
> this, Zolton, or do you want me to?
> 

Well. I am not really familiar with Mozilla's LL macros. I've recognized that
the two file uses different guidelines, DBView only use those, GroupView does
not. If you can hack my code to conform to the guidelines, I would really
appreciate.

Zoltan

Comment 11

[[e-mail]]

Comment on attachment 183288 [details] [diff] [review]
Patch for the Short Date for the last week problem (more verbose)

You also need to allow for the dst_offset. I'm in BST so my gmt_offset is zero
but between midnight and 01:00 I see 7-day-old messages using today's day.

Comment 12

[Petres, Zoltan]

(In reply to comment #11)
> (From update of attachment 183288 [details] [diff] [review] [edit])
> You also need to allow for the dst_offset. I'm in BST so my gmt_offset is zero
> but between midnight and 01:00 I see 7-day-old messages using today's day.
> 

I agree. I forgot to take account of daylight saving, but it is true, we have
add this too to get the real local time.

David, please, add the tp_dst_offset to the message and current time, when you
update the diff to makei it Mozilla conform.

Zoltan

Comment 13

[Mike Cowperthwaite]

Might as well confirm this, as there's a patch in progress.  I believe there's 
another bug somewhere that mentions this same symptom.

Comment 14

[Petres, Zoltan]

(In reply to comment #13)
> Might as well confirm this, as there's a patch in progress.  I believe there's 
> another bug somewhere that mentions this same symptom.

I've found the bug 263678. It mentions this problem, but others too I have not
experienced. I guess those should be reconsidered after this patch.

Comment 15

[Petres, Zoltan]

Attachment: Rewritten patch according to reviewer's comment

I rewritten the code with LL macros.

Comment 16

[David :Bienvenu]

Comment on attachment 183587 [details] [diff] [review]
Rewritten patch according to reviewer's comment

thx for using the macros. I wish we didn't have to use them. - using nsInt64
might make the code easier to read but I can worry about that some other time.

Comment 17

[[e-mail]]

Comment on attachment 183587 [details] [diff] [review]
Rewritten patch according to reviewer's comment

sr=me if you change the tabs back into spaces.

Comment 18

[Petres, Zoltan]

Attachment: Rewritten patch according to reviewer's comment #2

Tab problems corrected

Comment 19

[Petres, Zoltan]

Can someone commit the patch? I does not have the rights.

Comment 20

[Mike Shaver (:shaver emeritus)]

Comment on attachment 183821 [details] [diff] [review]
Rewritten patch according to reviewer's comment #2

a=shaver

Comment 21

[David :Bienvenu]

fixed on trunk, will be in 1.1a2, thx, Zoltan.

Comment 22

[David :Bienvenu]

Comment on attachment 183290 [details] [diff] [review]
Patch for the GroupView problem (more verbose)

clearing old request