test_treeview_date.xul fails at certain times

RESOLVED FIXED in Firefox 3.6a1

Status

()

RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: mak, Assigned: mak)

Tracking

({fixed1.9.1})

Trunk
Firefox 3.6a1
fixed1.9.1
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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

Updated

10 years ago
Blocks: 435322
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.
(Assignee)

Comment 3

10 years ago
Created attachment 365809 [details] [diff] [review]
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.
Assignee: nobody → mak77
Status: NEW → ASSIGNED
(Assignee)

Comment 4

10 years ago
Created attachment 365863 [details] [diff] [review]
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.
Attachment #365809 - Attachment is obsolete: true
Attachment #365863 - Flags: review?(dietrich)

Comment 5

10 years ago
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 ;-)
(Assignee)

Comment 6

10 years ago
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?
> 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.
(Assignee)

Comment 8

10 years ago
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?
Flags: in-testsuite?
> 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?
(Assignee)

Comment 10

10 years ago
(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.
Attachment #365863 - Flags: review?(dietrich) → review+
Comment on attachment 365863 [details] [diff] [review]
patch v1.1

r=me, thanks
(Assignee)

Comment 12

10 years ago
http://hg.mozilla.org/mozilla-central/rev/def7e16617e4
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Flags: in-testsuite? → in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.2a1
Comment on attachment 365863 [details] [diff] [review]
patch v1.1

a191=beltzner
Attachment #365863 - Flags: approval1.9.1+
(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 ;-)
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.
btw, yeah, minor but annoying, you're right.
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
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.