481765 - test_treeview_date.xul fails at certain times

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Marco Bonardo [:mak]]

i'm going to disable this test, till i can get a reason for it to fail at certain times.

Comment 1

[Marco Bonardo [:mak]]

disabled with:
http://hg.mozilla.org/mozilla-central/rev/2a03f4a992fa

Comment 2

[Boris Zbarsky [:bzbarsky]]

It might be nice to put more information in the error messages... would help see what's going on better.

I've certainly had this fail consistently for me recently.

Comment 3

[Marco Bonardo [:mak]]

Attachment: patch

this is finally working as intended in both my timezone and boris's one.

But before asking for a review i'll do a complete cycle of all timezones locally and check it works as intendend for all of them.

Comment 4

[Marco Bonardo [:mak]]

Attachment: patch v1.1

Tested with all timezones, moved to a DST date, and retested against all timezones.
This fixes a date calculation issue and reneables the test, based on boris comments i've changes ok checks to is, so both strings are printed in case of failures. And cleaned up the created bookmark.

Comment 5

[[e-mail]]

Looks like the 3.0 code that suite copied might fail on the 25th hour of the day
the clocks go back - at 11:45 the date might show on history for 00:15 ;-)

Comment 6

[Marco Bonardo [:mak]]

this code is not based on 3.0 though, is unique for 3.2 trunk, btw, i don't clearly understand what do you mean by the 25th hour :\
Do you think this code could be affected?

Comment 7

[Boris Zbarsky [:bzbarsky]]

> i don't clearly understand what do you mean by the 25th hour :\

The day when the switch from DST to standard time happens has 25 hours in it.  The day when the switch the other way happens has 23 hours in it.

Comment 8

[Marco Bonardo [:mak]]

mh if i'm not wrong the time does not even changes at midnight in such a case, but the hour at which the time is changed varies from year to year... Am i wrong? That sounds like a minor case though, to print out the time we use getHours, so shouldn't it be correct for the current localTime?

Comment 9

[Boris Zbarsky [:bzbarsky]]

> the time does not even changes at midnight in such a case

Correct.  I'm not sure whether this matters for your calculations, though: you're assuming 86400000 ms in a day, which is only true for most days.  So your midnight will be off by an hour for half the year, no?  Does that matter?

If what you're really trying to find is a Date() object representing today's midnight in localtime, why not just explicitly create one?

  var midnight = new Date();
  midnight.setHours(0);
  midnight.setMinutes(0);
  midnight.setSeconds(0);
  midnight.setMilliseconds(0);

or some such?

Comment 10

[Marco Bonardo [:mak]]

(In reply to comment #9)
> Correct.  I'm not sure whether this matters for your calculations, though:
> you're assuming 86400000 ms in a day, which is only true for most days.

consider that the calculation is based on a corrected "now" based on timezoneOffset, and is a diff... i tested it during DST moving on my date to a DST one, and i did not notice any issue.

> If what you're really trying to find is a Date() object representing today's
> midnight in localtime, why not just explicitly create one?
> 
>   var midnight = new Date();
>   midnight.setHours(0);
>   midnight.setMinutes(0);
>   midnight.setSeconds(0);
>   midnight.setMilliseconds(0);

because it will be slower than this couple of maths, we were doing that before, but with some sort of caching, it's a much more verbose code.

Comment 11

[Dietrich Ayala (:dietrich)]

Comment on attachment 365863 [details] [diff] [review]
patch v1.1

r=me, thanks

Comment 12

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/def7e16617e4

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 365863 [details] [diff] [review]
patch v1.1

a191=beltzner

Comment 14

[Marco Bonardo [:mak]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/261e7310cf11

Comment 15

[[e-mail]]

(In reply to comment #10)
> consider that the calculation is based on a corrected "now" based on
> timezoneOffset, and is a diff... i tested it during DST moving on my date to a
> DST one, and i did not notice any issue.
But you failed to test it during DST moving to a non-DST one, bug 525739 ;-)

Comment 16

[Marco Bonardo [:mak]]

not exactly, looks like there is some disalignment during the hour around DST movement, indeed now it's working correctly again. Also sounds like the time is correct but the "sameday" detection in the treeview code is not working around that time.

Comment 17

[Marco Bonardo [:mak]]

btw, yeah, minor but annoying, you're right.

Comment 18

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv