Problem/Motivation

This is happening on every page load:

    public function setDefaultStrategy($defaultStrategy)
    {
        // for BC
        if (true === $defaultStrategy) {
            @trigger_error('Using "true" as the default strategy is deprecated. Use "html" instead.', E_USER_DEPRECATED);

Proposed resolution

Use 'html'

Remaining tasks

Review patch

User interface changes

N/A

API changes

N/A

Data model changes

N/A

Beta evaluation

Major bug because we are going into the error handler for a deprecation notice on every request

CommentFileSizeAuthor
#3 interdiff.txt1008 bytesstar-szr
#3 twig_autoescape_default-2567139-3.patch1.56 KBstar-szr
twig-autoescape-html.patch593 bytesstefan.r

Comments

stefan.r created an issue. See original summary.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Needs work

+1 to this. Twig's default is this as well so makes sense to change this.

class Twig_Environment {
    public function __construct(Twig_LoaderInterface $loader = null, $options = array())
    ...
        $options = array_merge(array(
           ...
            'autoescape' => 'html',
            ...
        ), $options);
}

Can we ensure that TwigExtension::escapeFilter()'s last parameter is also using this?

and TwigExtensionTest::testEscaping() and TwigExtensionTest::testSafeStringEscaping()

star-szr’s picture

star-szr
he/him
Primary language English
commented
Status: Needs work » Needs review
Issue tags: +Twig
Parent issue: » #2555243: Upgrade path / plan to Twig 2.x aka 2.0
StatusFileSize
new twig_autoescape_default-2567139-3.patch1.56 KB
new interdiff.txt1008 bytes

Thanks for this @stefan.r, I found this too as part of #2555243: Upgrade path / plan to Twig 2.x aka 2.0.

Here's the other ones @joelpittet mentioned. I think escapeFilter() is different isn't it? It's not $strategy, the docs say:

* @param bool $autoescape
* Whether the function is called by the auto-escaping feature (TRUE) or by
* the developer (FALSE).

Which is the same as what the upstream docs say plus/minus some capitalization ;)

Leaving that one alone for now.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Reviewed & tested by the community

You're right @Cottser ignore ::escapeFilter(), the strategy is got elsewhere.

This is a nice fix to keep this in sync with upstream and prevent setDefaultStrategy from triggering the deprecation notice.

xjm’s picture

xjm
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Nice find. Committed and pushed to 8.0.x. Thanks!

  • xjm committed 86bc1ce on 8.0.x 
    Issue #2567139 by Cottser, stefan.r, joelpittet: Twig autoescape default...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.