Bug 1404334 - Change NewTabUtils Bookmarks query to fetch bookmarks with no visits. r=Mardak draft
authorAndrei Oprea <andrei.br92@gmail.com>
Mon, 02 Oct 2017 16:15:44 -0400
changeset 675709 0dc719df7a991854fca84aaf4a92863e409e25f5
parent 674983 ee1c41cf306df0043a8e68af042f133acf2ef94e
child 734680 4d969f2bd9425c0fe1725d81195bb6bfcd7e9559
push id83211
push userbmo:andrei.br92@gmail.com
push dateThu, 05 Oct 2017 18:32:16 +0000
reviewersMardak
bugs1404334
milestone58.0a1
Bug 1404334 - Change NewTabUtils Bookmarks query to fetch bookmarks with no visits. r=Mardak MozReview-Commit-ID: 6RsVIsIbsBt
toolkit/modules/NewTabUtils.jsm
toolkit/modules/tests/xpcshell/test_NewTabUtils.js
--- a/toolkit/modules/NewTabUtils.jsm
+++ b/toolkit/modules/NewTabUtils.jsm
@@ -853,22 +853,21 @@ var ActivityStreamProvider = {
         SELECT id
         FROM moz_bookmarks p
         WHERE p.id = b.parent
           AND p.parent <> :tagsFolderId
       ) NOTNULL
     ) AS bookmarkGuid`,
 
   /**
-   * Shared WHERE expression filtering out undesired pages, e.g., hidden,
-   * unvisited, and non-http/s urls. Assumes moz_places is in FROM / JOIN.
+   * Shared WHERE expression filtering out undesired pages, e.g., hidden
+   * and non-http/s urls. Assumes moz_places is in FROM / JOIN.
    */
   _commonPlacesWhere: `
     AND hidden = 0
-    AND last_visit_date > 0
     AND (SUBSTR(url, 1, 6) == "https:"
       OR SUBSTR(url, 1, 5) == "http:")
   `,
 
   /**
    * Shared parameters for getting correct bookmarks and LIMITed queries.
    */
   _getCommonParams(aOptions, aParams = {}) {
@@ -1036,16 +1035,17 @@ var ActivityStreamProvider = {
         description,
         guid,
         preview_image_url,
         title,
         url
       FROM moz_places h
       WHERE description NOTNULL
         AND preview_image_url NOTNULL
+        AND last_visit_date > 0
         ${this._commonPlacesWhere}
       ORDER BY last_visit_date DESC
       LIMIT :limit
     `;
 
     return this._processHighlights(await this.executePlacesQuery(sqlQuery, {
       columns: this._highlightsColumns,
       params: this._getCommonParams(options)
@@ -1082,16 +1082,17 @@ var ActivityStreamProvider = {
         frecency,
         guid,
         last_visit_date / 1000 AS lastVisitDate,
         rev_host,
         title,
         url
       FROM moz_places h
       WHERE frecency >= :frecencyThreshold
+        AND last_visit_date > 0
         ${this._commonPlacesWhere}
       ORDER BY frecency DESC
       LIMIT :limit
     `;
 
     let links = await this.executePlacesQuery(sqlQuery, {
       columns: [
         "bookmarkGuid",
--- a/toolkit/modules/tests/xpcshell/test_NewTabUtils.js
+++ b/toolkit/modules/tests/xpcshell/test_NewTabUtils.js
@@ -406,24 +406,24 @@ add_task(async function getHighlights() 
       url: "https://mozilla1.com/nowNew"
     }
   ];
   for (let placeInfo of bookmarks) {
     await PlacesUtils.bookmarks.insert(placeInfo);
   }
 
   links = await provider.getHighlights();
-  Assert.equal(links.length, 0, "adding bookmarks without visits doesn't yield more links");
+  Assert.equal(links.length, 2, "bookmarks are returned even if not visited");
 
   // Add a history visit
   let testURI = "http://mozilla.com/";
   await PlacesTestUtils.addVisits(testURI);
 
   links = await provider.getHighlights();
-  Assert.equal(links.length, 0, "adding visits without metadata doesn't yield more links");
+  Assert.equal(links.length, 2, "adding visits without metadata doesn't yield more links");
 
   // Add bookmark visits
   for (let placeInfo of bookmarks) {
     await PlacesTestUtils.addVisits(placeInfo.url);
   }
 
   links = await provider.getHighlights();
   Assert.equal(links.length, 2, "adding visits to bookmarks yields more links");