Thank you!
It's just my assumption, but I don't think it can affect Google Analytics in any way. Prefetching is basically loading the HTML page from the server. Since Google Analytics is a client-side library, it will not run until the page has been rendered. Also, I seriously doubt that Google Analytics treats prefetches in any way, because they can be easily detected. Not only do they use a special link format, but they also include a request header with "Purpose: prefetch". Not to mention, the library itself is built and maintained by Google. If that were the case, they would surely mention it on their repo. I think it is just coincidence.
But, as far as I understand, the library itself (not the Drupal module) can work with the Speculation Rules API. In theory, this could cause an issue because it can also prerender pages, instead of simply preloading, and pre-rendering also means that some JavaScript might be executed. (But I'm not sure; I haven't dug into it too deeply.)
You might want to check the Speculation Rules on the website. They can be in the response headers or on the page.
<script type="speculationrules">…
Speculation-Rules: "…."
If you are using Cloudflare or a similar service, please double-check that. For example, Cloudflare has an option called "Speed Brain" which is essentially the Speculation Rules API.
Knowing that Quicklink (library) can interact with speculative rules, and it can render pages in the background, this could be a problem in theory that affects Google Analytics statistics.
With a given fix, an element is always used from the context instead of a global reference via DrupalSettings.
Hi, I think your concerns are already covered by these settings:
You also have the option to ignore links that use specific CSS selectors. If you hide links with CSS, consider using a class like "visually-hidden" and adding it to this option. In this case, they will not be pre-fetched.
It is also possible to use selectors such as :not(.prefetch) to disable pre-fetching for anything except links with the prefetch class, allowing you to control what should be pre-fetched and what shouldn't. For example, product pages on an e-commerce site.
Also, it is better to set the 'Cache-Control' setting with this module enabled. In this way, the pages that the user has already visited will be pre-fetched from the memory cache instead of making a request to the server.
I agree that these module and library should be used with caution, as they are not a silver bullet and will not magically improve performance. They do have their drawbacks and should be used wisely. If your website consists primarily of static pages without user sessions, this is a great tool to improve overall page responsiveness to user actions. With the right Drupal setup, all these issues are minor for most hosting providers.
Tested locally and it seems to have resolved the issue as well.
niklan → made their first commit to this issue’s fork.
Updated MR and improved data provider.
P.S. Sorry for the delay, I did not receive notification from MR's comments.
Basically, the validation itself is covered by \Drupal\Tests\Core\Theme\Component\ComponentValidatorTest::dataProviderValidatePropsInvalid and ctaTarget violates the allowed properties in the enum. What is not covered is the message itself.
Should it be a dedicated test method for that case or should the \Drupal\Tests\Core\Theme\Component\ComponentValidatorTest::testValidatePropsInvalid assert the exception message as well?
What is the best way to cover it?
Added comment and adjusting issue title
Updated summary and simplified it a little, since there is a failtest and a possible solution exists.
Changed message to be:
[foo:foo/example] Does not have a value in the enumeration ["foo","bar"]. "baz" provided
Well, the pipeline confirms that it fixes a problem and doesn't break anything else (at least what has been tested). On my project, everything works as expected now as well. Honestly, for me, it is hard to explain why this helps.
Just an assumption from debugging: It looks like such components are represented twice in the Twig Node Tree, one of them is some kind of container (wrapper), it doesn't have a payload with variables, but \Drupal\Core\Template\ComponentNodeVisitor::leaveNode still adds a validation function to it, which leads to an exception.
I'm almost given up, but found a major clue. It seems that \Drupal\Core\Template\ComponentNodeVisitor::leaveNode should exit if the node has a parent.
It seems like when you render a node with a parent set, this filter validate_component_props receives a wrong context, which leads to an exception during validation.
I've tested locally, and everything seems to be working fine now.
Did a quick and dirty test with just Twig, and it works fine.
script.php:
<?php
use Twig\Environment;
use Twig\Loader\FilesystemLoader;
$loader = new FilesystemLoader(__DIR__ . '/templates');
$twig = new Environment($loader, [
'cache' => __DIR__ . '/cache',
]);
echo $twig->render('foo.twig');
./templates/foo.twig:
<div class="wrapper">
{% embed 'bar.twig' with {} only %}{% endembed %}
</div>
./templates/bar.twig:
Hello from Bar!
Output:
$ drush scr var/scripts/twig/test.php
<div class="wrapper">
Hello from Bar!
</div>
Thank you!
Added a failtest which highlights the issue as well: https://git.drupalcode.org/issue/drupal-3464719/-/jobs/2292129#L3250
I'm not sure what I should add/change in MR at this point. I can't understand where this bug comes from. It seems to me that it is potentially a bug in Twig itself. I'm not familiar with Twig's internal code, but I think I should try to find out where the template is compiled to find out why it compiles it with a wrong template reference which leads to a recursion.
I just don't know where to approach this task. It's definitely not a ComponentValidator problem. Everything hints and leads to Twig. Basically, that 'example:foo' component tried to be rendered twice, while it is a single call. And the second call comes from a wrong Twig compiled template, which is a little hard to follow and understand, so I want to try to find out where and how these templates are built in the first place.
Maybe I'll try to make a failtest later.
Also, I'm thinking, can this message be improved? I personally don't like the end result right now with [foo:foo] [example]. Maybe it is better to be something like: [provider:component/properties/property] — [foo:foo/properties/example]?
Added condition to append information about value only if it is provided.
Also tried to add a value provided, which will help to find an issue even faster:
Drupal\Core\Render\Component\Exception\InvalidComponentException: [foo:foo] [example] Does not have a value in the enumeration ["foo","bar"]. "baz" provided. in Drupal\Core\Theme\Component\ComponentValidator->validateProps() (line 203 of core/lib/Drupal/Core/Theme/Component/ComponentValidator.php).
Witht the merge request the error message will be:
Drupal\Core\Render\Component\Exception\InvalidComponentException: [foo:foo] [example] Does not have a value in the enumeration ["foo","bar"] in Drupal\Core\Theme\Component\ComponentValidator->validateProps() (line 203 of core/lib/Drupal/Core/Theme/Component/ComponentValidator.php).
Added check for property type definition values, and if it contains a non-string value, throws an exception. In my case, it is now clear where the problem is:
Drupal\Core\Render\Component\Exception\InvalidComponentException: The component "foo:bar" uses non-string types for properties: required. in Drupal\Core\Theme\Component\ComponentValidator->validateDefinition() (line 96 of core/lib/Drupal/Core/Theme/Component/ComponentValidator.php).
$schema: https://git.drupalcode.org/project/drupal/-/raw/HEAD/core/assets/schemas/v1/metadata.schema.json
name: Bar
props:
type: object
properties:
required:
type: [null, boolean]
title: Required
I propose to throw an exception if the types of the property contain values that are not strings, which will include floats and special YAML types. We definitely do not want to correct the value !!float 12.3 in any way. We should throw a meaningful exception, indicating the component ID and property name. Then developers can easily identify and fix the problem.
If we are going to fix NULL for developers automatically, we still have to check for other types and throw an exception. So it's just more code with basically the same result.
Added the test that will fail with such definition. It is practically useless, I think, but it shows that the input and expected data are not the same. If you try to render it, a PHP error occurs. Maybe we should throw an exception in that case, I don't know.
Patch for Drupal 10.3+
It also fails when using libraries with external dependencies, but with a slightly different exception.
Only file JavaScript assets can be optimized.
E.g. of such library:
example:
version: 1
remote: https://example.com
license:
name: Example
url: https://example.com/license
gpl-compatible: true
js:
js/init.js: { }
//example.com/script.js: { preprocess: false }
The suggested solution won't work in this case because the type here is external, which is handled in the method \Drupal\Core\Asset\JsCollectionOptimizer::optimize, but not in the method \Drupal\Core\Asset\JsCollectionOptimizerLazy::optimizeGroup. But for now, I’m out of ideas on how to solve this issue for external libraries. The result of the lazy method should be optimized JavaScript.
For those who have encountered this problem, I suggest explicitly setting the following in the code: {preprocess: false, minified: true} for such JavaScript files.
It's also worth mentioning that the solution from MR and Drupal\Core\Asset\JsCollectionOptimizer::optimize does not include injecting library license information.
I created the MR with the fix and it works with the google_tag module. However, I'm still not sure about this part of the code: $data = file_get_contents($group['items'][0]['data']);. The same code occurs in the methodDrupal\Core\Asset\JsCollectionOptimizer::optimize. It seems that there may be more than one 'item' in the group, and in such a case, some content may be missing. Need some advice here is that safe or not.
I'm facing the same issue. I don't understand the current logic. If the library doesn't want to be preprocessed, why is it automatically minified by default? In that case it makes no sense in preprocess: false.
I believe the issue lies in the logic implemented in two methods: \Drupal\Core\Asset\JsCollectionOptimizerLazy::optimizeGroup and \Drupal\Core\Asset\JsCollectionOptimizer::optimize.
The \Drupal\Core\Asset\JsCollectionOptimizer::optimize respects asset group preprocess status:
if (!$js_group['preprocess']) {
$uri = $js_group['items'][0]['data'];
$js_assets[$order]['data'] = $uri;
}
The \Drupal\Core\Asset\JsCollectionOptimizerLazy::optimizeGroup function optimizes the group regardless of any conditions. It doesn't have a condition and simply moves on to the optimization process.
This method should looks like:
public function optimizeGroup(array $group): string {
$data = '';
$current_license = FALSE;
// No preprocessing, single JS asset: just use the existing URI.
if ($group['type'] === 'file' && !$group['preprocess']) {
$data = file_get_contents($group['items'][0]['data']);
}
else {
foreach ($group['items'] as $js_asset) {
// Ensure license information is available as a comment after
// optimization.
if ($js_asset['license'] !== $current_license) {
$data .= "/* @license " . $js_asset['license']['name'] . " " . $js_asset['license']['url'] . " */\n";
}
$current_license = $js_asset['license'];
// Optimize this JS file, but only if it's not yet minified.
if (isset($js_asset['minified']) && $js_asset['minified']) {
$data .= file_get_contents($js_asset['data']);
}
else {
$data .= $this->optimizer->optimize($js_asset);
}
// Append a ';' and a newline after each JS file to prevent them from
// running together.
$data .= ";\n";
}
}
// Remove unwanted JS code that causes issues.
return $this->optimizer->clean($data);
}->optimizer->clean($data);
}
I encountered the same issue again, and there was a lot of noise in the logs about it. If you just want to fix images and avoid patching the core system, you can use the following script (adapt it to your needs):
#!/usr/bin/env bash
# Fixes iCCP: known incorrect sRGB profile
cd web/sites/default/files || exit
find . -type f -name '*.png' -exec mogrify \{\} \;
This tool will go through all your PNG images and re-save them with fixes for any issues found. However, it fixes more than just the iCCP profile, so use it with caution. I haven't faced any issues yet.
If you know exactly which images have problems, it's best to run the command directly on those files, like this (you can play with broken files from that issue):
mogrify filename.png
I recommend testing this tool locally before applying it to your live files. Alternatively, you could run the entire process locally and then sync the changes using rsync.
Please note that this command may take some time to complete, because it will open and save each image, regardless of whether it has any issues or not. The more and bigger in size files you have, the slower the process will be.
It utilizes the Mogrify binary provided with the ImageMagick package.
Having the same issue.
Call to undefined method Drupal\Core\Entity\Sql\SqlContentEntityStorage::findMatchingRedirect() in Drupal\easy_breadcrumb\EasyBreadcrumbBuilder->getRequestForPath()
I don't understand how it's supposed to work. Previously, the method for the service RedirectRepository was called.
$redirect = \Drupal::service('redirect.repository')
->findMatchingRedirect($redirect_path, [], $this->languageManager->getCurrentLanguage()
->getId());
As mentioned in the original post, the code was changed:
$redirect = $this->entityTypeManager->getStorage('redirect')->findMatchingRedirect($redirect_path, [], $this->languageManager->getCurrentLanguage()->getId());
It loads the storage for the redirect entity and calls ::findMatchingRedirect() on it. Maybe I'm missing something, but the redirect entity doesn't define its own custom storage that would implement that method. In that case, Drupal falls back to the default storage, which is SqlContentEntityStorage for content entity types. But this storage is clearly not implementing that method, which leads to an error.
Merged, thank you!
Thank you.
Hello again. I see some folks not very happy with silencing all the errors from that call. I understand that as well, it can be helpful sometimes. Without it, I won't be found this issue as well. Maybe we wrap this call $image = @$function($this->getSource()); by a condition?
We have already configuration `system.logging` with `error_level` setting. Maybe we will do it like:
if ($error_level === 'verbose') {
$image = $function($this->getSource());
}
else {
$image = @$function($this->getSource());
}
This allows a developer to find out that there is a problem, but it won't be logged and showed on production instances. It might be looking ugly, but it solves most of the concerns here. Currently, as I remember correctly (I've already fixed that images), this will be logged on production on every over page with a broken image when it's processed for the first time (if no style yet created). In some cases, it can create a lot of useless noise in the logging system.
Hi folks, I've just created a similar issue and found this one. I'm not an expert in a11y, but I expected all these field types in the grid should be tabbable. I don't even thought that you should be able to navigate them with the arrow keys. Do they conflict with each other or what? I think all of them should be available by just TABbing other them, no?
I think #3390914 📌 Grid-style field type keyboard navigation Needs work is related directly, but it solves another problem as I see.
This profiling results for drush site:install --existing-config with a specific website configuration. It is not an issue on default profiles (because they are simple and small) or a smaller website (which have fewer configs). But a multilingual website, with something like 10 languages, a lot of content types, fields, etc., configs grows in size very fast, and this is where the issue starts. The profiler is clearly shows, that the more configurations you have, the more calls for Drupal\Core\Config\FileStorage->getAllCollectionNamesHelper() which is bottlenecked by a php::SplFileInfo->isDir() - the unique calls of which is hundreds times fewer than the actual one. In my example, it means for one unique file, it calls this check 587 times.
This issue reproduced everywhere, on different developers PCs and CI's.
I think plugin should be automatic by automatic = TRUE,:
* @EmailAdjuster(
* id = "maillog",
* label = @Translation("Maillog"),
* description = @Translation("Use Maillog handling."),
* weight = 100000,
* automatic = TRUE,
* )
Currently, to make it work, you have to add it manually, and this is stored in configuration, means, that you have to bring maillog to production because of that plugin. By making it automatic, Symfony Mailer will include it for every email and won't affect any configuration changes. Anyway this is controller by Maillog settings, not by adjuster.
It looks like some code will break. Can we decorate that service specifically for installation process? Because I don't think this caching is actually needed for regular runtime process, but it's a very significant improvement for installation. I don't see any cases where configurations are changed on disc during installation, hence, this caching should not do any harm.
This is a very dirty patch to see how tests will react on it.
This is TOP 10 calls during installation after fix applied.
You can see that both, Drupal\Core\Config\FileStorage->getAllCollectionNamesHelper() and php::SplFileInfo->isDir() disappears from the list.
Drupal\Core\Config\FileStorage->getAllCollectionNamesHelper()calls went from 36'392 → 4'551php::SplFileInfo->isDir()calls went from 13'937'541 → 23'710
I measured installation time by time drush site:install --existing-config and results are:
- Before:
real 8m 38.06s - After:
real 7m 9.09s
It's roughly ~15% increase in site installation speed.
Niklan → created an issue.
OK this is a nice simple/safe patch and I believe it fixes the bug. Please can other people test/review?
The patch in #24 contains the test, why doing it manually? You can test with it on all supported Drupal versions by module. Most people coming here are more likely already on Drupal 10.1+, but it still should work on Drupal 10.0- after it released.
I'm not sure, especially considering that PrimitiveBase is an abstract class that extends another abstract class TypedData with exact same methods, and logic inside.
Maybe we should do overwise, remove logic of these methods from TypedData? Just make them simply abstract and empty? Because for now they are simply broken (which test is shows): they assume that $value property is presented, but it is not defined by it, and I don't think it should define it as well. But in this case we have to refactor some other classes which extends TypedData.
For example, \Drupal\Core\Config\Schema\Element extends TypedData directly, because of the problem, it also defines $value property which is makes no sense and just extra code, it should extend PrimitiveBase or provide custom setter and getter, because if TypedData at some point will be changed, it will break. This all a little bit confusing, because TypedData using a property that it doesn't define, and other code define it for it, but we also have PrimitiveBase that does that and duplicate methods as well.
Test that simply shows that it can lead to a problem with undefined $value property.
Niklan → created an issue.
@Mingsong you are right, #24 doesn't cache CSS in any way for 10.1+ workaround (for 10.0- Drupal handles it itself). So it is always generated in runtime for each email (which is also can be bottleneck). It just doesn't sound right to me to save it into cache.render, because, if I want to override it for some reason, it will affect the whole render cache for no reason. I'll prefer to have control over it.
Thank you, @catch, for your input regarding Cache API usage in that case.
I can't see anywhere those content would be cached on disk (file).
Yes, because this logic is moved to \Drupal\system\Controller\AssetControllerBase::deliver() + \Drupal\Core\Asset\AssetDumperUriInterface::dumpToUri() in 10.1
Drupal cache all page contents that have been ever accessed in the database.
The cache.render bin is designed for caching large contents, such as html markup for entire page across the website.
Database in most cases for big Drupal websites is a bottleneck, and introducing additional I/O during email send, which can be in thousands in minute in bulks - waste of resources and slowing database further. If an email has 10 assets, it will create a 10 DB requests per email at minimum.
Also, this is not a cache.render responsibility to store CSS, it is intended to store rendered HTML markup from render array. At least it should be using cache.data bin. If you want to store it in cache, I think it is better to introduce a dedicated bin for that (e.g. cache.symfony_mailer.css), in that case, developers will have control over CSS saved in this bin. For example, I can switch it to cache.backend.null, so each email will generate CSS in runtime every time it sends, or something like cache.backend.php to save it on disk and avoid database usage. At least it can be controlled.
But I still think, if we are going to «cache» result, it should mimic the core behavior on 10.1+ and use asset dumper for that.
I think an aggregated file should be cached on disk, not in database. We should avoid loading database in this case.
Since the aggregated result is exactly the same as it will be for direct request for regular pages, we better to save it under expected filename in asset://css. If someone shares libraries for main theme and email, this will be served for both from the disk.
Basically, I highly against overload database for such things. It actually can be much slower than runtime preparation when website under heavy load and DB is busy.
Could probably check file_exists() and use the old behaviour as long as the aggregate exists. This would mitigate some of the performance impact.
We considered that approach, but the thing is, in most cases (99% as far I can see), it will never exist, because email will probably use their own sets of assets (CSS), rather than main ones. E.g. symfony_mailer/email — it will never will be a part of the main requests, hence, will never be generated.
Also could potentially make a guzzle request to the URL if the file exists in lieue of a method to create a file manually.
Yeah, we also discussed that, but it sounds weird and can lead to other problems. We simply bootstrap Drupal for the task that can be done in the same process — it is wasting time and resources, not to mention that any HTTP problem during request will interrupt email sending. To create an aggregated file, we have to either doing HTTP request (or fake it and rely on other API), either create it manually using other available API. It does not sound very reliable solution. So basically, we have an API to get the result file URI, we have an API to get aggregated file contents, but we have no API for saving this file properly to an assets stream wrapper. We basically have to re-implement part of the logic inside, \Drupal\system\Controller\AssetControllerBase::deliver() which sounds like a lack of API. Even if we're going to implement the manual saving of the file to expected URI if files are not created yet, should we take care about validation and state management which is done inside AssetControllerBase::deliver()?
I want to add my few cents to this. The current CSS/JS aggregation is using similar approach to Image Style functionality. Which means, you can generate a result URI, but the actual file might not exist. Not always images, processed by Image Style, are requested by an HTTP request. For example, when generating some kind of file (PDF, XLSX etc.) that should include an image processed by Drupal, the libraries usually expect a relative path to the image, when they simply check for file_exists() and embed if it does, or skip if a file is missing. They don't make any suggestions how these files are created. And you definitely don't want to provide URLs for that files, which will trigger HTTP request on each image and slow down the process very badly. This is a non-issue for Image Styles, because they provide two public APIs:
\Drupal\image\ImageStyleInterface::buildUri()— build URI, but file might be missing. This is basically how is now CSS/JS aggregation works.\Drupal\image\ImageStyleInterface::createDerivative()— generates an image derivative on request. Calling::buildUri()after that will lead to already existing image processed by Drupal.
So, new CSS/JS aggregation system basically lacks of ::createDerivative() analogue. The main logic is in \Drupal\system\Controller\AssetControllerBase::deliver().
I think \Drupal\Core\Asset\AssetOptimizerInterface should require a new method implementation that will generate that files on demand.
My opinion is that such plugins should be declared final or internal, don't see why anyone should extend it and module maintainers care about that BC for no reason. (it's better to create abstract class to extend, but module implementation should be final anyway) But since it is already public, I can only propose to remove these new dependencies from the constructor and trigger deprecation warning when it called that way and inject them from global container.
public function __construct(array $configuration, $plugin_id, $plugin_definition, AssetResolverInterface $asset_resolver, ?AssetCollectionGrouperInterface $cssCollectionGrouper = NULL, ?AssetCollectionOptimizerInterface $cssCollectionOptimizer = NULL) {
parent::__construct($configuration, $plugin_id, $plugin_definition);
$this->assetResolver = $asset_resolver;
$this->cssInliner = new CssToInlineStyles();
if (!$cssCollectionGrouper) {
@trigger_error('Calling InlineCssEmailAdjuster::__construct without the $cssCollectionGrouper argument is deprecated in drupal/symfony_mailer:1.2.2 and will be required before drupal/symfony_mailer:2.0.0. See https://www.drupal.org/project/symfony_mailer/issues/3371042.', E_USER_DEPRECATED);
$cssCollectionGrouper = \Drupal::service('asset.css.collection_grouper');
}
$this->cssGrouper = $cssCollectionGrouper;
if (!$cssCollectionOptimizer) {
@trigger_error('Calling InlineCssEmailAdjuster::__construct without the $cssCollectionOptimizer argument is deprecated in drupal/symfony_mailer:1.2.2 and will be required before drupal/symfony_mailer:2.0.0. See https://www.drupal.org/project/symfony_mailer/issues/3371042.', E_USER_DEPRECATED);
$cssCollectionOptimizer = \Drupal::service('asset.css.collection_optimizer');
}
$this->cssOptimizer = $cssCollectionOptimizer;
}
public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
return new static(
$configuration,
$plugin_id,
$plugin_definition,
$container->get('asset.resolver'),
);
}
Intentionally removed comments for this comment to make it smaller.
There are other ways around to solve that issue, but all of them require minimal involvement from the developer who extended this plugin.
🎊 #24 results as expected. Without fix test failed on 10.1 but passed on 9.5. It failed with exactly the same problem:
file_get_contents(/vfs://root/sites/simpletest/20840169/files/css/css_HdH2dfVTlI0FmVLOZ7NJXm6Cx2rIsHeSvnEihBvtkrk.css?delta=0&language=en&theme=core&include=eJwrrsxNy8-rjM9NzMxJLYovSS0u0c_My8nMS9VLy88HAMGgDC4): Failed to open stream: No such file or directory
And second patch shows that it fixed now on 10.1 and still works as expected on 10.0.
This patch breaks BC for code that has extended InlineCssEmailAdjuster. Probably we should allow the extra params to be NULL, and supply a default. We can have a deprecation notice for passing NULL.
It's not clear for me in which way it breaks it? If they call parrent::postRender() it should work the same.
Sorry for noise, this commit conflicts with the patch https://git.drupalcode.org/project/symfony_mailer/-/commit/becf7869f4b65... I did the same :)
Update patched so they are applies correctly.
Added tests that attach library with CSS file, which imports another CSS file via @import.
The tests-only patch expected to fail on Drupal 10.1 branch but pass on 10.0. It will show the exact problem for that issue.
The test with fix from #20 should pass on both branches.
This patch basically handles CSS differently for Drupal 10.0- and 10.1+. Let's see how it goes.
Well, this patch (#17) will only work with Drupal 10.1+, even if we are going to use AssetCollectionGrouperInterface and AssetCollectionOptimizerInterface for dependency injection, the Drupal 10.0- doesn't have ::optimizeGroup() method.
Also, \Drupal\Core\Asset\CssCollectionOptimizer::optimize() have different signature for Drupal 10.0 and Drupal 10.1
10.1: public function optimize(array $assets, array $libraries);
10.0: public function optimize(array $assets);
Even if we are to address this, the Drupal 10.1 in that case will result in the same problem, it will prepare aggregated CSS filename, but won't generate it.
I think we have to do what #17 patch does, but provide a backward compatibility layer for 10.0- and remove it when 10.1 will become a minimum supported version by a module.
So, actually, we have a public API for that. So, I've created a new patch which use CSS optimizers and grouper to build a CSS. This also process @import url('other.css') as expected.
Yes, you are right, without aggregation @import url('other.css') won't be included, did my testings, and it works with aggregation.
So, we need to focus on solution #2, but the problem is, it basically lacks of public API. But again, I don't think this is a core bug, this is how it works right now, it is a task of feature request. I'll create a separate issue for that.
Facing same problem after project update to Drupal 10.1. I don't think set preprocess: false is a proper solution, because its possible, that email library will depend on another library which can be provided by a third-party module/theme and this will be a problem to adjust. Also, it makes no sense in that case to use libraries, because if we restrict dependencies - we're basically restricting usage of that, and it's not obvious behavior.
I have two proposal how we can solve it.
Solution #1
\Drupal\symfony_mailer\Plugin\EmailAdjuster\InlineCssEmailAdjuster::postRender in this method, the $this->assetResolver->getCssAssets($assets, TRUE) is called. The second parameter responsible for aggregation and optimization of CSS assets. By passing it as, TRUE it tries to return aggregated file path, but this file should be created by direct request since Drupal 10.1.
The solution is straightforward - pass FALSE instead. In that case, it returns original CSS assets which can be easily processed by file_get_contents($file['data']), because they are always exists as a part of the library. If it doesn't - it's not a module responsibility.
Basically, it is not important to use optimized CSS files by Inline CSS adjuster, because they are not part of email and size is not important. I think this is the simplest and risk-free solution.
Solution #2
This solution assumes that we stick with aggregated assets, in that case we should check the result file for existence first, and if it doesn't exist, well, trigger its creation by doing an HTTP request. It might sound a reasonable approach, but there are few concerns here. First of which - we should do a dedicated HTTP request, basically for no reason. This can create additional load and slow down the emails sending process. Secondly, this can lead to a problem when request is taking too long or even failed, in that case, the email won't be sent, or will be sent without styles (?).
The problem here, that the whole logic of building such file is in \Drupal\system\Controller\AssetControllerBase::deliver method. That means, we either should(not) copy-paste it to avoid HTTP requests, either doing HTTP request or fake it via direct call this method, which is also most like won't be expected to be used in that way.
Seems like it was fixed and available in Peast 1.15.2 release. So I think we have to update this issue to reflect that.
Not sure if we should keep provided test, because it tests third-party library, actually.
P.s. I'm not certain about bumping minimal version, but it sounds reasonable, because otherwise it can break aggregation.
Faced same problem after upgrade on Drupal 10.1.
This
async foo() {}
Become this:
foo(){}
but
foo: async () => {}
works as expected
foo:async()=>{}
Updated patch to not only affect available block list, but configuration of the field itself.
Thank you, available with alpha8
Hi,
No, this module is completely standalone solution which integrates Drupal with https://gwfh.mranftl.com/fonts (https://github.com/majodev/google-webfonts-helper) API.
Thank you.
Niklan → made their first commit to this issue’s fork.
URL was changes, heroku still not working. Thank you.
Thank you, merged.
Thank you, will be available soon as rc6 release.