Revolution: can't get the variable value of a snippet filled with a PH when it contains an '=' sign

[intersel]

Summary

Perhaps I'm missing something but I feel that filters on placeholder may have sometimes some weird behaviour...
when I put an "=" sign in a if-the-else filter to set a value... filter used on a placeholder... used to fill a variable content of a snippet... the variable is not set in the snippet...

Step to reproduce

From a complete fresh install of modx version 2.6.5
1/ Create a snippet "test"

<?php
if (isset($test)) return $test;
else return 'test not set';

2/ add in the content of a resource

1/ [ [+aPH:eq=``:then=`aPH is null`:else=`aPH is not null`] ]<br>
Result: [[+aPH:eq=``:then=`aPH is empty`:else=`aPH is not empty`]]<br>
Sounds good...<br>

2/ [ [test? &test=`[ [+aPH:eq=``:then=`aPH is empty`:else=`aPH is not empty`] ]`] ]<br>
Result: [[test? &test=`[[+aPH:eq=``:then=`aPH is empty`:else=`aPH is not empty`]]`]]  <br>
Output ???<br>

Ha ok... we need to use empty... but test 1 was working...<br>

2bis/ [ [test? &test=`[ [+aPH:empty=`aPH is empty`] ]`] ]<br>
Result: [[test? &test=`[[+aPH:empty=`aPH is empty`]]`]]  <br>
Output ok<br>

3/ let's try adding a equal sign in the text...<br>
[ [+aPH:eq=``:then=`aPH is = null`:else=`aPH is not null`] ]<br>
Result: [[+aPH:eq=``:then=`aPH is = null`:else=`aPH is not null`]]<br>
Sounds good...<br>

4/ [ [test? &test=`[ [+aPH:eq=``:then=`aPH is = null`:else=`aPH is not null`] ]`] ] <br>
Result: [[test? &test=`[[+aPH:eq=``:then=`aPH is = null`:else=`aPH is not null`]]`]] <br>
My variable is not set...??<br>

4bis/ let's try with empty.... [ [test? &test=`[ [+aPH:empty=`aPH is = null`] ]`] ] <br>
Result: [[test? &test=`[[+aPH:empty=`aPH is = null`]]`]] <br>
My variable is still not set...??

Observed behavior

I obtain this results:

1/ [ [+aPH:eq=``:then=`aPH is null`:else=`aPH is not null`] ]
Result: aPH is empty
Sounds good...
2/ [ [test? &test=`[ [+aPH:eq=``:then=`aPH is empty`:else=`aPH is not empty`] ]`] ]
Result: aPH is not empty 
Output ???
Ha ok... we need to use empty... but test 1 was working...
2bis/ [ [test? &test=`[ [+aPH:empty=`aPH is empty`] ]`] ]
Result: aPH is empty 
Output ok
3/ let's try adding a equal sign in the text...
[ [+aPH:eq=``:then=`aPH is = null`:else=`aPH is not null`] ]
Result: aPH is = null
Sounds good...
4/ [ [test? &test=`[ [+aPH:eq=``:then=`aPH is = null`:else=`aPH is not null`] ]`] ] 
Result: test not set 
My variable is not set...??
4bis/ let's try with empty.... [ [test? &test=`[ [+aPH:empty=`aPH is = null`] ]`] ] 
Result: test not set 
My variable is still not set...??

Expected behavior

1/ [ [+aPH:eq=``:then=`aPH is null`:else=`aPH is not null`] ]
Result: aPH is empty

2/ [ [test? &test=`[ [+aPH:eq=``:then=`aPH is empty`:else=`aPH is not empty`] ]`] ]
Result: aPH is is empty

2bis/ [ [test? &test=`[ [+aPH:empty=`aPH is empty`] ]`] ]
Result: aPH is empty 

3/ let's try adding a equal sign in the text...
[ [+aPH:eq=``:then=`aPH is = null`:else=`aPH is not null`] ]
Result: aPH is = null

4/ [ [test? &test=`[ [+aPH:eq=``:then=`aPH is = null`:else=`aPH is not null`] ]`] ] 
Result:  aPH is = null

4bis/ let's with empty.... [ [test? &test=`[ [+aPH:empty=`aPH is = null`] ]`] ] 
Result:  aPH is = null 

