(dev/core#1328) Drop support for CIVICRM_SMARTY_DEFAULT_ESCAPE

[totten]

Overview

The constant CIVICRM_SMARTY_DEFAULT_ESCAPE provided a discreet opt-in with the ambition to invert the Smarty escaping behavior (e.g. preferring to default to HTML-encoding all variables; rather defaulting to passthrough variables). It arose an offshoot from https://lab.civicrm.org/dev/core/-/work_items/1328). However, as later discussions found, it's really hard to design a meaningful transition plan with a single/global switch. So newer work is moving toward per-file opt-in.

Which leaves the question - do we still need CIVICRM_SMARTY_DEFAULT_ESCAPE?

(Note: This relates to test-failures in the proposed #36121.)

Before

Setting CIVICRM_SMARTY_DEFAULT_ESCAPE=true does... something to your system.

After

Setting CIVICRM_SMARTY_DEFAULT_ESCAPE does nothing. All systems have the same (default) escaping mechanics.

Comments

• My sense was that we had @colemanw @eileenmcnaughton @seamuslee001 on board with move to per-file model. But I don't recall if it's been discussed with @demeritcowboy or other interested parties.
• I think it's going to be easier to comprehend slimmer system. But if someone has a reason this needs to stay longer, then... OK. 🤷
• The basic problem with a global switch is this: site-builders toggle the switch; and when they toggle, it shouldn't break with a random mix of core+contrib code. This basically means that developers (for that core+contrib code) must maintain each Smarty TPL in a way that supports both modes of execution. But dual-mode-TPLs are picky/complex/counter-intuitive; the notations become pretty wonky. (See (DEMO) smartyup - Demonstration of auto-updating *.tpls #33623)
• Patch application note -- If your system had the opt-in for CIVICRM_SMARTY_DEFAULT_ESCAPE before, and if you apply the patch, then... you should run cv flush to clear-out templates_c. (If someone thinks this is too onerous upgraders... then I could post something like https://gist.github.com/totten/51543aeadd1961455ae12506c95904bf.)

[civibot]

🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷

Introduction for new contributors...

• If this is your first PR, an admin will greenlight automated testing with the command ok to test or add to whitelist.
• A series of tests will automatically run. You can see the results at the bottom of this page (if there are any problems, it will include a link to see what went wrong).
• A demo site will be built where anyone can try out a version of CiviCRM that includes your changes.
• If this process needs to be repeated, an admin will issue the command test this please to rerun tests and build a new demo site.
• Before this PR can be merged, it needs to be reviewed. Please keep in mind that reviewers are volunteers, and their response time can vary from a few hours to a few weeks depending on their availability and their knowledge of this particular part of CiviCRM.
• A great way to speed up this process is to "trade reviews" with someone - find an open PR that you feel able to review, and leave a comment like "I'm reviewing this now, could you please review mine?" (include a link to yours). You don't have to wait for a response to get started (and you don't have to stop at one!) the more you review, the faster this process goes for everyone 😄
• To ensure that you are credited properly in the final release notes, please add yourself to contributor-key.yml
• For more information about contributing, see CONTRIBUTING.md.

PR commands & links...

• /rebase <branch-name> will rebase your branch and change the base of the PR.
• /squash will combine all commits (keeping only the first commit messsage).
• /port <branch-name> will create a copy of this PR against a different branch.
• /lintroll will automatically fix linting errors, amending commits as needed.
• retest this please will rerun the tests and rebuild the demo site.
• 📖 Review standards
• 🗒️ Review template (brief or verbose)

➡️ Online demo of this PR 🔗

[colemanw]

@totten all 5 test failures are the same:

E2E\Core\SmartyConsistencyTest
Error: Call to a member function execute() on null

/home/homer/buildkit/build/build-4/web/sites/all/modules/civicrm/tests/phpunit/E2E/Core/SmartyConsistencyTest.php:366
/home/homer/buildkit/build/build-4/web/sites/all/modules/civicrm/tests/phpunit/E2E/Core/SmartyConsistencyTest.php:436
/home/homer/buildkit/build/build-4/web/sites/all/modules/civicrm/tests/phpunit/E2E/Core/SmartyConsistencyTest.php:118

[demeritcowboy]

I'm personally ok with a different strategy.

[totten]

...all 5 test failures are the same:

E2E\Core\SmartyConsistencyTest...

Squashed with more aggressive dial-down on the test.