Bug 1354536 - Part 2 - Add an option to ActivityStreamLinks.getHighlights() to allow results to be returned including favicons. r?Mardak draft
authorMike de Boer <mdeboer@mozilla.com>
Tue, 19 Sep 2017 16:17:09 +0200
changeset 666985 b7c236afd29f9422bb08383ada8e0391853fc4c8
parent 666884 e4261f5b96ebfd63e7cb8af3035ff9fea90c74a5
child 666986 14126eaa8e19efdc479c745486bfba33d78b81e7
push id80579
push usermdeboer@mozilla.com
push dateTue, 19 Sep 2017 14:22:45 +0000
reviewersMardak
bugs1354536
milestone57.0a1
Bug 1354536 - Part 2 - Add an option to ActivityStreamLinks.getHighlights() to allow results to be returned including favicons. r?Mardak MozReview-Commit-ID: 5NCTCtooScy
toolkit/modules/NewTabUtils.jsm
toolkit/modules/tests/xpcshell/test_NewTabUtils.js
--- a/toolkit/modules/NewTabUtils.jsm
+++ b/toolkit/modules/NewTabUtils.jsm
@@ -1291,16 +1291,17 @@ var ActivityStreamLinks = {
   },
 
   /**
    * Get the Highlights links to show on Activity Stream
    *
    * @param {Object} aOptions
    *   {bool} excludeBookmarks: Don't add bookmark items.
    *   {bool} excludeHistory: Don't add history items.
+   *   {bool} withFavicons: Add favicon data: URIs, when possible.
    *   {int}  numItems: Maximum number of (bookmark or history) items to return.
    *
    * @return {Promise} Returns a promise with the array of links as the payload
    */
   async getHighlights(aOptions = {}) {
     aOptions.numItems = aOptions.numItems || ACTIVITY_STREAM_DEFAULT_LIMIT;
     const results = [];
 
@@ -1323,16 +1324,21 @@ var ActivityStreamLinks = {
           // Stop adding pages once we reach the desired maximum
           if (results.length === aOptions.numItems) {
             break;
           }
         }
       }
     }
 
+    if (aOptions.withFavicons) {
+      return ActivityStreamProvider._faviconBytesToDataURI(
+        await ActivityStreamProvider._addFavicons(results));
+    }
+
     return results;
   },
 
   /**
    * Get the top sites to show on Activity Stream
    *
    * @return {Promise} Returns a promise with the array of links as the payload
    */
--- a/toolkit/modules/tests/xpcshell/test_NewTabUtils.js
+++ b/toolkit/modules/tests/xpcshell/test_NewTabUtils.js
@@ -473,16 +473,25 @@ add_task(async function getHighlights() 
   });
 
   links = await provider.getHighlights();
   Assert.equal(links.length, 3, "a visited bookmark doesn't appear as bookmark and history");
   Assert.equal(links[0].url, testURI, "history is now the first, i.e., most recent, bookmark");
   Assert.equal(links[0].type, "bookmark", "was history now bookmark");
   Assert.equal(links[1].url, bookmarks[1].url, "still have younger bookmark now second");
   Assert.equal(links[2].url, bookmarks[0].url, "still have older bookmark now third");
+
+  // Test the `withFavicons` option.
+  links = await provider.getHighlights({ withFavicons: true });
+  Assert.equal(links.length, 3, "We're not expecting a change in links");
+  // We don't have the favicon service all bootstrapped and that's unnecessary
+  // for this unit.
+  Assert.ok("favicon" in links[0], "Link 1 should contain a favicon");
+  Assert.ok("favicon" in links[1], "Link 2 should contain a favicon");
+  Assert.ok("favicon" in links[2], "Link 3 should contain a favicon");
 });
 
 add_task(async function getTopFrecentSites() {
   await setUpActivityStreamTest();
 
   let provider = NewTabUtils.activityStreamLinks;
   let links = await provider.getTopSites({topsiteFrecency: 100});
   Assert.equal(links.length, 0, "empty history yields empty links");