Environment

MODX 2.6.5 , php 7.0, Apache/2.4.10 (Debian)

[OptimusCrime]

I do not understand this. Can you give specific input/outputs with the expected and observed behavior?

[intersel]

Thanks for your interest... I thought it was clear but... visibly not...
I give the procedure (create a snippet, create a resource) with the content to set, the observed and expected behaviour in each chapter... what would you need more?
In my scenario, aPH is not set .... I have scenarios where input values may be missing, and sometime the behaviour becomes erratic with side effects... actually the one where the value is not set at all is disturbing.... the one where I just add a "=' sign in the string and the result sounds weird...
PS/ there's no plugin, no other things, just a fresh install...
Perhaps I'm the only one who experiences it...?

[Jako]

Could you please reformat your examples. I don't get the clue at the moment, because I am not sure if you really mean [ [ or [[ in your code. And you are mixing normal text with code inside of the code fences.

[JoshuaLuckers]

The isset function determines if a variable is set and is not NULL. In your case the variable $test will most likely be set to an empty value.
Can you check if you use the function empty instead of isset that it will produce the expected output?

[alroniks]

I found the issue.
It can be solved by calling all snippets uncaching, adding ! like [[!test?]]
But the original issue that here https://github.com/modxcms/revolution/blob/2.x/core/model/modx/modparser.class.php#L360 pair of `` symbols replaced by one and it broke that inserted call. When it called uncashed it use different order of parsing (my thoughts) and works well, but it place wrong value to cache and when it grabbed from cache it is broken.

[intersel]

@Jako
here the reformatted examples:

Assuming aPH is not defined

1st problem:
Test: [[test? &test=`[[+aPH:eq=``:then=`aPH is empty`:else=`aPH is not empty`]]`]]
Result: "aPH is not empty"
Expected: "aPH is empty"

The problem for me is that it does not answer the same result as this one:
[[+aPH:eq=``:then=`aPH is empty`:else=`aPH is not empty`]] that gives "aPH is empty"

2nd problem:
Test: [[test? &test=`[[+aPH:eq=``:then=`aPH is = null`:else=`aPH is not null`]]`]]
Result: "test not set"
Expected: "aPH is = null"

I would expect test to be set... I understand that using the "empty" function, i can solve this but it sounds curious. And I'm not sure that it is a correct setting of test according to the input.

3nd problem:
Test: [[test? &test=`[[+aPH:empty=`aPH is = null`]]`]]
Result: "test not set"
Expected: "aPH is = null"
But if you do: [[+aPH:empty=`aPH is null`]] The result is correct! and gives "aPH is null"

[intersel]

@Alroniks
Great to have put a finger on the problem... is it the cause of the 3 problems?
Well, actually with your solution, I would need to uncache all my site to be sure everything is fine :-(
Or find everywhere the problem could occur...
I suppose there are good reasons to do this str_replace the `to one but the side effects are not that good and I had a tough time to figure out why some of my parsing was completly wrong with errors!

[Jako]

Conditionals (or other modifiers) on not existing placeholders are bad!

But the issue that an equal sign inside of a parameter value changes the output is strange.

Could you check: [[+aPH:empty=`aPH is null`]] vs [[+aPH:empty=`aPH is = null`]]

[intersel]

except for a philosophical point of view, I don't see why it would be bad...
and if it is so, I would hope a little error message to help me to identify them... as I can see several scenarios where it could happen... it can be really hard to find all of them...
[[+aPH:empty=aPH is null]] vs [[+aPH:empty=aPH is = null]] gives correct answers, I mean the one we would expect:
aPH is null vs aPH is = null

[intersel]

and the problem occurs only when the placeholder is not set, everything is fine when "aPH" is set even with nothing in it...

[JoshuaLuckers]

Hi @intersel, is it possible for you to write a Unit Test? Might be good nice to have tests for this in https://github.com/modxcms/revolution/blob/2.x/_build/test/Tests/Model/modParserTest.php

[intersel]

HI @JoshuaLuckers
Ok... is there a procedure or a doc to do that? Just add a new test in the file and do a PR?

[JoshuaLuckers]

@intersel sorry for the delay, yes that’s all!