384677 - for history sidebar and history menu only showing visits from past 30 days

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[(not reading, please use [e-mail] instead)]

for history sidebar and history menu, don't specify begin / end times

spun off from bug #384671

I claim this is a perf win, patch and details to follow.

Comment 1

[(not reading, please use [e-mail] instead)]

Attachment: patch

it's a (slight) perf win as we no longer SELECT the db with visit_date ranges, as in:

"... v.visit_date >= X AND v.visit_date <= Y ..."

additionally, in nsNavHistoryQueryResultNode::OnVisit(), we don't need to normalize the time anymore, but that's negligable.

Comment 2

[(not reading, please use [e-mail] instead)]

fixed.

cvs ci browser/base/content/browser-menubar.inc browser/components/places/conten
t/controller.js
Checking in browser/base/content/browser-menubar.inc;
/cvsroot/mozilla/browser/base/content/browser-menubar.inc,v  <--  browser-menuba
r.inc
new revision: 1.114; previous revision: 1.113
done
Checking in browser/components/places/content/controller.js;
/cvsroot/mozilla/browser/components/places/content/controller.js,v  <--  control
ler.js
new revision: 1.160; previous revision: 1.159
done

Comment 3

[(not reading, please use [e-mail] instead)]

history sidebar only showing visits from past 30 days.  the history menu had the same problem.

morphing this bug, as a result of findings in bug #385208

I noticed that the history sidebar was appearing much slower, and I thought it was a regression from bug #373944.  But, it turns out to be a regression from this bug.

ria wrote:  "By Last Visited was kind of broken (shows only the history of the last day) before 18 June".  I'm not seeing that, but I am seeing that we only show the history of the past 30 days.  (Ria, can you confirm?)

the fix for this bug, our history sidebar query was:

"place:beginTime=-2592000000000&beginTimeRef=1&endTime=7200000000&endTimeRef=2&type=1"

translated, that is:  "30 days before today until two hours from now"

After, it is:

place:type=1

So there is no time restriction.

"30 days before today" can yield a lot less results, especially for people like jay patel and schrep, who have monster histories.

A lot less results means we appear much faster than fx 2.

So now we show all the results.  But the performance problem with the places based history sidebar is very obvious.  I'll spin up a new bug about fixing that fx 2 regression.

Comment 4

[Ria Klaassen (not reading all bugmail)]

Could very well be true. I retested it with my default profile and now I see a lot more. The test profile is recreated at least once a day while it reimports a very old history file, hence my probably incorrect conclusion.
 

Comment 5

[(not reading, please use [e-mail] instead)]

> Could very well be true.

Thanks for checking, Ria.

Comment 6

[(not reading, please use [e-mail] instead)]

> I claim this is a perf win

note, it was not a perf win, unless your history only contained items < 30 days.

Comment 7

[Al Billings [:abillings - ex-MoCo]]

Some version of this seems present in Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a7pre) Gecko/200707110404 Minefield/3.0a7pre.

I have a history with 7 items in it. One of them is older than month. In the sidebar. The sidebar shows the 7 items, including the one older than a month.

The History menu shows only four items. The two lists (menu and sidebar) don't match. I'm attaching my places.sqlite.

Comment 8

[Al Billings [:abillings - ex-MoCo]]

Attachment: My places.sqlite.

Comment 9

[(not reading, please use [e-mail] instead)]

Al, I believe your places.sqlite demonstrates a bug, but not this one.

Eyeballing your places.sqlite file and trying it out in my debug build one issue I see is that one of your places (id=47, title="404: File Not Found", url="http://www.mozilla.org/projects/firefox/3.0a6pre/whatsnew/") has no visits, so it should not be showing up in the history sidebar or the menu, but it is.

How did you create that places.sqlite?  (by simple usage or by hand?)

I'd like to spin this off into a new bug.

Comment 10

[Al Billings [:abillings - ex-MoCo]]

Sweet. This was created by hand on 7/11 from a brand new FF3 profile. The 404 is one of the two default tabs that comes up the first time Minefield is run, as I recall.

So, if it is not incrementing a visit counter, that would be bad since it is getting the result from the server.

Comment 11

[(not reading, please use [e-mail] instead)]

> This was created by hand on 7/11 from a brand new FF3 profile.

"created by hand (using sqlite database browser)" vs "created by just using firefox".  Sounds like you did the latter?

> So, if it is not incrementing a visit counter, that would be bad since it is
> getting the result from the server.

I'm not sure what happened.  Did you clear your history?

Comment 12

[(not reading, please use [e-mail] instead)]

al, sorry for the delay in getting back to you.  what you were seeing is actually bug #375777.

from comment #10:  "The 404 is one of the two default tabs that comes up the first time Minefield is run, as I recall."

That is exactly the problem in bug #375777.  To summarize: any pages we load on startup do not have a referrer (a bookmark, a link, etc), so we were adding them as history visits without an type.

the history menu had code to deal with this, but not the sidebar.  (note, url bar autocomplete does not yet, but will soon, see bug #389491)

I've got a couple screen shots of your places.sqlite demonstrating the before / after of the history menu / sidebar with the patch in bug #375777.

but as for this bug, we are showing "old" history (before 30 days), so remarking fixed. (and it has been fixed since a6)

Comment 13

[(not reading, please use [e-mail] instead)]

as for showing old history, from ispiked's places.sqlite, I am seeing items from 1178766298499719 (aka:  Thu, 10 May 2007 03:04:58 GMT).

Comment 14

[(not reading, please use [e-mail] instead)]

Attachment: al's places.sqlite after the fix for bug #375777

Comment 15

[(not reading, please use [e-mail] instead)]

Attachment: al's places.sqlite before the fix for bug #375777

Comment 16

[Tracy Walker [:tracy]]

verified with: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre)
Gecko/2007080204 Minefield/3.0a7pre

Comment 17

[Tracy Walker [:tracy]]

not going to add this to Litmus as many perf tests are now being run automatically. Besides Litmus testing is based on reasonably fresh profiles.  So  30 days is not a simple thing to test with a fresh profile.

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