fix (content): #2083 make large favicons fill the tile

addon/Feeds/TopSitesFeed.js

@@ -31,7 +31,7 @@ module.exports = class TopSitesFeed extends Feed {
       if (experiments.screenshots) {
         try {
           links = yield getScreenshots(links, site => {
-            if (site.favicon_height >= 64 && site.favicon_width >= 64 && isRootDomain(site.url)) {
+            if (site.favicon_height >= 96 && site.favicon_width >= 96 && isRootDomain(site.url)) {
               // If we are at the "root domain path" and the icon is big enough,
               // we don't show a screenshot.
               return false;

content-src/components/TopSites/TopSites.js

@@ -25,20 +25,15 @@ const TopSitesItem = React.createClass({
   _faviconSize(site, topSitesExperimentIsOn, screenshot) {
     let faviconSize = 32;
     if (topSitesExperimentIsOn && !screenshot) {
-      if (!site.favicon_width) {
-        // default to 64 if not specified
+      if (site.favicon_url && (site.favicon_url.startsWith("favicons/images") || site.favicon_url.startsWith("resource://"))) {
+        // If it starts with favicons/images or resource:// then it's a tippy top icon.
+        // We want the size set to 64 for those.
+        // FIXME: long term we want the metadata parser to pass along where the image came from.
         faviconSize = 64;
       } else {
-        faviconSize = site.favicon_width;
-      }
-
-      // We want the favicon to be at least 32x32 and at most 64x64 for now because
-      // I noticed a bunch of issues when letting the icons fill the tile (96x96).
-      // And we don't want them to be smaller than 32, our previous fixed size.
-      if (faviconSize > 64) {
-        faviconSize = 64;
-      } else if (faviconSize < 32) {
-        faviconSize = 32;
+        // If we have a normal (non tippy top) favicon, we're going to stretch
+        // or shrink it to be wall to wall.
+        faviconSize = 96;
       }
     }
     return faviconSize;
@@ -51,6 +46,7 @@ const TopSitesItem = React.createClass({
     const topSitesExperimentIsOn = this.props.showNewStyle;
     const screenshot = topSitesExperimentIsOn && site.screenshot;
     const faviconSize = this._faviconSize(site, topSitesExperimentIsOn, screenshot);
+    const showBackground = faviconSize < 96;
 
     // The top-corner class puts the site icon in the top corner, overlayed over the screenshot.
     const siteIconClasses = classNames("tile-img-container", {"top-corner": screenshot});
@@ -66,6 +62,7 @@ const TopSitesItem = React.createClass({
           className={siteIconClasses}
           site={site} faviconSize={faviconSize}
           showTitle={!screenshot}
+          showBackground={showBackground}
           showNewStyle={topSitesExperimentIsOn} />
 
         {screenshot && <div ref="title" className="site-title">{label}</div>}

content-src/lib/first-run-data.js

@@ -9,37 +9,49 @@ module.exports = {
       "title": "Facebook",
       "url": "https://www.facebook.com/",
       "favicon_url": "facebook-com.png",
-      "background_color": [59, 89, 152]
+      "background_color": [59, 89, 152],
+      "favicon_height": 64,
+      "favicon_width": 64
     },
     {
       "title": "YouTube",
       "url": "https://www.youtube.com/",
       "favicon_url": "youtube-com.png",
-      "background_color": [219, 67, 56]
+      "background_color": [219, 67, 56],
+      "favicon_height": 64,
+      "favicon_width": 64
     },
     {
       "title": "Amazon",
       "url": "http://www.amazon.com/",
       "favicon_url": "amazon-com.png",
-      "background_color": [255, 255, 255]
+      "background_color": [255, 255, 255],
+      "favicon_height": 64,
+      "favicon_width": 64
     },
     {
       "title": "Yahoo",
       "url": "https://www.yahoo.com/",
       "favicon_url": "yahoo-com.png",
-      "background_color": [80, 9, 167]
+      "background_color": [80, 9, 167],
+      "favicon_height": 64,
+      "favicon_width": 64
     },
     {
       "title": "eBay",
       "url": "http://www.ebay.com",
       "favicon_url": "ebay-com.png",
-      "background_color": [237, 237, 237]
+      "background_color": [237, 237, 237],
+      "favicon_height": 64,
+      "favicon_width": 64
     },
     {
       "title": "Twitter",
       "url": "https://twitter.com/",
       "favicon_url": "twitter-com.png",
-      "background_color": [4, 159, 245]
+      "background_color": [4, 159, 245],
+      "favicon_height": 64,
+      "favicon_width": 64
     }
   ].map(item => {
     item.type = FIRST_RUN_TYPE;

content-test/addon/Feeds/TopSitesFeed.test.js

@@ -72,19 +72,19 @@ describe("TopSitesFeed", () => {
         const shouldGetScreenshot = getScreenshots.getCall(0).args[1];
         assert.equal(
           false,
-          shouldGetScreenshot({favicon_width: 64, favicon_height: 64, url: "https://mozilla.org/"}
+          shouldGetScreenshot({favicon_width: 96, favicon_height: 96, url: "https://mozilla.org/"}
         ));
         assert.equal(
           false,
-          shouldGetScreenshot({favicon_width: 64, favicon_height: 64, url: "https://mozilla.org"}
+          shouldGetScreenshot({favicon_width: 96, favicon_height: 96, url: "https://mozilla.org"}
         ));
         assert.equal(
           true,
-          shouldGetScreenshot({favicon_width: 64, favicon_height: 64, url: "https://mozilla.org/foo"}
+          shouldGetScreenshot({favicon_width: 96, favicon_height: 96, url: "https://mozilla.org/foo"}
         ));
         assert.equal(
           true,
-          shouldGetScreenshot({favicon_width: 32, favicon_height: 32, url: "https://mozilla.org"}
+          shouldGetScreenshot({favicon_width: 95, favicon_height: 95, url: "https://mozilla.org"}
         ));
       });
     });