🙇 Blog icon utils and publisher.logo for JSON-LD (#8297)

refs #8221, closes #7688, refs #7558

🙇  Improve meta data publisher logo behaviour
This is a follow-up PR for #8285.

Reasons: The code changes of #8285 caused error messages when falling back to the default `favicon.ico`, as the `image-size` tool doesn't support `ico` files.

This PR takes the logic to decide which logo needs to be listed in our schema into a new fn `blog_logo.js`. There we have now three decisions:
1. If we have a publication **logo**, we'll take that one
2. If we have no publication logo, but an **icon** we'll use this one.
3. If we have none of the above things, we fall back to our default `favicon.ico`

Additional, we're hard coding image dimensions for whenever the logo is an `.ico` file and built and extra decision to not call `image-size` when the dimension are already given.

I will create another follow-up PR, which checks the extension type for the file and offers it as a util.

🛠  Blog icon util

refs #7688

Serve functionality around the blog icon in its own util:
- getIconDimensions -> async function that takes the filepath of on ico file and returns its dimensions
- isIcoImageType -> returns true if file has `.ico` extension
- getIconType -> returns icon-type (`x-icon` or `png`)
- getIconUrl -> returns the absolut or relativ URL for the favicon: `[subdirectory or not]favicon.[ico or png]`

📖  Get .ico sizes for meta data & logo improvement

refs #7558
refs #8221

Use the new `blogIconUtil` in meta data to fetch the dimensions of `.ico` files.

Improvements for `publisher.logo`: We're now returning a hard-coded 'faked' image dimensions value to render an `imageObject` and prevent error our schema (Google structured data). As soon as an image (`.ico` or non-`.ico`) is too large, but - in case of non-`.ico` - a square format, be set the image-dimensions to 60px width and height. This reduces the chances of getting constantly error messages from Googles' webmaster tools.

- add getIconPath util

core/server/data/meta/asset_url.js

@@ -1,17 +1,13 @@
 var config = require('../../config'),
-    settingsCache = require('../../settings/cache'),
+    blogIconUtils = require('../../utils/blog-icon'),
     utils = require('../../utils');
 
 /**
  * Serve either uploaded favicon or default
  * @return {string}
  */
 function getFaviconUrl() {
-    if (settingsCache.get('icon')) {
-        return settingsCache.get('icon').match(/\.ico$/i) ? utils.url.urlJoin(utils.url.getSubdir(), '/favicon.ico') : utils.url.urlJoin(utils.url.getSubdir(), '/favicon.png');
-    }
-
-    return utils.url.urlJoin(utils.url.getSubdir(), '/favicon.ico');
+    return blogIconUtils.getIconUrl();
 }
 
 function getAssetUrl(path, hasMinFile) {

core/server/data/meta/blog_logo.js

@@ -0,0 +1,46 @@
+var utils            = require('../../utils'),
+    settingsCache    = require('../../settings/cache'),
+    blogIconUtils    = require('../../utils/blog-icon'),
+    Promise          = require('bluebird'),
+    config           = require('../../config'),
+    path             = require('path');
+
+function getBlogLogo() {
+    return new Promise(function getIconSize(resolve, reject) {
+        var logo = {},
+            filePath;
+
+        if (settingsCache.get('logo')) {
+            logo.url = utils.url.urlFor('image', {image: settingsCache.get('logo')}, true);
+        } else {
+            // CASE: no publication logo is updated. We can try to use either an uploaded publication icon
+            // or use the default one to make
+            // Google happy with it. See https://github.com/TryGhost/Ghost/issues/7558
+            logo.url = blogIconUtils.getIconUrl(true);
+
+            if (blogIconUtils.isIcoImageType(logo.url)) {
+                filePath = blogIconUtils.getIconPath();
+                // getIconDimensions needs the physical path of the ico file
+                if (settingsCache.get('icon')) {
+                    // CASE: custom uploaded icon
+                    filePath = path.join(config.getContentPath('images'), filePath);
+                }
+
+                return blogIconUtils.getIconDimensions(filePath).then(function (response) {
+                    logo.dimensions = {
+                        width: response.width,
+                        height: response.height
+                    };
+
+                    return resolve(logo);
+                }).catch(function (err) {
+                    return reject(err);
+                });
+            }
+        }
+
+        return resolve(logo);
+    });
+}
+
+module.exports = getBlogLogo;

core/server/data/meta/image-dimensions.js

@@ -13,7 +13,9 @@ function getImageDimensions(metaData) {
     var fetch = {
             coverImage: getCachedImageSizeFromUrl(metaData.coverImage.url),
             authorImage: getCachedImageSizeFromUrl(metaData.authorImage.url),
-            logo: getCachedImageSizeFromUrl(metaData.blog.logo.url)
+            // CASE: check if logo has hard coded image dimension. In that case it's an `ico` file, which
+            // is not supported by `image-size` and would produce an error
+            logo: metaData.blog.logo && metaData.blog.logo.dimensions ? metaData.blog.logo.dimensions : getCachedImageSizeFromUrl(metaData.blog.logo.url)
         };
 
     return Promise.props(fetch).then(function (resolve) {
@@ -30,7 +32,7 @@ function getImageDimensions(metaData) {
                 // We have some restrictions for publisher.logo:
                 // The image needs to be <=600px wide and <=60px high (ideally exactly 600px x 60px).
                 // Unless we have proper image-handling (see https://github.com/TryGhost/Ghost/issues/4453),
-                // we will not output an ImageObject if the logo doesn't fit in the dimensions.
+                // we will fake it in some cases or not produce an imageObject at all.
                 if (value === 'logo') {
                     if (key.height <= 60 && key.width <= 600) {
                         _.assign(metaData.blog[value], {
@@ -39,6 +41,17 @@ function getImageDimensions(metaData) {
                                 height: key.height
                             }
                         });
+                    } else if ((metaData.blog.logo && metaData.blog.logo.dimensions) || key.width === key.height) {
+                        // CASES:
+                        // 1. .ico files have image dimensions assigned already. If they're not
+                        // within the requirements of Google, we fake them...
+                        // 2. the logo (non-ico) is too large, but it is a square. We fake it as well...
+                        _.assign(metaData.blog[value], {
+                            dimensions: {
+                                width: 60,
+                                height: 60
+                            }
+                        });
                     }
                 } else {
                     _.assign(metaData[value], {

core/server/data/meta/index.js

@@ -1,12 +1,14 @@
 var Promise = require('bluebird'),
     settingsCache = require('../../settings/cache'),
     utils = require('../../utils'),
+    logging = require('../../logging'),
     getUrl = require('./url'),
     getImageDimensions = require('./image-dimensions'),
     getCanonicalUrl = require('./canonical_url'),
     getAmpUrl = require('./amp_url'),
     getPaginatedUrl = require('./paginated_url'),
     getAuthorUrl = require('./author_url'),
+    getBlogLogo = require('./blog_logo'),
     getRssUrl = require('./rss_url'),
     getTitle = require('./title'),
     getDescription = require('./description'),
@@ -61,33 +63,26 @@ function getMetaData(data, root) {
         }
     };
 
-    metaData.blog.logo = {};
+    return Promise.props(getBlogLogo()).then(function (result) {
+        metaData.blog.logo = result;
 
-    if (settingsCache.get('logo')) {
-        metaData.blog.logo.url = utils.url.urlFor('image', {image: settingsCache.get('logo')}, true);
-    } else {
-        metaData.blog.logo.url = utils.url.urlFor({relativeUrl: 'favicon.ico'}, true);
-        // Setting image dimensions to force the default logo to be an `ImageObject` and make
-        // Google happy with it. See https://github.com/TryGhost/Ghost/issues/7558
-        metaData.blog.logo.dimensions = {
-            width: 60,
-            height: 60
-        };
-    }
-
-    // TODO: cleanup these if statements
-    if (data.post && data.post.html) {
-        metaData.excerpt = getExcerpt(data.post.html, {words: 50});
-    }
+        // TODO: cleanup these if statements
+        if (data.post && data.post.html) {
+            metaData.excerpt = getExcerpt(data.post.html, {words: 50});
+        }
 
-    if (data.post && data.post.author && data.post.author.name) {
-        metaData.authorName = data.post.author.name;
-    }
+        if (data.post && data.post.author && data.post.author.name) {
+            metaData.authorName = data.post.author.name;
+        }
 
-    return Promise.props(getImageDimensions(metaData)).then(function () {
-        metaData.structuredData = getStructuredData(metaData);
-        metaData.schema = getSchema(metaData, data);
+        return Promise.props(getImageDimensions(metaData)).then(function () {
+            metaData.structuredData = getStructuredData(metaData);
+            metaData.schema = getSchema(metaData, data);
 
+            return metaData;
+        });
+    }).catch(function (err) {
+        logging.error(err);
         return metaData;
     });
 }

core/server/data/slack/index.js

@@ -4,6 +4,7 @@ var https           = require('https'),
     errors          = require('../../errors'),
     logging         = require('../../logging'),
     utils           = require('../../utils'),
+    blogIconUtils   = require('../../utils/blog-icon'),
     events          = require('../../events'),
     api             = require('../../api/settings'),
     i18n            = require('../../i18n'),
@@ -73,7 +74,7 @@ function ping(post) {
             slackData = {
                 text: message,
                 unfurl_links: true,
-                icon_url: utils.url.urlFor({relativeUrl: 'favicon.ico'}, true),
+                icon_url: blogIconUtils.getIconUrl(true),
                 username: 'Ghost'
             };
 

core/server/helpers/ghost_head.js

@@ -19,7 +19,7 @@ var proxy = require('./proxy'),
     api = proxy.api,
     settingsCache = proxy.settingsCache,
     config = proxy.config,
-    url = proxy.url;
+    blogIconUtils = proxy.blogIcon;
 
 function getClient() {
     if (labs.isSet('publicAPI') === true) {
@@ -96,11 +96,8 @@ module.exports = function ghost_head(options) {
             metaData: getMetaData(this, options.data.root),
             client: getClient()
         },
-        blogIcon = settingsCache.get('icon'),
-        // CASE: blog icon is not set in config, we serve the default
-        iconType = !blogIcon ? 'x-icon' : blogIcon.match(/\.ico$/i) ? 'x-icon' : 'png',
-        favicon = !blogIcon ? url.urlFor({relativeUrl: '/favicon.ico'}) :
-                  blogIcon.match(/\.ico$/i) ? url.urlFor({relativeUrl: '/favicon.ico'}) : url.urlFor({relativeUrl: '/favicon.png'});
+        favicon = blogIconUtils.getIconUrl(),
+        iconType = blogIconUtils.getIconType(favicon);
 
     return Promise.props(fetch).then(function (response) {
         client = response.client;

core/server/helpers/proxy.js

@@ -62,6 +62,7 @@ module.exports = {
 
     // Various utils, needs cleaning up / simplifying
     socialUrls: require('../utils/social-urls'),
+    blogIcon: require('../utils/blog-icon'),
     url: require('../utils').url,
     utils: {
         findKey: function findKey(key /* ...objects... */) {

core/server/middleware/serve-favicon.js

@@ -1,10 +1,10 @@
 var fs = require('fs'),
     path = require('path'),
     crypto = require('crypto'),
-    config = require('../config'),
     storage = require('../storage'),
     utils  = require('../utils'),
     settingsCache = require('../settings/cache'),
+    blogIconUtils = require('../utils/blog-icon'),
     buildContentResponse,
     content;
 
@@ -36,7 +36,7 @@ function serveFavicon() {
             // we are using an express route to skip /content/images and the result is a image path
             // based on config.getContentPath('images') + req.path
             // in this case we don't use path rewrite, that's why we have to make it manually
-            filePath = settingsCache.get('icon').replace(new RegExp(utils.url.STATIC_IMAGE_URL_PREFIX), '');
+            filePath = blogIconUtils.getIconPath();
 
             var originalExtension = path.extname(filePath).toLowerCase(),
                 requestedExtension = path.extname(req.path).toLowerCase();
@@ -51,7 +51,7 @@ function serveFavicon() {
                 storage.getStorage()
                     .read({path: filePath})
                     .then(function readFile(buf) {
-                        iconType = settingsCache.get('icon').match(/\.ico$/i) ? 'x-icon' : 'png';
+                        iconType = blogIconUtils.getIconType();
                         content = buildContentResponse(iconType, buf);
 
                         res.writeHead(200, content.headers);
@@ -61,7 +61,6 @@ function serveFavicon() {
                         next(err);
                     });
             } else {
-                filePath = path.join(config.get('paths:publicFilePath'), 'favicon.ico');
                 originalExtension = path.extname(filePath).toLowerCase();
 
                 // CASE: always redirect to .ico for default icon

core/server/middleware/validation/blog-icon.js

@@ -1,10 +1,9 @@
 var errors = require('../../errors'),
     config = require('../../config'),
-    fs = require('fs'),
     Promise = require('bluebird'),
     sizeOf = require('image-size'),
     i18n = require('../../i18n'),
-    _ = require('lodash'),
+    blogIconUtils = require('../../utils/blog-icon'),
     validIconSize,
     getIconDimensions;
 
@@ -14,40 +13,24 @@ validIconSize = function validIconSize(size) {
 
 getIconDimensions = function getIconDimensions(icon) {
     return new Promise(function getImageSize(resolve, reject) {
-        var arrayBuffer,
-            ICO = require('icojs');
-
-        // image-size doesn't support .ico files
-        if (icon.name.match(/.ico$/i)) {
-            arrayBuffer = new Uint8Array(fs.readFileSync(icon.path)).buffer;
-            ICO.parse(arrayBuffer).then(function (result, error) {
-                if (error) {
-                    return reject(new errors.ValidationError({message: i18n.t('errors.api.icons.couldNotGetSize', {file: icon.name, error: error.message})}));
-                }
-
-                // CASE: ico file contains only one size
-                if (result.length === 1) {
-                    return resolve({
-                        width: result[0].width,
-                        height: result[0].height
-                    });
-                } else {
-                    // CASE: ico file contains multiple sizes, return only the max size
-                    return resolve({
-                        width: _.maxBy(result, function (w) {return w.width;}).width,
-                        height: _.maxBy(result, function (h) {return h.height;}).height
-                    });
-                }
+        if (blogIconUtils.isIcoImageType(icon.name)) {
+            blogIconUtils.getIconDimensions(icon.path).then(function (response) {
+                return resolve({
+                    width: response.width,
+                    height: response.height
+                });
+            }).catch(function (err) {
+                return reject(err);
             });
         } else {
-            sizeOf(icon.path, function (err, dimensions) {
+            sizeOf(icon.path, function (err, response) {
                 if (err) {
                     return reject(new errors.ValidationError({message: i18n.t('errors.api.icons.couldNotGetSize', {file: icon.name, error: err.message})}));
                 }
 
                 return resolve({
-                    width: dimensions.width,
-                    height: dimensions.height
+                    width: response.width,
+                    height: response.height
                 });
             });
         }
@@ -64,9 +47,9 @@ module.exports = function blogIcon() {
             return next(new errors.ValidationError({message: i18n.t('errors.api.icons.invalidFile', {extensions: iconExtensions})}));
         }
 
-        return getIconDimensions(req.file).then(function (dimensions) {
+        return getIconDimensions(req.file).then(function (response) {
             // save the image dimensions in new property for file
-            req.file.dimensions = dimensions;
+            req.file.dimensions = response;
 
             // CASE: file needs to be a square
             if (req.file.dimensions.width !== req.file.dimensions.height) {
@@ -85,6 +68,8 @@ module.exports = function blogIcon() {
             }
 
             next();
+        }).catch(function (err) {
+            next(err);
         });
     };
 };

core/server/translations/en.json

@@ -115,6 +115,9 @@
                 "nameOrVersionMissing": "\"name\" or \"version\" is missing from theme package.json file.",
                 "willBeRequired": "This will be required in future. Please see {url}",
                 "themeFileIsMalformed": "Theme package.json file is malformed"
+            },
+            "blogIcon": {
+                "error": "Could not fetch icon dimensions."
             }
         },
         "config": {