Bug 1425496 - Add Recently Pocketed Items to Highlights
MozReview-Commit-ID: LyXKkQkPXte
--- a/browser/components/customizableui/content/panelUI.js
+++ b/browser/components/customizableui/content/panelUI.js
@@ -518,17 +518,18 @@ const PanelUI = {
* Fetches the list of Recent Highlights and replaces the items in the Library
* view with the results.
*/
async fetchAndPopulateLibraryRecentHighlights() {
let highlights = await NewTabUtils.activityStreamLinks.getHighlights({
// As per bug 1402023, hard-coded limit, until Activity Stream develops a
// richer list.
numItems: 6,
- withFavicons: true
+ withFavicons: true,
+ excludePocket: true
}).catch(ex => {
// Just hide the section if we can't retrieve the items from the database.
Cu.reportError(ex);
return [];
});
// Since the call above is asynchronous, the panel may be already hidden
// at this point, but we still prepare the items for the next time the
--- a/browser/extensions/activity-stream/lib/HighlightsFeed.jsm
+++ b/browser/extensions/activity-stream/lib/HighlightsFeed.jsm
@@ -82,17 +82,18 @@ this.HighlightsFeed = class HighlightsFe
// We broadcast when we want to force an update, so get fresh links
if (options.broadcast) {
this.linksCache.expire();
}
// Request more than the expected length to allow for items being removed by
// deduping against Top Sites or multiple history from the same domain, etc.
- const manyPages = await this.linksCache.request({numItems: MANY_EXTRA_LENGTH});
+ // Until bug 1425496 lands, do not include saved Pocket items in highlights
+ const manyPages = await this.linksCache.request({numItems: MANY_EXTRA_LENGTH, excludePocket: true});
// Remove adult highlights if we need to
const checkedAdult = this.store.getState().Prefs.values.filterAdult ?
filterAdult(manyPages) : manyPages;
// Remove any Highlights that are in Top Sites already
const [, deduped] = this.dedupe.group(this.store.getState().TopSites.rows, checkedAdult);
--- a/toolkit/modules/NewTabUtils.jsm
+++ b/toolkit/modules/NewTabUtils.jsm
@@ -18,16 +18,19 @@ ChromeUtils.defineModuleGetter(this, "Pl
"resource://gre/modules/PlacesUtils.jsm");
ChromeUtils.defineModuleGetter(this, "PageThumbs",
"resource://gre/modules/PageThumbs.jsm");
ChromeUtils.defineModuleGetter(this, "BinarySearch",
"resource://gre/modules/BinarySearch.jsm");
+ChromeUtils.defineModuleGetter(this, "pktApi",
+ "chrome://pocket/content/pktApi.jsm");
+
XPCOMUtils.defineLazyGetter(this, "gCryptoHash", function() {
return Cc["@mozilla.org/security/hash;1"].createInstance(Ci.nsICryptoHash);
});
XPCOMUtils.defineLazyGetter(this, "gUnicodeConverter", function() {
let converter = Cc["@mozilla.org/intl/scriptableunicodeconverter"]
.createInstance(Ci.nsIScriptableUnicodeConverter);
converter.charset = "utf8";
@@ -950,16 +953,21 @@ var ActivityStreamProvider = {
* length, and favicon size (width)
*/
_addFavicons(aLinks) {
// Each link in the array needs a favicon for it's page - so we fire off a
// promise for each link to compute the favicon data and attach it back to
// the original link object. We must wait until all favicons for the array
// of links are computed before returning
return Promise.all(aLinks.map(link => new Promise(async resolve => {
+ // Never add favicon data for pocket items
+ if (link.type === "pocket") {
+ resolve(link);
+ return;
+ }
let iconData;
try {
const linkUri = Services.io.newURI(link.url);
iconData = await this._getIconData(linkUri);
// Switch the scheme to try again with the other
if (!iconData) {
linkUri.scheme = linkUri.scheme === "https" ? "http" : "https";
@@ -970,16 +978,77 @@ var ActivityStreamProvider = {
}
// Add the icon data to the link if we have any
resolve(Object.assign(link, iconData || {}));
})));
},
/**
+ * Helper function which makes the call to the Pocket API to fetch the user's
+ * saved Pocket items.
+ */
+ fetchSavedPocketItems(requestData) {
+ if (!pktApi.isUserLoggedIn()) {
+ return Promise.resolve(null);
+ }
+ return new Promise((resolve, reject) => {
+ pktApi.retrieve(requestData, {
+ success(data) {
+ resolve(data);
+ },
+ error(error) {
+ reject(error);
+ }
+ });
+ });
+ },
+
+ /**
+ * Get the most recently Pocket-ed items from a user's Pocket list. See:
+ * https://getpocket.com/developer/docs/v3/retrieve for details
+ *
+ * @param {Object} aOptions
+ * {int} numItems: The max number of pocket items to fetch
+ */
+ async getRecentlyPocketed(aOptions) {
+ const pocketSecondsAgo = Math.floor(Date.now() / 1000) - ACTIVITY_STREAM_DEFAULT_RECENT;
+ const requestData = {
+ detailType: "complete",
+ count: aOptions.numItems,
+ since: pocketSecondsAgo
+ };
+ let data;
+ try {
+ data = await this.fetchSavedPocketItems(requestData);
+ if (!data) {
+ return [];
+ }
+ } catch (e) {
+ Cu.reportError(e);
+ return [];
+ }
+ /* Extract relevant parts needed to show this card as a highlight:
+ * url, preview image, title, description, and the unique item_id
+ * necessary for Pocket to identify the item
+ */
+ let items = Object.values(data.list)
+ // status "0" means not archived or deleted
+ .filter(item => item.status === "0")
+ .map(item => ({
+ description: item.excerpt,
+ preview_image_url: item.has_image === "1" && item.image.src,
+ title: item.resolved_title,
+ url: item.resolved_url,
+ item_id: item.item_id
+ }));
+ return this._processHighlights(items, aOptions, "pocket");
+ },
+
+ /**
* Get most-recently-created visited bookmarks for Activity Stream.
*
* @param {Object} aOptions
* {num} bookmarkSecondsAgo: Maximum age of added bookmark.
* {bool} ignoreBlocked: Do not filter out blocked links.
* {int} numItems: Maximum number of items to return.
*/
async getRecentBookmarks(aOptions) {
@@ -1291,30 +1360,36 @@ 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} excludePocket: Don't add Pocket 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 = [];
// First get bookmarks if we want them
if (!aOptions.excludeBookmarks) {
results.push(...await ActivityStreamProvider.getRecentBookmarks(aOptions));
}
+ // Add the Pocket items if we need more and want them
+ if (aOptions.numItems - results.length > 0 && !aOptions.excludePocket) {
+ results.push(...await ActivityStreamProvider.getRecentlyPocketed(aOptions));
+ }
+
// Add in history if we need more and want them
if (aOptions.numItems - results.length > 0 && !aOptions.excludeHistory) {
// Use the same numItems as bookmarks above in case we remove duplicates
const history = await ActivityStreamProvider.getRecentHistory(aOptions);
// Only include a url once in the result preferring the bookmark
const bookmarkUrls = new Set(results.map(({url}) => url));
for (const page of history) {
--- a/toolkit/modules/tests/xpcshell/test_NewTabUtils.js
+++ b/toolkit/modules/tests/xpcshell/test_NewTabUtils.js
@@ -371,19 +371,27 @@ add_task(async function addFavicons() {
// Check with http version of the link that doesn't have its own
const nonHttps = [{url: links[0].url.replace("https", "http")}];
await provider._addFavicons(nonHttps);
Assert.equal(provider._faviconBytesToDataURI(nonHttps)[0].favicon, base64URL, "Got the same favicon");
Assert.equal(nonHttps[0].faviconLength, links[0].faviconLength, "Got the same favicon length");
Assert.equal(nonHttps[0].faviconSize, links[0].faviconSize, "Got the same favicon size");
Assert.equal(nonHttps[0].mimeType, links[0].mimeType, "Got the same mime type");
+
+ // Check that we do not collect favicons for pocket items
+ const pocketItems = [{url: links[0].url}, {url: "https://mozilla1.com", type: "pocket"}];
+ await provider._addFavicons(pocketItems);
+ Assert.equal(provider._faviconBytesToDataURI(pocketItems)[0].favicon, base64URL, "Added favicon data only to the non-pocket item");
+ Assert.equal(pocketItems[1].favicon, null, "Did not add a favicon to the pocket item");
+ Assert.equal(pocketItems[1].mimeType, null, "Did not add mimeType to the pocket item");
+ Assert.equal(pocketItems[1].faviconSize, null, "Did not add a faviconSize to the pocket item");
});
-add_task(async function getHighlights() {
+add_task(async function getHighlightsWithoutPocket() {
const addMetadata = url => PlacesUtils.history.update({
description: "desc",
previewImageURL: "https://image/",
url
});
await setUpActivityStreamTest();
@@ -494,16 +502,89 @@ add_task(async function getHighlights()
await PlacesTestUtils.addFavicons(new Map([[testURI, image1x1]]));
links = await provider.getHighlights({ withFavicons: true });
Assert.equal(links.length, 3, "We're not expecting a change in links");
Assert.equal(links[0].favicon, image1x1, "Link 1 should contain a favicon");
Assert.equal(links[1].favicon, null, "Link 2 has no favicon data");
Assert.equal(links[2].favicon, null, "Link 3 has no favicon data");
});
+add_task(async function getHighlightsWithPocketSuccess() {
+ await setUpActivityStreamTest();
+
+ const fakeResponse = {
+ list: {
+ "12345": {
+ image: {src: "foo.com/img.png"},
+ excerpt: "A description for foo",
+ resolved_title: "A title for foo",
+ resolved_url: "http://www.foo.com",
+ item_id: "12345",
+ status: "0",
+ has_image: "1"
+ },
+ "56789": {
+ item_id: "56789",
+ status: "2",
+ }
+ }
+ };
+
+ // Add a bookmark
+ let bookmark = {
+ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+ title: "foo",
+ description: "desc",
+ preview_image_url: "foo.com/img.png",
+ url: "https://mozilla1.com/"
+ };
+ await PlacesUtils.bookmarks.insert(bookmark);
+ await PlacesTestUtils.addVisits(bookmark.url);
+
+ NewTabUtils.activityStreamProvider.fetchSavedPocketItems = () => fakeResponse;
+ let provider = NewTabUtils.activityStreamLinks;
+ let links = await provider.getHighlights();
+
+ // We should have 1 bookmark followed by 1 pocket story in highlights
+ // We should not have stored the second pocket item since it was deleted
+ Assert.equal(links.length, 2, "Should have 2 links in highlights");
+
+ // First highlight should be a bookmark
+ Assert.equal(links[0].url, bookmark.url, "The first link is the bookmark");
+
+ // Second highlight should be a Pocket item with the correct fields to display
+ Assert.equal(links[1].url, fakeResponse.list["12345"].resolved_url, "Correct Pocket item");
+ Assert.equal(links[1].type, "pocket", "Attached the correct type");
+ Assert.equal(links[1].preview_image_url, fakeResponse.list["12345"].image.src, "Correct preview image was added");
+ Assert.equal(links[1].title, fakeResponse.list["12345"].resolved_title, "Correct title was added");
+ Assert.equal(links[1].description, fakeResponse.list["12345"].excerpt, "Correct description was added");
+ Assert.equal(links[1].item_id, fakeResponse.list["12345"].item_id, "item_id was preserved");
+});
+
+add_task(async function getHighlightsWithPocketFailure() {
+ await setUpActivityStreamTest();
+
+ NewTabUtils.activityStreamProvider.fetchSavedPocketItems = function() {
+ throw new Error();
+ };
+ let provider = NewTabUtils.activityStreamLinks;
+ let links = await provider.getHighlights();
+ Assert.equal(links.length, 0, "Return empty links if we reject the promise");
+});
+
+add_task(async function getHighlightsWithPocketNoData() {
+ await setUpActivityStreamTest();
+
+ NewTabUtils.activityStreamProvider.fetchSavedPocketItems = () => {};
+
+ let provider = NewTabUtils.activityStreamLinks;
+ let links = await provider.getHighlights();
+ Assert.equal(links.length, 0, "Return empty links if we got no data back from the response");
+});
+
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");
// add a visit