diff --git a/browser/base/content/test/newtab/browser_newtab_enhanced.js b/browser/base/content/test/newtab/browser_newtab_enhanced.js index 83008dd80fed..619ad8bf21b8 100644 --- a/browser/base/content/test/newtab/browser_newtab_enhanced.js +++ b/browser/base/content/test/newtab/browser_newtab_enhanced.js @@ -4,11 +4,24 @@ const PRELOAD_PREF = "browser.newtab.preload"; gDirectorySource = "data:application/json," + JSON.stringify({ - "directory": [{ + "enhanced": [{ url: "http://example.com/", enhancedImageURI: "data:image/png;base64,helloWORLD", title: "title", + type: "organic", + }], + "directory": [{ + url: "http://example1.com/", + enhancedImageURI: "data:image/png;base64,helloWORLD2", + title: "title1", type: "organic" + }], + "suggested": [{ + url: "http://example1.com/2", + imageURI: "data:image/png;base64,helloWORLD3", + title: "title2", + type: "affiliate", + frecent_sites: ["test.com"] }] }); @@ -33,7 +46,7 @@ function runTests() { }; } - // Make the page have a directory link followed by a history link + // Make the page have a directory link, enhanced link, and history link yield setLinks("-1"); // Test with enhanced = false @@ -52,19 +65,29 @@ function runTests() { ({type, enhanced, title} = getData(0)); is(type, "organic", "directory link is organic"); isnot(enhanced, "", "directory link has enhanced image"); + is(title, "title1"); + + ({type, enhanced, title} = getData(1)); + is(type, "enhanced", "history link is enhanced"); + isnot(enhanced, "", "history link has enhanced image"); is(title, "title"); - is(getData(1), null, "history link pushed out by directory link"); + is(getData(2), null, "there are only 2 links, directory and enhanced history"); // Test with a pinned link setPinnedLinks("-1"); yield addNewTabPageTab(); ({type, enhanced, title} = getData(0)); - is(type, "history", "pinned history link is not enhanced"); - is(enhanced, "", "pinned history link doesn't have enhanced image"); - is(title, "site#-1"); + is(type, "enhanced", "pinned history link is enhanced"); + isnot(enhanced, "", "pinned history link has enhanced image"); + is(title, "title"); - is(getData(1), null, "directory link pushed out by pinned history link"); + ({type, enhanced, title} = getData(1)); + is(type, "organic", "directory link is organic"); + isnot(enhanced, "", "directory link has enhanced image"); + is(title, "title1"); + + is(getData(2), null, "directory link pushed out by pinned history link"); // Test pinned link with enhanced = false yield addNewTabPageTab(); @@ -78,4 +101,42 @@ function runTests() { ok(getContentDocument().getElementById("newtab-intro-what"), "'What is this page?' link exists"); + + yield unpinCell(0); + + + + // Test that a suggested tile is not enhanced by a directory tile + let origIsTopPlacesSite = NewTabUtils.isTopPlacesSite; + NewTabUtils.isTopPlacesSite = () => true; + yield setLinks("-1"); + + // Test with enhanced = false + yield addNewTabPageTab(); + yield customizeNewTabPage("classic"); + ({type, enhanced, title} = getData(0)); + isnot(type, "enhanced", "history link is not enhanced"); + is(enhanced, "", "history link has no enhanced image"); + is(title, "site#-1"); + + is(getData(1), null, "there is only one link and it's a history link"); + + + // Test with enhanced = true + yield addNewTabPageTab(); + yield customizeNewTabPage("enhanced"); + + // Suggested link was not enhanced by directory link with same domain + ({type, enhanced, title} = getData(0)); + is(type, "affiliate", "suggested link is affiliate"); + is(enhanced, "", "suggested link has no enhanced image"); + is(title, "title2"); + + // Enhanced history link shows up second + ({type, enhanced, title} = getData(1)); + is(type, "enhanced", "pinned history link is enhanced"); + isnot(enhanced, "", "pinned history link has enhanced image"); + is(title, "title"); + + is(getData(2), null, "there is only a suggested link followed by an enhanced history link"); } diff --git a/browser/modules/DirectoryLinksProvider.jsm b/browser/modules/DirectoryLinksProvider.jsm index ad8a5711ba61..00b89cca54fe 100644 --- a/browser/modules/DirectoryLinksProvider.jsm +++ b/browser/modules/DirectoryLinksProvider.jsm @@ -215,6 +215,10 @@ let DirectoryLinksProvider = { }, _cacheSuggestedLinks: function(link) { + if (!link.frecent_sites) { + // Don't cache links that don't have the expected 'frecent_sites'. + return; + } for (let suggestedSite of link.frecent_sites) { let suggestedMap = this._suggestedLinks.get(suggestedSite) || new Map(); suggestedMap.set(link.url, link); @@ -311,13 +315,15 @@ let DirectoryLinksProvider = { * or {'directory': [], 'suggested': []} if read or parse fails. */ _readDirectoryLinksFile: function DirectoryLinksProvider_readDirectoryLinksFile() { - let emptyOutput = {directory: [], suggested: []}; + let emptyOutput = {directory: [], suggested: [], enhanced: []}; return OS.File.read(this._directoryFilePath).then(binaryData => { let output; try { let json = gTextDecoder.decode(binaryData); let linksObj = JSON.parse(json); - output = {directory: linksObj.directory || [], suggested: linksObj.suggested || []}; + output = {directory: linksObj.directory || [], + suggested: linksObj.suggested || [], + enhanced: linksObj.enhanced || []}; } catch (e) { Cu.reportError(e); @@ -415,8 +421,7 @@ let DirectoryLinksProvider = { */ getEnhancedLink: function DirectoryLinksProvider_getEnhancedLink(link) { // Use the provided link if it's already enhanced - return link.type == "history" ? null : - link.enhancedImageURI && link ? link : + return link.enhancedImageURI && link ? link : this._enhancedLinks.get(NewTabUtils.extractSite(link.url)); }, @@ -456,16 +461,8 @@ let DirectoryLinksProvider = { this.isURLAllowed(link.enhancedImageURI, ALLOWED_IMAGE_SCHEMES); }.bind(this); - let setCommonProperties = function(link, length, position) { - // Stash the enhanced image for the site - if (link.enhancedImageURI) { - this._enhancedLinks.set(NewTabUtils.extractSite(link.url), link); - } - link.lastVisitDate = length - position; - }.bind(this); - rawLinks.suggested.filter(validityFilter).forEach((link, position) => { - setCommonProperties(link, rawLinks.suggested.length, position); + link.lastVisitDate = rawLinks.suggested.length - position; // We cache suggested tiles here but do not push any of them in the links list yet. // The decision for which suggested tile to include will be made separately. @@ -473,8 +470,17 @@ let DirectoryLinksProvider = { this._frequencyCaps.set(link.url, DEFAULT_FREQUENCY_CAP); }); + rawLinks.enhanced.filter(validityFilter).forEach((link, position) => { + link.lastVisitDate = rawLinks.enhanced.length - position; + + // Stash the enhanced image for the site + if (link.enhancedImageURI) { + this._enhancedLinks.set(NewTabUtils.extractSite(link.url), link); + } + }); + let links = rawLinks.directory.filter(validityFilter).map((link, position) => { - setCommonProperties(link, rawLinks.directory.length, position); + link.lastVisitDate = rawLinks.directory.length - position; link.frecency = DIRECTORY_FRECENCY; return link; }); diff --git a/browser/modules/test/xpcshell/test_DirectoryLinksProvider.js b/browser/modules/test/xpcshell/test_DirectoryLinksProvider.js index 36dae814b5a5..9b314df0f7e8 100644 --- a/browser/modules/test/xpcshell/test_DirectoryLinksProvider.js +++ b/browser/modules/test/xpcshell/test_DirectoryLinksProvider.js @@ -1026,7 +1026,7 @@ add_task(function test_DirectoryLinksProvider_getAllowedEnhancedImages() { }); add_task(function test_DirectoryLinksProvider_getEnhancedLink() { - let data = {"directory": [ + let data = {"enhanced": [ {url: "http://example.net", enhancedImageURI: "data:,net1"}, {url: "http://example.com", enhancedImageURI: "data:,com1"}, {url: "http://example.com", enhancedImageURI: "data:,com2"}, @@ -1035,7 +1035,7 @@ add_task(function test_DirectoryLinksProvider_getEnhancedLink() { yield promiseSetupDirectoryLinksProvider({linksURL: dataURI}); let links = yield fetchData(); - do_check_eq(links.length, 3); + do_check_eq(links.length, 0); // There are no directory links. function checkEnhanced(url, image) { let enhanced = DirectoryLinksProvider.getEnhancedLink({url: url}); @@ -1066,17 +1066,63 @@ add_task(function test_DirectoryLinksProvider_getEnhancedLink() { checkEnhanced("http://127.0.0.1", undefined); // Make sure old data is not cached - data = {"directory": [ + data = {"enhanced": [ {url: "http://example.com", enhancedImageURI: "data:,fresh"}, ]}; dataURI = 'data:application/json,' + JSON.stringify(data); yield promiseSetupDirectoryLinksProvider({linksURL: dataURI}); links = yield fetchData(); - do_check_eq(links.length, 1); + do_check_eq(links.length, 0); // There are no directory links. checkEnhanced("http://example.net", undefined); checkEnhanced("http://example.com", "data:,fresh"); }); +add_task(function test_DirectoryLinksProvider_enhancedURIs() { + let origIsTopPlacesSite = NewTabUtils.isTopPlacesSite; + NewTabUtils.isTopPlacesSite = () => true; + + let data = { + "suggested": [ + {url: "http://example.net", enhancedImageURI: "data:,net1", title:"SuggestedTitle", frecent_sites: ["test.com"]} + ], + "directory": [ + {url: "http://example.net", enhancedImageURI: "data:,net2", title:"DirectoryTitle"} + ] + }; + let dataURI = 'data:application/json,' + JSON.stringify(data); + yield promiseSetupDirectoryLinksProvider({linksURL: dataURI}); + + // Wait for links to get loaded + let gLinks = NewTabUtils.links; + gLinks.addProvider(DirectoryLinksProvider); + gLinks.populateCache(); + yield new Promise(resolve => { + NewTabUtils.allPages.register({ + observe: _ => _, + update() { + NewTabUtils.allPages.unregister(this); + resolve(); + } + }); + }); + + // Check that we've saved the directory tile. + let links = yield fetchData(); + do_check_eq(links.length, 1); + do_check_eq(links[0].title, "DirectoryTitle"); + do_check_eq(links[0].enhancedImageURI, "data:,net2"); + + // Check that the suggested tile with the same URL replaces the directory tile. + links = gLinks.getLinks(); + do_check_eq(links.length, 1); + do_check_eq(links[0].title, "SuggestedTitle"); + do_check_eq(links[0].enhancedImageURI, "data:,net1"); + + // Cleanup. + NewTabUtils.isTopPlacesSite = origIsTopPlacesSite; + gLinks.removeProvider(DirectoryLinksProvider); +}); + add_task(function test_DirectoryLinksProvider_setDefaultEnhanced() { function checkDefault(expected) { Services.prefs.clearUserPref(kNewtabEnhancedPref); diff --git a/toolkit/modules/NewTabUtils.jsm b/toolkit/modules/NewTabUtils.jsm index e8d9030c861c..9776fb318843 100644 --- a/toolkit/modules/NewTabUtils.jsm +++ b/toolkit/modules/NewTabUtils.jsm @@ -1014,11 +1014,7 @@ let Links = { this._decrementSiteMap(siteMap, existingLink); } else { // Update our copy's properties. - for (let prop of this._sortProperties) { - if (prop in aLink) { - existingLink[prop] = aLink[prop]; - } - } + Object.assign(existingLink, aLink); // Finally, reinsert our copy below. insertionLink = existingLink;