Hey!
I don't have much time for this by now, therefore I use the MR diff in the meantime.
Still, I checked in the tests if a quick win could be done (Unfortunately, it can take some time).
Nevertheless, some inputs:
By now, all this `escapeFilter()` method is covered by only one test method (`TwigExtensionTest::testEscaping()`) and seems to implement "Happy Path testing anti-pattern", because all the scenarios provided by the `providerTestEscaping` data provider, are based on `path()` twig function. And this twig function always returns a string.
Then, the escape method always pass by `$return = (string) $arg;` path, and then the call to `$env->getRuntime(EscaperRuntime::class)->escape();` with `$arg` or `$return` argument, is pretty identical.. That's why we missed this issue.
Unfortunately, I don't know if we can call a twig function that could return an object. I did not find any in that TwigExtension core class.
Is there a TwigExtension that could be loaded only during tests?
Maybe we can write an anonymous class and load it dynamically in that test method?
Cheers.
julienjoye → created an issue.
@raphaelbertrand Yes no pb. I will do it. Thanks ;)
Hey.
It seems that this fix introduces fatal errors on my Drupal instance (with `10.3.6` update), on every places where I use an `html_attr` escape filter.
Here is the error I get:
The website encountered an unexpected error. Try again later.
Error: Object of class Drupal\Core\Url could not be converted to string in Twig\Template->display() (line 350 of /app/vendor/twig/twig/src/Template.php).
Twig\Template->render(Array) (Line: 35)
Twig\TemplateWrapper->render(Array) (Line: 33)
Solution:
Use
return $env->getRuntime(EscaperRuntime::class)->escape($return, $strategy, $charset, $autoescape);
instead of
return $env->getRuntime(EscaperRuntime::class)->escape($arg, $strategy, $charset, $autoescape);
in `app/core/lib/Drupal/Core/Template/TwigExtension.php`.
I opened a PR on open_social project here:
https://github.com/goalgorilla/open_social/pull/3451
I attached a patch file.
julienjoye → created an issue.