Smarty notice fix on tell-a-friend #31808
Conversation
|
🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷 Introduction for new contributors...
Quick links for reviewers...
|
| ); | ||
| } | ||
| $extensionText[] = '"<div id="tell-a-friend" class="crm-section tell_friend_link-section"> | ||
| <a href="' . $friendURL . '" title="{' . $friendText . '}" class="button"><span><i class="crm-i fa-chevron-right" aria-hidden="true"></i> ' . $friendText . '</span></a> |
There was a problem hiding this comment.
@eileenmcnaughton these { are not right here
There was a problem hiding this comment.
@seamuslee001 - I;ll check but wanted to check in on what you think I should do re purify / escape first
|
arguably this should be moved to the extension but i think we should be escaping the text as part of including it in the PHP String IMO as a defensive measure |
|
@seamuslee001 yeah - I'm just not sure what escaping we should be doing - html_entities on the first instance & purify on the second? |
eileenmcnaughton
force-pushed
the
smart_friend
branch
from
10c165b to
b39846f
Compare
| ); | ||
| } | ||
| $extensionText[] = '"<div id="tell-a-friend" class="crm-section tell_friend_link-section"> | ||
| <a href="' . $friendURL . '" title="' . htmlentities($friendText) . '" class="button"><span><i class="crm-i fa-chevron-right" aria-hidden="true"></i> ' . CRM_Utils_String::purifyHTML($friendText) . '</span></a> |
There was a problem hiding this comment.
Technically, $friendURL also needs htmlentities, but it doesn't really matter in this case.
| <a href="{$friendURL}" title="{$friendText|escape:'html'}" class="button"><span><i class="crm-i fa-chevron-right" aria-hidden="true"></i> {$friendText}</span></a> | ||
| </div><br /><br /> | ||
| {/if} | ||
| {foreach from='extensionText' item='text'} |
There was a problem hiding this comment.
I think it would be more clear if these variables are named extensionHtml and html, instead of extensionText and text. That makes it more clear that the variables should not be escaped, and that the lack of escaping is intentional.
Overview
This addresses smarty notices :
PHP Warning: Undefined array key "friendText" in /.../wp-content/uploads/civicrm/templates_c/en_US/%%10/10E/10E3BF71%%ThankYou.tpl.php on line 24
Before
Notice present
After
Resolved
Technical Details
A secondary issue here is that tell-a-friend has been moved to an extension so it doesn't make sense to ensure the variable is always assigned by core - and indeed it should really be moved. My best idea at the moment is to 'help' the extension to inject it - although there is maybe no good answer - if I go this way though @seamuslee001 - what do you think re escaping / purifying - at the moment escaping is reduced by this patch
Comments