Skip to content
This repository was archived by the owner on Feb 29, 2020. It is now read-only.

fix(topsites): Combine frecency of the top pages of with/without www instead of all pages #1946

Merged
merged 1 commit into from
Jan 11, 2017
Merged

fix(topsites): Combine frecency of the top pages of with/without www instead of all pages #1946

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions addon/PlacesProvider.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,8 @@ Links.prototype = {

let blockedURLs = ignoreBlocked ? [] : this.blockedURLs.items().map(item => `"${item}"`);

// this query does "GROUP BY rev_nowww" (rev_host without www) to remove urls from same domain.
// GROUP first by rev_host to get the most-frecent page of an exact host
// then GROUP by rev_nowww to dedupe between top two pages of nowww host.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is also a piece of good tongue twister 🥇

// Note that unlike mysql, sqlite picks the last raw from groupby bucket.
// Which is why subselect orders frecency and last_visit_date backwards.
// In general the groupby behavior in the absence of aggregates is not
Expand All @@ -357,9 +358,9 @@ Links.prototype = {
let sqlQuery = `SELECT url, title, ${frecencyScore} frecency, guid, bookmarkGuid,
last_visit_date / 1000 as lastVisitDate, favicon, mimeType,
"history" as type
FROM
(
FROM (SELECT * FROM (
SELECT
rev_host,
CASE SUBSTR(rev_host, -5)
WHEN ".www." THEN SUBSTR(rev_host, -4, -999)
ELSE rev_host
Expand All @@ -379,8 +380,8 @@ Links.prototype = {
on moz_places.id = moz_bookmarks.fk
WHERE hidden = 0 AND last_visit_date NOTNULL
AND moz_places.url NOT IN (${blockedURLs})
ORDER BY rev_host, frecency, last_visit_date, moz_places.url DESC
)
ORDER BY frecency, last_visit_date, moz_places.url DESC
) GROUP BY rev_host)
GROUP BY rev_nowww
ORDER BY frecency DESC, lastVisitDate DESC, url
LIMIT :limit`;
Expand Down
9 changes: 9 additions & 0 deletions test/test-PlacesProvider.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,15 @@ exports.test_Links_getTopFrecentSites_dedupeWWW = function*(assert) {
links = yield provider.getTopFrecentSites();
assert.equal(links.length, 1, "adding both www. and no-www. yields one link");
assert.equal(links[0].frecency, 200, "frecency scores are combined");

// add another page visit with www and without www
testURI = NetUtil.newURI("http://mozilla.com/page");
yield PlacesTestUtils.addVisits(testURI);
testURI = NetUtil.newURI("http://www.mozilla.com/page");
yield PlacesTestUtils.addVisits(testURI);
links = yield provider.getTopFrecentSites();
assert.equal(links.length, 1, "adding both www. and no-www. yields one link");
assert.equal(links[0].frecency, 200, "frecency scores are combined ignoring extra pages");
};

exports.test_Links_getTopFrecentSites_limit = function*(assert) {
Expand Down