Hi, is this finally going to be addressed within the context of S3FS? I am not sure if it is intended to allow s3fs to be able to somehow manage assets again with the new stream wrapper. If that possibility exists within the module roadmap, it might be interesting to bring this issue back to active status to see if we can push the functionality forward, either by the maintainers or the community itself.
I know that there is a module that provides this functionality, but I am not sure if it is the best approach, and on the other hand if all this would be within the context of S3fs it would be more interesting.
Translated with DeepL.com (free version)
Wrong patch filename, fixing it.
Still a problem in D10.2, attached patch with 10.2 version compatibility.
Uploaded patch in #6 with 10.2 compatibility for projects that rely on this patch, until related Issue is fixed, or a valid solution is found.
Juanjol → changed the visibility of the branch 8.x-1.x to hidden.
Juanjol → created an issue.
Worked for me too! Moving to RTBC!
This is now critical due to the security issue in migrate tools.
Juanjol → changed the visibility of the branch 3405966-other-non-related-sync to active.
Juanjol → changed the visibility of the branch 3405966-other-non-related-sync to hidden.
Juanjol → changed the visibility of the branch 1.0.x to hidden.
@guignonv what do you think? Can we merge this?
Juanjol → created an issue.
The above patch no longer applies to the latest dev versions, here is an update that should work.
After some thought and testing, as far as I can see the solution in #39 🐛 Setting 'Enforce clean and canonical URLs.' breaks CSS aggregation on multilingual Drupal 10.1.x with browser caching enabled RTBC is much cleaner than the one we had previously so far, so with your permission I have removed the MR and leave the latest patch as the proposed solution.
Hi mullzk! Welcome :)
If you have tested the latest patch or the MR changes and they are working for you, please consider changing the issue state from Needs review to 'Reviewed and tested by the community'.
Thanks!
Added patch with changes from MR to facilitate the installation of the patch with composer until the issue is fixed.
I have been rethinking and update a bit the MR so that instead of checking the uri of the request, which contains the base path included and extra parameters, what we check is the path info of the request. This is more similar to what we get from the getDirectoryPath of the asset stream.
I have been rethinking and update a bit the MR so that instead of checking the uri of the request, which contains the base path included and extra parameters, what we check is the path info of the request. This is more similar to what we get from the getDirectoryPath of the asset stream.
I confirm that this Issue is currently still occurring in core version 10.1.5 with Redirect version 8.x-1.9.
I have created an MR to make it easier for maintainers to merge this issue. I have added some of the previous patches to the issue so that the contributors don't lose the cŕedits, I hope I did it right. In this RM I have also corrected a problem from patch #20 with CSS/JS added on sites whose basepath is not /.
What I have done is that when checking the requests not only is done on $assetsStreamWrapper->getDirectoryPath() but also takes into account the basePath of the $request.
I had thought that perhaps it would be more efficient to use a str_starts_with instead of preg_match, but I have seen that it would not be controlled if these requests are expressly CSS/JS and I believe that on this occasion it is interesting to do it this way, isn't it?
Finally, it seems to me that this issue is important because CSS and JS aggregation is failing in lot of sites, so I have raised the priority to Major. I hope that this last one does not generate discomfort.
Juanjol → made their first commit to this issue’s fork.
Updated #35 patch to be compatible with D10.1.5
Looks great! Thanks for sharing.
Juanjol → created an issue.
@anybody Do you have some news from the maintainer?
Tested and working as expected, moved to RTBC.
Please propose changes in MR, don't provide more patches as MR is open.
Is there anything necessary for this issue to be approved or reviewed?
The main approach is to review all image field definitions, review the associated rendering, and from there deduce the clipping being used to generate only the necessary images.
PHP 7.3 will not be supported so I think that the best approach to this is to add constraints to require PHP 8.0 or higher version.
Already fixed.
Thanks but already fixed this.
@eduardo-morales-alberti please review again, there was a typo in composer.json. External entities is covered by security team.
Thanks both by your review. All comments are fixed in 1.0.x branch, please review again!
MR ready, changed to needs review.
Juanjol → created an issue.
Juanjol → created an issue.
Added MR, set to needs review!
It is working fine in Drupal 10 right now, I think this should be RTBC.
Juanjol → created an issue.
Juanjol → created an issue.
Tested and merged.
I have refactored the drush command and added a submodule that decorates the main service in charge of regenerating the derivatives to add the functionality to generate them in webp format. It also has the dependency of the webp module (for now we don't support the webp functionality of the core).