-
Notifications
You must be signed in to change notification settings - Fork 29
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
Update leading to different base64 encoding in picture srcset #146
Comments
|
Could you please provide steps to reproduce this? Is this happening on SSG generated pages or without SSG? |
Hi @ncla, Sorry for the late reply. It is happening on SSG generated pages. As long as nobody runs into my issue, I would need to create a fresh project. |
It is possible that this could occur again at random. The placeholder fetching can fail silently, however only if it's a It is also possible that it is a bug in ssg package. I am experiencing image issues as well in one of my projects. statamic/ssg#91 When you ran this test, did you try running ssg:generate multiple times? |
I ran My project is quite different from the test setup. I customised the ssg process (different commands and workflow). I did not touch the image handling though. Now I am thinking about carefully moving the project's code into the test setup. |
Side note: I found out that the error shown in the first post is not restricted to the ssg output.
|
As you mentioned, I am touching the exception block for the images in question, if I do not clear the glide cache beforehand. statamic-responsive-images/src/Breakpoint.php Line 270 in 6aa7591
|
In the SSG issue statamic/ssg#91 I wrote this:
Which sounds like your issue:
In the 2nd step, my ssg:generate actually has failed on regular Glide manipulation too (that are not using |
CMS 3.3.8 version introduced some new Glide cache stuff (statamic/cms#5725) which could make this code section outdated in Breakpoint.php. https://github.com/spatie/statamic-responsive-images/blob/main/src/Breakpoint.php#L262-L272 It has not been updated since 3.3.8. Maybe there is more correct, up-to-date way of fetching an image from cache. try {
$source = base64_encode($server->getCache()->read($path));
$cache = $server->getCache();
$mimetype = method_exists($cache, 'getMimetype')
? $cache->getMimetype($path)
: $cache->mimeType($path);
$base64Placeholder = "data:{$mimetype};base64,{$source}";
} catch (FileNotFoundException $e) {
return '';
} It is possible that this section actually is buggy since 3.3.8. |
Yes, you are right. Nothing is done to mitigate the outcome this exception is leading to (invalid base64 string). |
I have used one of my addons to overwrite (alias) the Breakpoint.class. Condition
Return empty strings here:
Now the faulty 32w is missing in the srcset and the other images are rendered. I also found out that There is a lot of testing necessary now. |
So if I understood correctly, getting rid of placeholder yielded consistent error free output?
It would be great if you could open separate issue for this. One issue at a time. :) |
Yes, your are correct. |
Unfortunately the issue I linked previously in this conversation to statamic/ssg#91 might not be relevant to this issue, which I thought it could be. My issue stemmed from Glide tag pairs instead, see issue I opened here: statamic/ssg#110 So we are back to where we were before: could you try and make a reproducible repository with this bug? Then from there I can take a look. You said you had some custom SSG generation stuff, which obviously makes things complicated. |
Hey, is there any updates on this? |
Hi @ncla, Sorry that I did not comment on your last post. I managed to temporarily fix my problem with the above solution: There was no more time left for a detailed investigation. |
I am going to close the issue, as it probably has something to do with my setup. Thank you very much for looking into this! |
Hey @globalexport, I think the latest update / pull request should address the issue you had and you may remove your work around. You were hitting that exception because you are on older Laravel framework version, while I was on newer - that's why I couldn't reproduce this issue. 🙈 I failed to notice this. Newer Laravel versions has some changes to Flysystem which changed some Glide stuff. You can read the #171 issue and and #174 PR, but essentially the issue was that placeholder generation was reading cached manipulated image that didn't exist in the cache storage. This storage changes location depending on the That's why |
Hi @ncla, Thanks a lot for the explanation! |
Hi @ncla, It is looking very well on one of the smaller projects. Thanks a lot! |
Hi,
It seems like there is a base64 string missing after updating this plugin (and other parts).
Has such a result been observed by somebody else?
The text was updated successfully, but these errors were encountered: