mirror of
https://github.com/TryGhost/Ghost.git
synced 2024-12-23 10:53:34 +03:00
🐛 Fixed email header images serving original image size (#15950)
fixes https://github.com/TryGhost/Team/issues/2368 - Removed the usage of the `isLocalContentImage` Koenig util for the email header and feature image url generation. - While we were trying to set the width to 1200px, we didn't have that size hardcoded. So that url would redirect back to the original location instead of serving a smaller image. So I added a new internal size to the `imageOptimization` config. - This is fixed in both the new and old email flow and includes some extra tests for the new flow.
This commit is contained in:
parent
527d718b76
commit
9e6f5e93d8
@ -34,6 +34,7 @@ class EmailServiceWrapper {
|
||||
const linkReplacer = require('@tryghost/link-replacer');
|
||||
const linkTracking = require('../link-tracking');
|
||||
const audienceFeedback = require('../audience-feedback');
|
||||
const storageUtils = require('../../adapters/storage/utils');
|
||||
|
||||
// capture errors from mailgun client and log them in sentry
|
||||
const errorHandler = (error) => {
|
||||
@ -60,6 +61,7 @@ class EmailServiceWrapper {
|
||||
},
|
||||
imageSize: null,
|
||||
urlUtils,
|
||||
storageUtils,
|
||||
getPostUrl: this.getPostUrl,
|
||||
linkReplacer,
|
||||
linkTracking,
|
||||
|
@ -10,7 +10,7 @@ const mobiledocLib = require('../../lib/mobiledoc');
|
||||
const lexicalLib = require('../../lib/lexical');
|
||||
const htmlToPlaintext = require('@tryghost/html-to-plaintext');
|
||||
const membersService = require('../members');
|
||||
const {isUnsplashImage, isLocalContentImage} = require('@tryghost/kg-default-cards/lib/utils');
|
||||
const {isUnsplashImage} = require('@tryghost/kg-default-cards/lib/utils');
|
||||
const {textColorForBackgroundColor, darkenToContrastThreshold} = require('@tryghost/color-utils');
|
||||
const logging = require('@tryghost/logging');
|
||||
const urlService = require('../../services/url');
|
||||
@ -19,6 +19,7 @@ const linkTracking = require('../link-tracking');
|
||||
const memberAttribution = require('../member-attribution');
|
||||
const feedbackButtons = require('./feedback-buttons');
|
||||
const labs = require('../../../shared/labs');
|
||||
const storageUtils = require('../../adapters/storage/utils');
|
||||
|
||||
const ALLOWED_REPLACEMENTS = ['first_name', 'uuid'];
|
||||
|
||||
@ -261,7 +262,7 @@ const PostEmailSerializer = {
|
||||
templateSettings.headerImageWidth = 600;
|
||||
}
|
||||
|
||||
if (isLocalContentImage(templateSettings.headerImage, urlUtils.getSiteUrl())) {
|
||||
if (storageUtils.isLocalImage(templateSettings.headerImage)) {
|
||||
// we can safely request a 1200px image - Ghost will serve the original if it's smaller
|
||||
templateSettings.headerImage = templateSettings.headerImage.replace(/\/content\/images\//, '/content/images/size/w1200/');
|
||||
}
|
||||
@ -348,7 +349,7 @@ const PostEmailSerializer = {
|
||||
post.feature_image_width = 600;
|
||||
}
|
||||
|
||||
if (isLocalContentImage(post.feature_image, urlUtils.getSiteUrl())) {
|
||||
if (storageUtils.isLocalImage(post.feature_image)) {
|
||||
// we can safely request a 1200px image - Ghost will serve the original if it's smaller
|
||||
post.feature_image = post.feature_image.replace(/\/content\/images\//, '/content/images/size/w1200/');
|
||||
}
|
||||
|
@ -86,7 +86,8 @@
|
||||
"w2400": {"width": 2400}
|
||||
},
|
||||
"internalImageSizes": {
|
||||
"icon": {"width": 256, "height": 256}
|
||||
"icon": {"width": 256, "height": 256},
|
||||
"email-header-image": {"width": 1200}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -50,6 +50,7 @@ class EmailRenderer {
|
||||
#imageSize;
|
||||
#urlUtils;
|
||||
#getPostUrl;
|
||||
#storageUtils;
|
||||
|
||||
#handlebars;
|
||||
#renderTemplate;
|
||||
@ -67,6 +68,7 @@ class EmailRenderer {
|
||||
* @param {{render(object, options): string}} dependencies.renderers.mobiledoc
|
||||
* @param {{getImageSizeFromUrl(url: string): Promise<{width: number}>}} dependencies.imageSize
|
||||
* @param {{urlFor(type: string, optionsOrAbsolute, absolute): string, isSiteUrl(url, context): boolean}} dependencies.urlUtils
|
||||
* @param {{isLocalImage(url: string): boolean}} dependencies.storageUtils
|
||||
* @param {(post: Post) => string} dependencies.getPostUrl
|
||||
* @param {object} dependencies.linkReplacer
|
||||
* @param {object} dependencies.linkTracking
|
||||
@ -79,6 +81,7 @@ class EmailRenderer {
|
||||
renderers,
|
||||
imageSize,
|
||||
urlUtils,
|
||||
storageUtils,
|
||||
getPostUrl,
|
||||
linkReplacer,
|
||||
linkTracking,
|
||||
@ -90,6 +93,7 @@ class EmailRenderer {
|
||||
this.#renderers = renderers;
|
||||
this.#imageSize = imageSize;
|
||||
this.#urlUtils = urlUtils;
|
||||
this.#storageUtils = storageUtils;
|
||||
this.#getPostUrl = getPostUrl;
|
||||
this.#linkReplacer = linkReplacer;
|
||||
this.#linkTracking = linkTracking;
|
||||
@ -633,9 +637,7 @@ class EmailRenderer {
|
||||
size.width = 600;
|
||||
}
|
||||
|
||||
// WARNING:
|
||||
// TODO: this whole `isLocalContentImage` can never ever work (always false), this is old code that needs a rewrite!
|
||||
if (isLocalContentImage(href, this.#urlUtils.urlFor('home', true))) {
|
||||
if (this.#storageUtils.isLocalImage(href)) {
|
||||
// we can safely request a 1200px image - Ghost will serve the original if it's smaller
|
||||
return {
|
||||
href: href.replace(/\/content\/images\//, '/content/images/size/w1200/'),
|
||||
|
@ -516,4 +516,66 @@ describe('Email renderer', function () {
|
||||
responsePaid.html.should.not.containEql('Become a paid member of Test Blog to get access to all');
|
||||
});
|
||||
});
|
||||
|
||||
describe('limitImageWidth', function () {
|
||||
it('Limits width of local images', async function () {
|
||||
const emailRenderer = new EmailRenderer({
|
||||
imageSize: {
|
||||
getImageSizeFromUrl() {
|
||||
return {
|
||||
width: 2000
|
||||
};
|
||||
}
|
||||
},
|
||||
storageUtils: {
|
||||
isLocalImage(url) {
|
||||
return url === 'http://your-blog.com/content/images/2017/01/02/example.png';
|
||||
}
|
||||
}
|
||||
});
|
||||
const response = await emailRenderer.limitImageWidth('http://your-blog.com/content/images/2017/01/02/example.png');
|
||||
assert.equal(response.width, 600);
|
||||
assert.equal(response.href, 'http://your-blog.com/content/images/size/w1200/2017/01/02/example.png');
|
||||
});
|
||||
|
||||
it('Limits width of unsplash images', async function () {
|
||||
const emailRenderer = new EmailRenderer({
|
||||
imageSize: {
|
||||
getImageSizeFromUrl() {
|
||||
return {
|
||||
width: 2000
|
||||
};
|
||||
}
|
||||
},
|
||||
storageUtils: {
|
||||
isLocalImage(url) {
|
||||
return url === 'http://your-blog.com/content/images/2017/01/02/example.png';
|
||||
}
|
||||
}
|
||||
});
|
||||
const response = await emailRenderer.limitImageWidth('https://images.unsplash.com/photo-1657816793628-191deb91e20f?crop=entropy&cs=tinysrgb&fit=max&fm=jpg&ixid=MnwxMTc3M3wwfDF8YWxsfDJ8fHx8fHwyfHwxNjU3ODkzNjU5&ixlib=rb-1.2.1&q=80&w=2000');
|
||||
assert.equal(response.width, 600);
|
||||
assert.equal(response.href, 'https://images.unsplash.com/photo-1657816793628-191deb91e20f?crop=entropy&cs=tinysrgb&fit=max&fm=jpg&ixid=MnwxMTc3M3wwfDF8YWxsfDJ8fHx8fHwyfHwxNjU3ODkzNjU5&ixlib=rb-1.2.1&q=80&w=1200');
|
||||
});
|
||||
|
||||
it('Does not increase width of images', async function () {
|
||||
const emailRenderer = new EmailRenderer({
|
||||
imageSize: {
|
||||
getImageSizeFromUrl() {
|
||||
return {
|
||||
width: 300
|
||||
};
|
||||
}
|
||||
},
|
||||
storageUtils: {
|
||||
isLocalImage(url) {
|
||||
return url === 'http://your-blog.com/content/images/2017/01/02/example.png';
|
||||
}
|
||||
}
|
||||
});
|
||||
const response = await emailRenderer.limitImageWidth('https://example.com/image.png');
|
||||
assert.equal(response.width, 300);
|
||||
assert.equal(response.href, 'https://example.com/image.png');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
Loading…
Reference in New Issue
Block a user