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

Import - Fix warning on "Match Fields" step #23905

Closed
wants to merge 2 commits into from

Conversation

totten
Copy link
Member

@totten totten commented Jun 29, 2022

Overview

Fix PHP warning.

Before

Screenshot from 2022-06-29 00-10-58

After

No warning

@civibot civibot bot added the 5.51 label Jun 29, 2022
@civibot
Copy link

civibot bot commented Jun 29, 2022

(Standard links)

@totten totten marked this pull request as ready for review June 29, 2022 07:15
@@ -28,7 +28,7 @@ if ( document.getElementsByName("saveMapping")[0].checked ) {
document.getElementsByName("saveMapping")[0].checked = false;
}
{/literal}
{if $isCheked}
{if isset($isCheked) && $isCheked}
Copy link
Member Author

Choose a reason for hiding this comment

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

Believe it or not, the variable is spilled consistently.

Copy link
Contributor

Choose a reason for hiding this comment

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

that won't work :-( it will cause a fatal with smarty is secure mode :-(

I broke my brain on that particular notice so I was really excited when I saw you had tried but this isn't the answer (I concluded re-writing the js probably was)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, the life on that flag is weird.

Is there a configuration flag that puts into the non-working mode?

Copy link
Contributor

@eileenmcnaughton eileenmcnaughton Jun 29, 2022

Choose a reason for hiding this comment

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

/**
 * Smarty escape on output.
 *
 * This tells smarty to pass all variables through the escape function
 * unless they are piped to smarty:nodefaults (eg. {$myScript|smarty:nodefaults}
 * At this stage it should only be enabled on development sites.
 * @see https://github.com/civicrm/civicrm-core/pull/21935
 */
//if (!defined('CIVICRM_SMARTY_DEFAULT_ESCAPE')) {
//  define('CIVICRM_SMARTY_DEFAULT_ESCAPE', TRUE);
//}

Copy link
Member Author

Choose a reason for hiding this comment

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

You've probably looked at this, but just to verbalize my read... When using CIVICRM_SMARTY_DEFAULT_ESCAPE, it registers a "default modifier" with CRM_Core_Smarty::escape(..."htmlall"). But it's quite zealous in applying the default modifier:

<!-- Given input... -->
{if $isCheked}
<!-- With CIVICRM_SMARTY_DEFAULT_ESCAPE=TRUE, this is compiled as... -->
<?php if (((is_array($_tmp=$this->_tpl_vars['isCheked'])) 
    ? $this->_run_mod_handler('escape', true, $_tmp, 'htmlall') 
    : CRM_Core_Smarty::escape($_tmp, 'htmlall'))): ?>
<!-- With CIVICRM_SMARTY_DEFAULT_ESCAPE=FALSE, this is compiled as... -->
<?php if ($this->_tpl_vars['isCheked']): ?>

This doesn't really make sense to me (since {if $x} isn't outputted, and escaping just complicates the conditional). Actually, it amazes me that anything works with that constraint. (But I guess most conditions work b/c 0, '', FALSE, NULL are mapped through exactly.) AFAICS, one is basically required to have {if} use the default-modifier all the time.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah - you can also use {if $isCheked|smarty:nodefautls}

@totten totten marked this pull request as draft June 29, 2022 10:20
@eileenmcnaughton
Copy link
Contributor

eileenmcnaughton commented Jun 29, 2022

@totten just so you know - elsewhere we use $form->addExpectedSmartyVariable - it's just that this specific one is so weird (ie the validate function is doing the assign because it is a badly written script) - that it is painful here - I'm hoping that other-Tim is coming up with a sensible script over on #23906

@totten
Copy link
Member Author

totten commented Jun 29, 2022

$form->addExpectedSmartyVariable 🙈

May I suggest alternative #23908

@totten totten closed this Jun 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants