Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass charset to escaper callback again #4088

Closed
wants to merge 1 commit into from
Closed

Conversation

fritzmg
Copy link

@fritzmg fritzmg commented May 12, 2024

Fixes #4087

This restores the behaviour of 3.9.x where the $charset was passed to the escaper callback.

@@ -132,7 +132,7 @@ public function setEscaper($strategy, callable $callable)

$this->escapers[$strategy] = $callable;
$callable = function ($string, $charset) use ($callable) {
return $callable($this->environment, $string);
return $callable($this->environment, $string, $this->environment->getCharset());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was indeed a mistake. I don't think we should restore this odd behavior as the callables never got the charset before (so without changing the escapers, the charset is not useful). And as adding the charset does not make migrating easier, I don't think we should support it.

Copy link
Author

@fritzmg fritzmg May 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as the callables never got the charset before

Not sure what you mean? The callables did get the charset before:

return $escapers[$strategy]($env, $string, $charset);

That's the BC break.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'm stupid sometimes. See #4091 for the fix. Here, we need the charset passed to the escape filter, not the default charset.

@fritzmg fritzmg changed the title Parse charset to escaper callback again Pass charset to escaper callback again May 12, 2024
@fabpot fabpot closed this in f553f16 May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Possible BC break with EscaperExtension in 3.10.x
2 participants