Account created on 16 July 2008, over 16 years ago
#

Merge Requests

Recent comments

🇭🇺Hungary tikaszvince

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?

🇭🇺Hungary tikaszvince

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.

🇭🇺Hungary tikaszvince

Please give more details about this. Which Drupal version do you use?

🇭🇺Hungary tikaszvince

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.

🇭🇺Hungary tikaszvince

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

🇭🇺Hungary tikaszvince

Version 2.x is compatible with Drupal 11.

🇭🇺Hungary tikaszvince

On various cases the original problem still exists.
I try to add more cache tags to built breadcrumb.

🇭🇺Hungary tikaszvince

Hi,
Patch from #10 is fixing this in my case.

🇭🇺Hungary tikaszvince

The latest patch resolved this in my cases, when I tried with Drupal 10.2.7 and 10.3.1

🇭🇺Hungary tikaszvince

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.

🇭🇺Hungary tikaszvince

Hi @Kasey_MK,
Can you check the latest dev version?

🇭🇺Hungary tikaszvince

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

🇭🇺Hungary tikaszvince

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

🇭🇺Hungary tikaszvince

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"

🇭🇺Hungary tikaszvince

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.

🇭🇺Hungary tikaszvince

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.

🇭🇺Hungary tikaszvince

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

🇭🇺Hungary tikaszvince

Feeds and any integration is not the scope of this module.

🇭🇺Hungary tikaszvince

You can use "Bible verse reference - Complex" field type since version 2.1.0.

🇭🇺Hungary tikaszvince

I cannot reproduce this when I use 2.1.1 version and setup a "Bible verse reference - Simple" field.

🇭🇺Hungary tikaszvince

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+?

🇭🇺Hungary tikaszvince

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?
🇭🇺Hungary tikaszvince

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

🇭🇺Hungary tikaszvince

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.

🇭🇺Hungary tikaszvince

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

🇭🇺Hungary tikaszvince

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?

🇭🇺Hungary tikaszvince

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.

🇭🇺Hungary tikaszvince

@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?

🇭🇺Hungary tikaszvince

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?

🇭🇺Hungary tikaszvince

Attached a modified patch which adds a new assert to Drupal\FunctionalTests\Asset\AssetOptimizationTest::assertAggregate to check response always delivers content-type header.

🇭🇺Hungary tikaszvince

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"
    }
  },
🇭🇺Hungary tikaszvince

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.

🇭🇺Hungary tikaszvince

Hi @rudolfbyker,

Is there any chance to create a new, D10 compatible release which contains this fix too?

🇭🇺Hungary tikaszvince

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

🇭🇺Hungary tikaszvince

Yes, i'm using the current version with this patch, and works correctly

🇭🇺Hungary tikaszvince

Hi @rudolfbyker

These are the steps you can reproduce this problem:

  1. Start a new drupal project with composer:
    composer create-project drupal/recommended-project:10.0.9 "install-dir"
  2. Step into "install-dir"
    cd install-dir
  3. 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

Production build 0.71.5 2024