-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dianoga does not optimize next gen image formats with resized JSS media URLs #129
Comments
OK I think I've figured this out, It's because the JSS image resizing whitelist blocks parsing of the querystring if ALL the querystring parameters from "protectedMediaQueryParameters" are not listed in the allowedMediaParams list (apart from rev which is ignored). "extension" is added to the "protectedMediaQueryParameters" by Dianoga's webp config file and it seems to be required for the different images to be served/cached independently. So can't remove this. Since it is an exact match of the combination of all the parameters, you can't realistically add all combinations of mw/mh/extension (perhaps you could if you only want webp since I think the comma used in extension=avif,webp would be an issue because allowedMediaParams uses commas as a separate) I have therefore created a override of this class public class ConfigurationWhitelistProcessor : Sitecore.JavaScriptServices.Media.Pipelines.RestrictMediaRequest.ConfigurationWhitelistProcessor
{
public ConfigurationWhitelistProcessor(BaseMediaManager mediaManager, WhitelistConfiguration whitelistConfiguration, BaseLog log, BaseSettings settings) : base(mediaManager, whitelistConfiguration, log, settings)
{
}
/// <summary>
/// Extension is added to the protectedMediaQueryParameters needed for each variation to be cached seperately.
/// But it should be ignored by the JSS image resize whitelist since it isn't listed in allowedMediaParams
/// as that would require every combination of mw/mh/extension to be added.
/// </summary>
/// <param name="queryString"></param>
/// <returns></returns>
protected override IDictionary<string, string> GetProtectedParams(NameValueCollection queryString)
{
var p = base.GetProtectedParams(queryString);
if (p.ContainsKey("extension") && IsValidExtension(p["extension"]))
{
p.Remove("extension");
}
return p;
}
string[] validExtensions = new string[] { "webp", "avif" };
private bool IsValidExtension(string extension)
{
if (!string.IsNullOrEmpty(extension))
{
var splitExt = extension.Split(',');
if (splitExt.All(s => validExtensions.Contains(s)))
{
return true;
}
}
return false;
}
} Ideally the list of valid extensions would come from the Dianoga enabled next-gen configs but I haven't figured out how to do that yet. The dianogaGetSupportedFormats isn't suitable as-is because you have to pass in the Accept header which defeats the point as I'd have to hardcode that so for now I'm just hardcoding the common next-gen extensions, that we are using. This pipeline has to be replaced via config include: <configuration xmlns:patch="http://www.sitecore.net/xmlconfig/" xmlns:role="http://www.sitecore.net/xmlconfig/role/" xmlns:set="http://www.sitecore.net/xmlconfig/set/">
<sitecore>
<pipelines>
<group groupName="javaScriptServices">
<pipelines>
<restrictMediaRequest>
<processor type="Sitecore.JavaScriptServices.Media.Pipelines.RestrictMediaRequest.ConfigurationWhitelistProcessor, Sitecore.JavaScriptServices.Media">
<patch:delete />
</processor>
<processor type="Your.Optimisation.Media.ConfigurationWhitelistProcessor, Your.Optimisation" resolve="true" />
</restrictMediaRequest>
</pipelines>
</group>
</pipelines>
</sitecore>
</configuration> |
Worth noting for completeness that if you want next-gen optimization of images when NOT passing any resize querystring then you need to override using Dianoga.NextGenFormats;
using Sitecore.Resources.Media;
using System.Web;
public class JssMediaRequestHandler : Sitecore.JavaScriptServices.Media.MediaRequestHandler
{
protected override bool DoProcessRequest(HttpContext context, MediaRequest request, Sitecore.Resources.Media.Media media)
{
request.AddCustomOptions(new HttpContextWrapper(context));
return base.DoProcessRequest(context, request, media);
}
} Although thinking about it, if you are using CDN maybe don't want extensionless media requests to get converted to next-gen formats... |
Do you have SXA installed? If so I'd suggest use the SXA MediaRequestHandler |
Hi, no I don’t. But thanks for the tip.
Sent from Outlook for iOS<https://aka.ms/o0ukef>
…________________________________
From: Mark Gibbons ***@***.***>
Sent: Wednesday, June 28, 2023 2:59:14 AM
To: kamsar/Dianoga ***@***.***>
Cc: Andrew Lansdowne ***@***.***>; Author ***@***.***>
Subject: Re: [kamsar/Dianoga] Dianoga does not optimize next gen image formats with resized JSS media URLs (Issue #129)
Do you have SXA installed? If so I'd suggest use the SXA MediaRequestHandler
—
Reply to this email directly, view it on GitHub<#129 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/A5NLROKCMQN4A4YG23PHBETXNOFXFANCNFSM6AAAAAAZHPVX24>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
We have exactly the same issue. We tried to implement the solution from above, but no luck. The 'jssmedia' webp image does not get resized although it works for example for a jpeg image. What we also notice is that when we have a 'jssmedia' url we don't have extension and hash query params. Any idea how we should approach this any further? |
@kpicavet jssmedia URLs intentionally don't use hash query param - instead it relies on the extension whitelist to determine valid requests - https://doc.sitecore.com/xp/en/developers/hd/20/sitecore-headless-development/media-handler.html Firstly, try adding something like this to the whitelist: mw=640,extension=webp In hindsight, we only used webp in the end, so I could have avoided most of the custom code and just duplicated all the whitelist params to add extension=webp versions of each. We also had to customise our frontend next.js app to append the extension=webp querystring (when browser supports it) because however we were generating them before, didn't add it.. If you report back the detail I may be able to offer further suggestions.... |
adding extension=webp queryparam seems to work! Now we need to figure out a way to add this queryparam to the url. We use angular as frontend & the build in image component from sitecore jss. Anyway, thanks a lot already for the help. |
@kpicavet not sure if this will work for you, but in react we are using srcset on the Image and for each of our srcset options we specify both size ( mw:640 ) and extension:webp |
@kpicavet did you get to make this work with angular ? |
Version of Dianoga
6.1.0
Environment description
Sitecore 10.2 XP0 running locally with Headless Services 20.0.2 module installed, running a jss next.js headless site.
Ensure you have some entries in the JSS media resizing whitelist (allowedMediaParams) e.g. mw=640.
What configs you have enabled
Dianoga.NextGenFormats.CDN.config
Dianoga.Strategy.GetMediaStreamSync.config
z.01.Dianoga.NextGenFormats.WebP.config
z.02.Dianoga.NextGenFormats.Avif.config
Reproducible steps (1... 2... 3...) that cause the issue
What you expected to see, versus what you actually saw
I see an optimized resized jpeg, and in the Dianoga logs it says it optimized as a jpeg.
I expect to see an optimized resized webp or avif file depending on the extension querystring.
Relevant logs
No errors, just the expected info logs saying the image was resized into jpeg. For example:
24172 00:22:37 INFO Dianoga: optimized /site/filename.jpg [original size: 1736039 bytes] [final size: 341112 bytes] [saved 1394927 bytes / 80.35%] [Optimized in 685ms] [Extension jpg]
Hi there,
I've raised this issue to document the details of this issue as I am actively investigating and believe I have a fix for this which if it works I will submit a PR or at the least document it below.
Thanks,
Andy
The text was updated successfully, but these errors were encountered: