dev/core#4146 Remove (old) Smarty-forward incompatible syntax from Address.tpl

[eileenmcnaughton]

Overview

dev/core#4146 Remove (old) Smarty-forward incompatible syntax from Address.tpl

Before

Use of ` in the template is not forward compatible - see https://lab.civicrm.org/dev/core/-/issues/4146

After

The url on map still has location type id (lid - civicrm/contact/map?reset=1&cid=203&lid=1)

Technical Details

To r-run this I had to enable google geocoding with fake credentials AND add a fake geocode for the address

Comments

[civibot]

No issue was found matching the number given in the pull request title. Please check the issue number.

[civibot]

(Standard links)

• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[demeritcowboy]

I'll give this a quick run with a real geocode I just want to understand what is incompatible:

As far as I know this is not "old code" and in "modern" smarty the backticks still serve a purpose to allow e.g. putting expressions with a . in them inside quotes. So I'm wondering if the incompatibility is something introduced by the escape-by-default code. Having said that, I've always found the backticks confusing and breaking it out into an intermediate variable seems fine.

[eileenmcnaughton]

@demeritcowboy it hard fails on Smarty3

[demeritcowboy]

It shouldn't, according to smarty docs. Are you using escape-by-default with it?

[eileenmcnaughton]

@demeritcowboy hmm - it's toggling between the 2 smarty versions, not changing escape by default that matters. I think v3 is always escape by default

[demeritcowboy]

It's just given all the other things that need fixing it would be good to determine if this is actually something that needs changing and what the incompatibility is caused by, since I expect there's a lot of backticks. Is it something like templates_c just needs clearing between toggling, because it compiles differently?

[eileenmcnaughton]

@demeritcowboy it wasn't my impression that it was a common syntax - if you want to try firing up Smartyv3 it is possible with WIP patches + an extension & a define (you can toggle back & forth on the define) https://lab.civicrm.org/dev/core/-/issues/4146

[demeritcowboy]

grep -r "`" templates | wc -l
gives 280 possible instances. So not insane but it takes up review time too.
I'll try it if I get a chance but no promises. If you already have the fail error message that might be useful.

[eileenmcnaughton]

@demeritcowboy I'll grap some screenshots of the fatal error next time