Closed Bug 630225 Opened 10 years ago Closed 10 years ago
Expose frecency as a sorting order for the history sidebar (with slight query builder optimization).
Most of the analysis is in bug 365992. I'll just report what this bug can improve and what it cannot improve. While this is not a regression from FX3.x, it's the second most common complain I can find regarding history (first one is history loss), and is in a release we based on speed improvements and ui responsitivity. improves: - most visited and last visited are instant (limited to 200 results) - searching terms with lots of results is much faster (limited to 100 results) - the matching algorithm is the same as the location bar (code reuse, safe, better matching) - searches based on terms are more useful (sorted by frecency rather than by title) - makes the move to async containers easier (less post-filtering) WILL NOT improve: - searches returning few results or no results at all (and their live-update) use-case scenarios: - using history sidebar (by most/last visited) is a pain - leaving history sidebar open on browser close sums up the search time to the next startup time (Ts increases by seconds) - searching history in Library or sidebar is impossible, especially for slow-typers - User leaves a history search open in a background window and does not associate browser hangs to something he doesn't see, but directly to the browser. Risk is somehow limited by the small amount of changes and code reuse, clearly there is some risk, but we don't plan further changes to this pieces of code so it will stay easily backed-out-able.
will do another trybuild from this.
Attachment #508433 - Flags: review?(dietrich)
Comment on attachment 508433 [details] [diff] [review] patch v1.0 r+ on the code changes. however, the frecency result sorting needs a test. i'll r+ with that done.
Attachment #508433 - Flags: review?(dietrich) → review-
Comment on attachment 508603 [details] [diff] [review] patch v1.1 r=me. I'm fully behind limiting the search results so that we don't hang the browser for 12 seconds, but given the late stage of the release, get approval from Johnath or Beltzner, so we get another set of eyes on this change before landing it.
Attachment #508603 - Flags: review?(dietrich) → review+
Comment on attachment 508603 [details] [diff] [review] patch v1.1 OK, asking approval, risk and gains are described in comment 0, the change mitigates the most common cases of a longstanding issue that in certain situations can make the user think the browser has awful performances (without actually blaming the right UI piece). It also tries to be conservative in the changes.
Attachment #508603 - Flags: approval2.0?
Will this limit affect both the library and the sidebar? I sometimes manually delete entries from the history when I feel like a lot of useless them are useless and clutter the results or when I feel the urlbar is getting slower its getting slower. When I do this I usually search for a domain, sort by visitation number and delete the ones that have 1 (or 0...) visitations and are older than a few weeks or months. And these results can be in the number of hundreds or thousands. If its limited to 100 I wouldn't be able to do this.
(In reply to comment #7) > Will this limit affect both the library and the sidebar? yes > I sometimes manually delete entries from the history when I feel like a lot of > useless them are useless and clutter the results or when I feel the urlbar is > getting slower its getting slower. When I do this I usually search for a > domain, sort by visitation number and delete the ones that have 1 (or 0...) > visitations and are older than a few weeks or months. And these results can be > in the number of hundreds or thousands. If its limited to 100 I wouldn't be > able to do this. you can delete domains in the sidebar view "by site". or you can delete the "older than 6 months" container in the view "by date". you can search multiple times too, actually with this limit you would take much less time doing so, since with your usual way you have to wait seconds at each refresh, while with the change even if for your specific use case requires multiple searches they will be much faster. Btw, you don't need to to this manual cleanup, we expire entries when we reach our natural limit, if you think the limit is too large you can reduce it (see http://blog.bonardo.net/2010/01/20/places-got-async-expiration)
Comment on attachment 508603 [details] [diff] [review] patch v1.1 Too much changing here to be comfortable about taking this at this point
Attachment #508603 - Flags: approval2.0? → approval2.0-
please don't push this, I want to revise it a bit since there is more time available to make it more awesome.
I'm slightly reducing scope of the bug, I'm removing from the patch all the numeric limits. The limited views were a solution I was mostly suggesting for FF4, but now that pressure is solved and I only want to take the good part of the patch without introducing behavioral differences. I actually got new ideas to improve performances in the meanwhile. Well, there is still one difference, the sidebar, when not sorted by last or most visited, will be sorted by frecency, that should be a better ordering to find stuff, it will act like an infinite awesomebar (with some less awesomeness though). From various testing the limit was also effective only in some case, while when searching for rare terms we were still going the slow path. This patch goes toward the right direction allowing to reduce some of the post-filtering work, thus pushing it since the changes are not additive from the r+.
Attachment #508603 - Attachment is obsolete: true
Summary: Limit history searches in the UI to a reasonable amount → Expose frecency as a sorting order for the history sidebar (with slight query builder optimization).
Whiteboard: not-ready → [not-ready for cedar][fixed-in-places]
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
dev-doc-needed for the additional SORT_BY_FRECENCY_* options
Documented: https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsINavHistoryQueryOptions#Sorting_methods Also fixed a couple of the other constants, which had the wrong numbers, and removed two that apparently were documented at one point then removed before shipping. Also mentioned on Firefox 6 for developers.
You need to log in before you can comment on or make changes to this bug.