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>
Fri, 15 Sep 2017 10:46:23 +0200
changeset 665319 1cc9d9f4dfd4dbf156751ae0e0f134ae084b32d1
parent 665318 f4bad99b6b30f404c5ff8d0a3cf754b512fea99b
child 665320 75022844588c93caf00666572d205ff89e448f5a
push id80016
push usermdeboer@mozilla.com
push dateFri, 15 Sep 2017 09:13:48 +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");