tikaszvince → made their first commit to this issue’s fork.
tikaszvince → created an issue.
I believe that exception you got is not related the TinyPNG module's route subscriber.
I'm trying to reproduce with a clean Drupal 10.3.6 installation and I cannot reproduce this. What was the original exception that the AuthenticationSubscriber caught?
We know about the plugin system.
The idea behind our module is that calling the TinyPNG API should be the very last step.
The optimal solution would be to catch that point when a new image derivative has been created. Currently there is no way to catch this. There are some Drupal core issues about this problem, i.e. ✨ Make possible to respond to image derivative creation Needs work ✨ Split ImageStyle into the config entity and a separate event-based image processing service Needs work .
When this approach will be possible we will replace the route altering way.
Please give more details about this. Which Drupal version do you use?
You request to make possible to modify the previously uploaded files. Anything can go wrong during the TinyPNG API communication.
And if an error is not handled, or if we overwrite an existing file with incorrect data it can lead to data loss.
I cannot take responsibility for this possibilities, so even if the patch or MR will be created that will be no part of this module.
I suggest to create a separated module to implement this feature.
It is not scope of this module to batch process existing images.
There are plenty of solutions for bulk process files, which you can find here: https://tinypng.com/third-party#development
Version 2.x is compatible with Drupal 11.
On various cases the original problem still exists.
I try to add more cache tags to built breadcrumb.
tikaszvince → created an issue.
It looks good.
Thank you
Hi,
Patch from #10 is fixing this in my case.
I've created a merge request with the requested changes.
tikaszvince → created an issue.
tikaszvince → created an issue.
tikaszvince → created an issue.
The latest patch resolved this in my cases, when I tried with Drupal 10.2.7 and 10.3.1
Updated patch for 10.3.1
I started the new 2.x branch.
The newly released 1.4 version contains only the changes to mark that branch is compatible to <=10.2.
For Drupal 10.3 TinyPng 2.x branch is the recommended.
Hi @Kasey_MK,
Can you check the latest dev version?
Thank you,
Can you check if this patch with Drupal 10.0, 10.1 and 10.2?
If it not I have to create a new major version wich marked compatible from 10.3 and mark the current version ot TinyPng not working with 10.3
Hi @johnv,
Any chance of creating a new release with these fixes in the next few days?
Fixed in dev version
All good now! I close this because all of my projects using this module the updates report page not reporting as a "Non supported" module.
Thank you
Hi @tamarpe,
Thank you the new release! On the project page the release appears now. But I have to reopen this issue for the following reasons
But on the updates report page still says this module and the installed version is "Not supported".
I don't know if it is releated, but when I click on the "Source code" link from the sidebar of the project page, it points to the URL https://git.drupalcode.org/project/issuu_viewer but it redirects to a tree URL which alerts "The default branch of this project clashes with another ref"
Why is this patch was necessary to create? I've already linked the RTBCd 📌 Automated Drupal 10 compatibility fixes Fixed issue containing a patch with the same changes.
tikaszvince → created an issue.
tikaszvince → created an issue.
tikaszvince → created an issue.
Thank you
Ok, thank you for the help!
Since we found the root cause of my original problem and we have a solution for it i think this is not a bug anymore.
You can always implement hook_bibleref_format_data()
Also the format strings are translatable strings, so you can always provide a translation with the context bibleref
Feeds and any integration is not the scope of this module.
You can use "Bible verse reference - Complex" field type since version 2.1.0.
I cannot reproduce this when I use 2.1.1 version and setup a "Bible verse reference - Simple" field.
tikaszvince → created an issue.
Fixed in released 2.1.1 →
Thank you the report!
Please check new dev version.
I've just attached an updated version of my patch from #5 to handle error on Drupal10.1 + PHP8.1 caused by INF passed as max_nesting_level
.
---
Hey @markdorison,
Is there any news around how we can upgrade to Commonmark v2.4+?
tikaszvince → created an issue.
I think I found the cause of this strange behaviour. Our CSS contains this line
@import url('https://fonts.googleapis.com/css2?family=Fraunces:opsz,wght@9..144,400;9..144,600&display=swap');
When I remove this line and add the URL to the library as an external asset the CSS aggregation just works fine. So I think this is an other issue 🐛 CssOptimizer silently drop the attached library when a CSS file @include an external CSS. Postponed: needs info .
About my original question: Is it possible to handle other typed of media than "all" and "print"? Is it possible to create a separated aggregated file if a library defined a media query with the asset definition?tikaszvince → created an issue.
Still the same.
This is how my project filesystem looks like after clearing the cache, also the library definition visible on this screenshot.
This is how the page appears in the browser when i load it after a cache clear:
After I see the page only the lazy-optimize.txt file appears and the aggregated CSS files also created.
And this is how the page appears in the browser when I disable CSS aggregation clear the cache and load the page
On cache clear (no matter the method) the sites/default/files/css
directory is deleted. When i reload the page in the browser, the same files with the same name and content placed.
If i load the same page with curl after a cache clear (to make sure we generate only the HTML output and prevent downloading any of the assets) the CSS directory does not created. If I download one of the CSS assets linked to header the CSS directory and the requested file is appeared.
I've tried cache clear with varios ways, drush cr
, also drush cc drush && drush cc theme-registry && drush cc css-js && drush cc render
and as I mentioned before from the admin toolbar, hitting the "Flush all caches" menu item, and also tried the "Clear all caches" button on /admin/config/development/performance
page.
In my settings.local.php
I'm using null cache backend for render and dynamic page cache. In live environment we use Redis cache backend.
I'm using Drupal core 10.1.1 with your latest patch from 🐛 When AssetControllerBase delivers existing file should add content-type Fixed and with Drush 12.1.2. No advagg module installed.
Installed extensions implementing hook_library_info_build
: dropzonejs
Installed extensions implementing hook_library_info_alter
: entity_browser_enhanced, locale, gin_toolbar, dropzonejs, field_group, system, image_widget_crop, ckeditor5, responsive_image
Installed extensions implementing hook_css_alter
: gin_toolbar, gin
I've simplified my library definition in the theme.
global-styling: css: theme: assets/css/style-00-base.css: weight: 0 assets/css/style-01-tablet.css: weight: 2 assets/css/style-02-desktop.css: weight: 3 assets/css/style-03-large.css: weight: 4
My theme info file contains
libraries: - [MY_THEME]/global-styling
When CSS aggregation is disabled via settings.php $config['system.performance']['css']['preprocess'] = FALSE;
. All the 4 css file is loaded on the page and the styles are applied on elements.
When I enforce CSS aggregation and load the same page The HTML document contains only 2 link element
https://[...]/sites/default/files/css/css_U1alzjUJOwF2aPk0mkaT8DevuUlYGLTwyS9VaScX9J0.css?delta=0&language=hu&theme=[...]&include=eJyNkFluwzAMRC-kWEcSGJu15XJRKdmpc_oyUYEUrT_6o2U4BOeRYT7ABAppukLFiDvacVvQMNSjNuT4kAPLs5zagoxnEmOtMGMNoxpGUWOgfMf-nWwrQEMxnc19j9beBSt8Bv6dgXVCk3y3l3EmvQJdajsoy_zSFwT3_vBpalr61LVeRtX3jAG31F9-caEMMjrpifh_pye1b7oszeM63_qx-fKGN4cPe8Zbjc9zcJ6N8C8nyp5NhVFaAMkMDadUR1Mip4gqqfhKEylMZ_Xc61-1h7Db https://[...]/sites/default/files/css/css_pV_APjWugivh9z5_eKDEzV5JFUx6aylPuzZCI6_7kW8.css?delta=1&language=hu&theme=[...]&include=eJyNkFluwzAMRC-kWEcSGJu15XJRKdmpc_oyUYEUrT_6o2U4BOeRYT7ABAppukLFiDvacVvQMNSjNuT4kAPLs5zagoxnEmOtMGMNoxpGUWOgfMf-nWwrQEMxnc19j9beBSt8Bv6dgXVCk3y3l3EmvQJdajsoy_zSFwT3_vBpalr61LVeRtX3jAG31F9-caEMMjrpifh_pye1b7oszeM63_qx-fKGN4cPe8Zbjc9zcJ6N8C8nyp5NhVFaAMkMDadUR1Mip4gqqfhKEylMZ_Xc61-1h7Db
Both redirected with HTTP 302 to self
https://[...]/sites/default/files/css/css_U1alzjUJOwF2aPk0mkaT8DevuUlYGLTwyS9VaScX9J0.css?delta=0&language=hu&theme=[...]&include=eJyNkFluwzAMRC-kWEcSGJu15XJRKdmpc_oyUYEUrT_6o2U4BOeRYT7ABAppukLFiDvacVvQMNSjNuT4kAPLs5zagoxnEmOtMGMNoxpGUWOgfMf-nWwrQEMxnc19j9beBSt8Bv6dgXVCk3y3l3EmvQJdajsoy_zSFwT3_vBpalr61LVeRtX3jAG31F9-caEMMjrpifh_pye1b7oszeM63_qx-fKGN4cPe8Zbjc9zcJ6N8C8nyp5NhVFaAMkMDadUR1Mip4gqqfhKEylMZ_Xc61-1h7Db https://[...]/sites/default/files/css/css_pV_APjWugivh9z5_eKDEzV5JFUx6aylPuzZCI6_7kW8.css?delta=1&language=hu&theme=[...]&include=eJyNkFluwzAMRC-kWEcSGJu15XJRKdmpc_oyUYEUrT_6o2U4BOeRYT7ABAppukLFiDvacVvQMNSjNuT4kAPLs5zagoxnEmOtMGMNoxpGUWOgfMf-nWwrQEMxnc19j9beBSt8Bv6dgXVCk3y3l3EmvQJdajsoy_zSFwT3_vBpalr61LVeRtX3jAG31F9-caEMMjrpifh_pye1b7oszeM63_qx-fKGN4cPe8Zbjc9zcJ6N8C8nyp5NhVFaAMkMDadUR1Mip4gqqfhKEylMZ_Xc61-1h7Db
The new request to the redirected URL is served with HTTP status 200.
It seems the AssetControllerBase::deliver does not called at all, the include_string.txt
does not created. (I make a syntax error on that file and it does not throw an error).
I placed a dump on these places: AssetControllerBase::deliver CssCollectionOptimizerLazy::optimize, CssCollectionOptimizerLazy::optimizeGroup and CssOptimizer::optimize methods to dump the arguments to different files. Patch attached.
Only the CssCollectionOptimizerLazy::optimize created any file, so i think only this method was called. This dump contains all the CSS files defined by the library.
I'm using Apache with these rewrite rules
RewriteEngine on RewriteBase / RewriteCond %{REQUEST_FILENAME} !-f RewriteCond %{REQUEST_FILENAME} !-d RewriteCond %{REQUEST_URI} !=/favicon.ico RewriteRule ^ index.php [L]
I'm not allowed to attach project specific codes and examples here. I understand that every project has its own constellation and it is possible our code messing up something. What should I check to figure this out?
I'm understand the intention to reduce the number of HTTP queries. Overall i think the current implementation does not allow us to differentiate between assets, or allow fine tuning how we want to group and/or link CSS files.
Currently if i run without my patch, and apply the latest patch from 🐛 Ensure that edge caches are busted on deployments for css/js aggregates Fixed on 10.1.1 and revert my library definition to this:
global-styling: css: theme: assets/css/style-00-base.css: weight: 0 assets/css/style-01-tablet.css: media: 'all and (min-width: 768px)' weight: 2 assets/css/style-02-desktop.css: media: 'all and (min-width: 992px)' weight: 3 assets/css/style-03-large.css: media: 'all and (min-width: 1440px)' weight: 4
none of these CSS files or any part of its content delivered to client. Not as a single file or part of any aggregated CSS file.
This defines 4 CSS files (I removed the sourceMap comment from the compiled css files). Two contains additional @media
queries the other two does not. These files are not containing any syntax error. One contains a direct import from Google Fonts.
My first thought was it must be some error in the compiled CSS files which the optimizer cannot handle. But if i replace the content of these files one simple rule (body {background: red;}
) still skipped, so i think the content of these files are not count in this situation.
And there is nothing in the watchdog which could help me why these files are skipped from the aggregation at al.
@catch Yesterday I was in a hurry to fix this on live project as soon as possible. I went forward step-by-step. First I've tried to add preprocess: true, than add my media query as an attribute, than I tried to mark these css files as minified (we compile these files from SCSS as compressed as possible). None of these changes created any link
element with the required media query attribute. Currently this is the combination which works for me: the attached patch, and the preprocess: false
together produces the required link
elements with the required media
attributes.
If i revert my patch, and revert my library definition to contains only weight and the media queries (e.g. media: 'all and (min-width: 768px)'
) none of these CSS files delivered to the client. No separated, no aggregated, no part of an aggregated file.
I couldn't find any explanation yet but sometimes some CSS file is just missing from the aggregated files, sometimes whole libraries skipped.
And one more weird behaviour which i found and i don't know if it is related or not. Before 10.1 when I change something in my CSS than clear the cache the new aggregated CSS created from the new files. This worked without any additional step, no version string or variable change was needed to get the CSS changes to work. After 10.1 I'm this method does not work. I've tried to clear the cache with drush, from the admin menu, full cache rebuild, or step by step (flus CSS/JS cache, theme cache, render cache, etc). I've tried to delete css
directory, but the changes does not appeared in the aggregated CSS assets.
So overall i think we have a very severe regression I cannot understand. Why Drupal thinks there is only two type of media (print or all) when the specification says media attribute can be any media type or media query?
tikaszvince → created an issue.
I think this is already fixed in 🐛 When AssetControllerBase delivers existing file should add content-type Fixed
How do you exactly use these modules?
What is your TinyPNG module (/admin/config/tinypng
) configuration? Is the "Compress on upload" enabled? Is "Integration Method" value is "Download" or "Upload"?
Is your File field path is configured to use the private://
path?
How your image field exactly configured?
When does this error usually appears?
reroll patch to make possible to apply on 2.36
Sure. Updated patch is attached.
Attached a modified patch which adds a new assert to Drupal\FunctionalTests\Asset\AssetOptimizationTest::assertAggregate
to check response always delivers content-type
header.
tikaszvince → created an issue.
My workaround for this is to add the autoload lines to my project's composer.json
The path should point to the proper place of bootstrap/src
"autoload": { "psr-4": { "Drupal\\bootstrap\\": "www/themes/bootstrap/src" } },
I had=ve a problem with the approach checking which class exists. The Commonmark Environment should be constructed in different way with version 2.0 and there were some changes since 2.0 to 2.4. So i had to check for the version string.
The more i think about this the more I'm convinced about the proper approach should be a different @MarkdownParser
for newer Commonmark versions.
My patch also uses the \ReflectionObject
to merge settings which is against the intention of the Commonmark, which triggers a deprecation warning too when you call merge()
.
I'm using the markdown module and Commonmark for convert texts from an API and store them in a formatted text field. In my case these were the required changes to make my integration code work after upgrades.
Hi @rudolfbyker,
Is there any chance to create a new, D10 compatible release which contains this fix too?
Hi,
I have a very minimal patch, to prevent error on converting when we want to use commonmark 2.4.
This patch can apply on 3.0.0-rc2 version of markdown module
Attach updated patch for version 2.0.1 →
Yes, i'm using the current version with this patch, and works correctly
Hi @rudolfbyker
These are the steps you can reproduce this problem:
-
Start a new drupal project with composer:
composer create-project drupal/recommended-project:10.0.9 "install-dir"
-
Step into "install-dir"
cd install-dir
-
Try to install oai_pmh_harvester with composer
composer require drupal/oai_pmh_harvester
composer will report about requirement version mismatch
Your requirements could not be resolved to an installable set of packages. Problem 1 - drupal/oai_pmh_harvester 2.0.0 requires drupal/core ^8 || ^9 -> found drupal/core[8.0.0, ..., 8.9.20, 9.0.0, ..., 9.5.9] but the package is fixed to 10.0.9 (lock file version) by a partial update and that version does not match. Make sure you list it as an argument for the update command. - drupal/oai_pmh_harvester[2.0.1, ..., 2.0.2] require guzzlehttp/guzzle ^6.3 -> found guzzlehttp/guzzle[6.3.0, ..., 6.5.8] but the package is fixed to 7.5.1 (lock file version) by a partial update and that version does not match. Make sure you list it as an argument for the update command. - Root composer.json requires drupal/oai_pmh_harvester * -> satisfiable by drupal/oai_pmh_harvester[2.0.0, 2.0.1, 2.0.2].
As this reports says oai_pmh_harvester composer.json says it requires "guzzlehttp/guzzle": "^6.3"
But Drupal core requiers ^7.5
version.
I think since the used "caseyamcl/phpoaipmh" packages requires guzzlehttp/guzzle you do not need to lock this requirement at all.
So my suggestion is remove this requirement from composer or change the required version to match both Drupal and phpoapmh requirements
tikaszvince → created an issue.