435322 - Places History Visit Date doesn't show _date_ portion properly

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Tim Riley [:timr]]

Attachment: Screen snap of the History/Show All History window with missing date values for yeasterday evening

For time/dates that are less then 24 hours ago but earlier than midnight, the Visit Date column only shows the time.  For most apps that show dates and times, they normally show the time and _date_ portion if the time/date are earlier than midnight.

Give that the time is say.... 3:30pm...
Expected date format for a sequence bookmarks:
     5:26 PM
     3:42 PM
     9:32 AM
     5/21/08 7:17 PM
     5/21/08 4:35 PM
     5/21/08 8:00 AM

In the nightlies it actually shows up like this:

     5:26 PM
     3:42 PM
     9:32 AM
     7:17 PM
     4:35 PM
     5/21/08 8:00 AM

STR:
1) Go to the History Menu -> Show All History
2) Add the Visit Date column to the columnar display
3) Look at Visit Dates that are before midnight, but < 24 hours ago.  Like if the current time is 3:30pm, look at times that are between 3:35 and midnight yesterday.  They are missing the date value.

My config:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008052104 Minefield/3.0pre

Comment 1

[jjerison]

Can't you find the author of this bug and assign it to that person?  He or she should be able to fix it easily.  If it's more complicated than it looks to fix it, a second-best alternative would be to show all times with the date.

Comment 2

[jjerison]

I should have said the author of the routine--I didn't mean any disrespect, this is a great program and it's not surprising that it has a few glitches.  I'm just saying that the author should be able to fix the problem easily while anyone else would have do a lot of work.

Comment 3

[David Gasaway]

Confirmed on Windows XP

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1

Comment 8

[Igor Velkov]

 Bug 470543 contain fix of this bug for seamonkey

Comment 9

[Marco Bonardo [:mak]]

Attachment: patch

thanks for reporting Seamonkey bug

Comment 10

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/9cc88fcba152

Comment 11

[Marco Bonardo [:mak]]

Comment on attachment 356195 [details] [diff] [review]
patch

asking approval to fix wrong formatted dates, no risk

Comment 12

[Igor Velkov]

requesting for reopening, regression. Bug was fixed, but now it on place
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090220 Minefield/3.2a1pre

Comment 13

[Marco Bonardo [:mak]]

Igor, could you better explain what you mean please?

Comment 14

[Igor Velkov]

Now there is 21 feb 2009, 1:36.
I open history, sorted by date, desc, find for place, where time start format as date time
.
I see:
2:02
2:00
20.02.2009 1:59
20.02.2009 1:55

the border should be as
00:01
20.02.2009 23:59

Comment 15

[Marco Bonardo [:mak]]

could be a timezone issue, what's you timezone based on GMT?

Comment 16

[Igor Velkov]

GMT+2, or EET

Comment 17

[Marco Bonardo [:mak]]

yeah that's the issue, timezone offset

Comment 18

[Marco Bonardo [:mak]]

So internally we use PR_Now that is number of milliseconds based on GMT(UTC), while Date.now() is number of milliseconds based on current timezone.

Actually a part this minor issue that i can patch, our interfaces take PRTime but do not specify that the passed in time should be UTC based, so we could end up comparing visit times the wrong way. i'll file a bug on this.

Comment 19

[Marco Bonardo [:mak]]

mh, documentation says date.now() is milliseconds from UTC, still the issue is due to the timezone...

Comment 20

[Marco Bonardo [:mak]]

ok, midnight calculation should be done not based on UTC but on local time.

Comment 21

[Marco Bonardo [:mak]]

Attachment: additional patch for timezone

this should fix it, locally (GMT+1) i see 23:59 with date, 00:00 and on without, while before the threshold was at 1:00am. i tried moving the date to late April to take DST in count, and that's still correct.

Comment 22

[Dietrich Ayala (:dietrich)]

Comment on attachment 363469 [details] [diff] [review]
additional patch for timezone

looks fine, r=me. possible to get a test for this?

Comment 23

[Igor Velkov]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090224 Minefield/3.2a1pre

when I reopen bug, error was 26 hour; now it is 2 hour:
now I have:

at 4:10, 25 feb:
2:02
25.02.2009 1:59

should be at 00:00, as at Comment #14
my timezone is europe/riga.
check my current wall-clock time (4:14), and server timestamp of this message

Comment 24

[Marco Bonardo [:mak]]

Attachment: test wip v0.1

we can add a chrome test creating a places tree, this is only a first mockup of it.

Comment 25

[Marco Bonardo [:mak]]

Attachment: additional patch for timezone, with test

here is the patch with a chrome test, i used a Makefile originally written by Drew for another patch, so don't care at the contributor reporting him :)

Comment 26

[Dietrich Ayala (:dietrich)]

Comment on attachment 364073 [details] [diff] [review]
additional patch for timezone, with test


>+            case "date":
>+              var timeObj = new Date(node.time / 1000);
>+              var dateFormat =(node.uri == "http://at.midnight.com/" ||
>+                               node.uri == "http://after.midnight.com/") ?
>+                Ci.nsIScriptableDateFormat.dateFormatNone :
>+                Ci.nsIScriptableDateFormat.dateFormatShort;

nit: space after =

actually, this is just painful to read, use if/else.

r=me

Comment 27

[Marco Bonardo [:mak]]

Attachment: additional patch for timezone, with test

addressed comment

Comment 28

[jjerison]

Attachment to #27 is empty

Comment 29

[Marco Bonardo [:mak]]

Attachment: additional patch for timezone, with test

ops, thanks

Comment 30

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/480d7c7f5c38

Comment 31

[Marco Bonardo [:mak]]

pushed additional changeset:
http://hg.mozilla.org/mozilla-central/rev/e532985b7482

chrome test was adding a "redirecting..." visit to the tree, causing random orange, so in case the uri is not one of those i want to test i skip the test.

Comment 32

[Marco Bonardo [:mak]]

i have disabled the test because has started to fail at a certain time, filed Bug 481765 to follow that.

Comment 33

[Marco Bonardo [:mak]]

regression is fixed, if we don't see any failure in the next 24 hours (and Bug 481765 is not reopened), i would say this is wanted for 3.1, old behavior gives users a really bad sense of "unpolished".

Comment 34

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Igor, can you please check with the latest nightly build of Firefox 3.6a1 if it is fixed for you now? That would be great. Thanks.

Comment 35

[Igor Velkov]

Yes, now it's fine for me, thank you.

Comment 36

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

Comment on attachment 356195 [details] [diff] [review]
patch

a191=beltzner

Comment 37

[Marco Bonardo [:mak]]

Attachment: merged patch for 1.9.1

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/46406627a6df

Comment 38

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #35)
> Yes, now it's fine for me, thank you.

Marking verified fixed on trunk based on comment 35. Igor, could you also check a latest Shiretoko build? That would be great. Thanks!

Comment 39

[Igor Velkov]

Now with shiretoko and minefield I see history grouped, and it's a shame for me. How can I swich old plain history view back?
Should I fill another bug about the missing plain history view?

Checked
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090423 Minefield/3.6a1pre
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b4pre) Gecko/20090422 Shiretoko/3.5b4pre

Comment 40

[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