Closed Bug 406358 Opened 17 years ago Closed 17 years ago

Location bar drop down ignores frequency of visits

Categories

(Firefox :: Bookmarks & History, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 3 beta3

People

(Reporter: Mardak, Assigned: Mardak)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

Clicking the drop down only shows items that are most recently visited ignoring visit count. From the patch for bug 389491.. // nsNavHistory::AutoCompleteTypedSearch ... // visit count (primary) and time since last visited (secondary). ... + "ORDER BY v.visit_date DESC LIMIT "); Which is kinda interesting because bug 329118 changed the query from/to.. - "WHERE h.typed = 1 ORDER BY visit_count DESC LIMIT "); + "WHERE h.typed = 1 ORDER BY visit_date DESC LIMIT ");
Depends on: 406359
Attached patch v1 testcase (obsolete) (deleted) — Splinter Review
Testcase that passes after bug 406359 is fixed. Before the fix, "test 3" (4th test) is the one that fails out of all 8 because the results for empty search ignores the visit count.
Attachment #291049 - Flags: review?(dietrich)
Oh yeah, the note from the testcase.. Because the interactions among *DIFFERENT* visit counts and visit dates is not well defined, this test holds one of the two values constant when modifying the other. Right now, we do SQL ORDER BY sorting so it prefers the earlier sort over the latter. We might be doing something different for a global ranking, so I didn't want to require fixes to this testcase when future changes define the ordering of "count=2 visit=2 hours ago vs count=1 visit=1 hour ago" (first is older but has more visits.. latter has fewer visits but more recent)
i think this is fixed by bug 394038 landing. mardak is this testcase still valid for review?
Actually.. the testcase doesn't pass for some reason. It seems like my multiple addVisits to increase the visit_count doesn't make a difference.
Fyi, I tried changing the count (c1, c2) and the dates (d1, d2) just incase they happened to fall into the same frecency "level", but that didn't seem to make a difference. I looked at the other test cases and they don't seem to take into account visit_count. Am I increasing visit_count in the wrong way by doing multiple addVisits ?
Comment on attachment 291049 [details] [diff] [review] v1 testcase I'll hold off on this for now... but potentially there /is/ a bug somewhere because this testcase used to pass when I implemented a fix with bug 406359 back when it wasn't global frecency.
Attachment #291049 - Flags: review?(dietrich)
Attached patch v2 (deleted) — Splinter Review
2 issues from the earlier testcase: 1) Seems like I wasn't setting the visit count correctly.. multiple addVisit != setPageDetails with a visit count. 2) Dates I used didn't fall into different buckets.. 1 hour vs 3 hours ago. I switched the latter to 10 days ago.
Attachment #291049 - Attachment is obsolete: true
Attachment #300915 - Flags: review?(dietrich)
The testcase tests for both the dropdown and searching (and passes on the current trunk because it was fixed by bug 394038)
Checking in toolkit/components/places/tests/unit/test_frecency.js; /cvsroot/mozilla/toolkit/components/places/tests/unit/test_frecency.js,v <-- test_frecency.js initial revision: 1.1 done
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 beta3
Comment on attachment 300915 [details] [diff] [review] v2 Checked on IRC and it was okay to land the testcase.
Attachment #300915 - Flags: review?(dietrich)
This will be useful to make sure the fix for bug 406359 doesn't break things.
No longer depends on: 406359
Flags: in-testsuite+
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.

Attachment

General

Creator:
Created:
Updated:
Size: