Output modifier issue in Modx 3.0.1-pl

claireallan · May 20, 2022, 4:47pm

Issue: Unwanted code being output on the page when using an output modifier.

This works (i.e. with the alt tag):

[[*id:is=`72`:then=`<img src="http://buccleuchproperty.processproduction.co.uk/[[*article-image]]" alt="Test image">`:else=`This is not ID 72`]]

But “`:else=” is output on the page when using this (i.e. without the alt tag):

[[*id:is=`72`:then=`<img src="http://buccleuchproperty.processproduction.co.uk/[[*article-image]]">`:else=`This is not ID 72`]]

Screenshot of what is output on the page when the alt tag is removed:

Of course, all images should always have an alt description, but just thought I should report this as it seems odd behaviour and could potentially be a cause of problems elsewhere.

dejaya · May 20, 2022, 5:20pm

Hmm … I can’t recreate this one @claireallan

Seems to work as expected for me.

Is your [[*id…]] call safe in a chunk somewhere? IE - not getting mangled in a TinyMCE or similar?

claireallan · May 20, 2022, 5:25pm

Hi @dejaya

Thanks for taking a look.

It’s definitely safe in a chunk, but there might be some other weird things going on. It’s part of a fairly complex site that I’ve been trying to upgrade to Modx 3, so I’ll try stripping things back tomorrow to see what’s making it not work as expected.

I’ll report back with any findings.

claireallan · May 21, 2022, 3:44pm

Hi @dejaya

I’ve created a new template and test page and stripped it right back to:

<html>
<head>
</head>
<body>
[[*id:is=`74`:then=`<img src="https://via.placeholder.com/150">`:else=`This is not ID 74`]]
</body>
</html>

I still get the “`:else=” code output on the page.

If I add the alt tag it works fine (no idea why this would make a difference??):

<html>
<head>
</head>
<body>
[[*id:is=`74`:then=`<img src="https://via.placeholder.com/150" alt="Test image">`:else=`This is not ID 74`]]
</body>
</html>

Or if I remove the file path and just use a placeholder without an alt tag it works (again, no idea why this fixes it):

<html>
<head>
</head>
<body>
[[*id:is=`74`:then=`<img src="[[*article-image]]">`:else=`This is not ID 74`]]
</body>
</html>

I have a custom media source with basePath and baseUrl set to assets/images/ which was set as the default but I’ve tried changing the default back to Filesystem and it doesn’t change the issue I’m seeing, so I don’t think that’s related.

Hopefully that info’s useful to anyone wanting to investigate this. It’s not a major issue as it’s easily fixed with the addition of the alt tag (which I’d always check for prior to live anyway), but I just thought I’d report this rather odd behaviour because it certainly didn’t do this prior to the upgrade to version 3 and seems a bit glitchy.

dejaya · May 21, 2022, 4:08pm

Hey @claireallan

If I copy your first template code there - I still see just the image. Everything looks like it should.

I’m really not sure why that would be happening on your installation. Do you maybe have a custom plugin set up to alter <img> tags? Might run on OnWebPagePrerender.

claireallan · May 21, 2022, 4:18pm

Hi @dejaya – thanks for testing.

Really strange that it seems like it’s just me seeing this behaviour. I’ve got the pthumb plugin installed, but can’t imagine that could be causing the issue as it’s not being used on that image.

I’ll be starting a new site build within the next couple of weeks which will be using a fresh install of 3.0.1 rather than an upgrade to 3.0.1, so I’ll see if I can recreate the issue on that new site.

Thanks!

markh · May 21, 2022, 4:50pm

Perhaps pdoTools has its parser enabled instead of the MODX default? What’s your parser_class system setting set to?

claireallan · May 21, 2022, 5:19pm

Hi @markh

My parser_class setting is set to pdoParser. Should I change it to something else?

Thanks!

markh · May 21, 2022, 5:33pm

Emptying it (and parser_class_path) should revert that to the default MODX one.

claireallan · May 21, 2022, 6:00pm

Hi @markh – I’ve deleted what was in the parser_class and parser_class_path settings and both are now blank. Does that mean that the default is restored or should these settings be populated with something?

The issue with the “`:else=” code being output on the page still persists.

dev_willis · May 23, 2022, 11:16am

I was able to reproduce this on a 3.0.0 site that I upgraded from 2.x. The site is using pdoParser.

While I can’t explain the behavior, I’ve noticed that it seems to be related to the colon in the URL.

E.g., this code breaks:
[[*id:is=`243`:then=`<img src="http://via.placeholder.com/150">`:else=`This is not that`]]
Output:
<img src="http://via.placeholder.com/150">`:else=`

But this code works:
[[*id:is=`243`:then=`<img src="//via.placeholder.com/150">`:else=`This is not that`]]
Output:
<img src="//via.placeholder.com/150">

dejaya · May 23, 2022, 1:48pm

Good find @dev_willis …

Interestingly I don’t see parser_class or parser_class_path under System Settings at all.

MODX 3.0.1-pl [fresh install] with pdoTools 3.0.0-beta

dev_willis · May 23, 2022, 2:04pm

I just searched for “parser” and they came up. They’re under the “Site” area.

dejaya · May 23, 2022, 2:29pm

Yep - they’re just not there:

claireallan · May 25, 2022, 12:28pm

Hi @dev_willis

Thanks for investigating further. It certainly seems odd that the colon appears to be causing an issue.

@markh – Does this give you any more to work with in terms of figuring out what’s going wrong? Can you give me any other info regarding what the default parser_class and parser_class_path should be so I can test your previous theory?

Thanks

halftrainedharry · May 25, 2022, 2:11pm

I don’t think the system settings parser_class and parser_class_path are used anymore in MODX 3.x.
It seems the setting is now called modParser.class. (This setting doesn’t exist on a fresh installation.)

MODX 3.x

github.com

modxcms/revolution/blob/cb6f56a19af818ad4d1d62dc0d31b9819aaa876f/core/src/Revolution/modX.php#L747-L757

      
        
            public function getParser() {
                if (!$this->services->has('parser')) {
                    $parserClass = $this->getOption('modParser.class', null, modParser::class);
                    $this->services->add('parser', new $parserClass($this));
                }
                if (!$this->parser instanceof modParser) {
                    $this->parser = $this->services->get('parser');
                }
            
            
    return $this->parser;
            }

MODX 2.x

github.com

modxcms/revolution/blob/6845340fddd97dcf733731765f50201a6abf7fd0/core/model/modx/modx.class.php#L724-L726

      
        
            public function getParser() {
                return $this->getService('parser', $this->getOption('parser_class', null, 'modParser'), $this->getOption('parser_class_path', null, ''));
            }

claireallan · May 25, 2022, 3:29pm

Hi @halftrainedharry

I can’t see a system setting called modParser.class on my install of 3.0.1-pl:

I’ve done a search for pdo in the system settings and get the following results:

Could any of these be causing the issue? I’m assuming from the responses so far that the working theory is that it’s something to do with having pdoTools installed that’s causing the problem?

halftrainedharry · May 25, 2022, 3:55pm

It turns out that pdoTools adds its own parser class via the bootstrap.php file (and not with a system setting).

github.com

modx-pro/pdoTools/blob/3a358ecee89963c8718fd6537f0ad44dec87bfe6/core/components/pdotools/bootstrap.php#L36-L39


      
          $modx->services->add('parser', function ($c) use ($modx) {
              $class = $modx->getOption('modParser.class', null, ModxPro\PdoTools\Parsing\Parser::class, true);
              return new $class($modx, $c->get('pdotools'));
          });

If you don’t want to change the code, it should be possible to add a system setting modParser.class with value MODX\Revolution\modParser to use the default MODX parser instead.

On a fresh installation of MODX 3.0.1 with pdoTools, I can’t reproduce the issue.
Maybe there is a problem with updating from an older version, or the cause is something entirely different.

claireallan · May 26, 2022, 2:46pm

Hi @halftrainedharry

I’ve gone ahead and created the new system setting but the issue with the “`:else=” code outputting on the page still exists. I’ve never created a new system setting before, so can you confirm that what I’ve done is what you intended?

All of this parser class stuff is over my head and I’m not sure what these settings are doing! But I’m happy to experiment with things if it’ll fix the problem and potentially prevent similar issues for others.

halftrainedharry · May 26, 2022, 5:04pm

Yes, your system setting is correct. But pdoTools is not the culprit.

I was finally able to reproduce the issue.
Weirdly enough it only happens on a linux server. On my local Windows WAMP server everything works correctly.

I could trace the bug back to this line

github.com

modxcms/revolution/blob/697a0286117a7e30b19c7f78ef30803806daf6c3/core/src/Revolution/Filters/modInputFilter.php#L56

      
        
            public function filter(&$element)
            {
                /* split commands and modifiers and store them as properties for the output filtering */
                $output = $element->get('name');
                $name = $output;
                $splitPos = strpos($output, ':');
                if ($splitPos !== false && $splitPos > 0) {
                    $matches = [];
                    $name = trim(substr($output, 0, $splitPos));
                    $modifiers = substr($output, $splitPos);
                    if (preg_match_all('~:([^:=]+)(?:=`((?:(?:[^:=][\s\S]*?|(?R))*?))`)?~s', $modifiers, $matches)) {
                        $this->_commands = $matches[1]; /* modifier commands */
                        $this->_modifiers = $matches[2]; /* modifier values */
                    }
                }
                $element->set('name', $name);
            }
            
            
/**
             * Indicates if the element has any input filter commands.
             *

I think this bug is related to these issues on github:

github.com/modxcms/revolution

wrong parse when having a semi-column with a setting variables in output filter 'then' or 'else'

opened 09:39PM - 07 Feb 22 UTC

intersel

bug

## Bug report ### Summary When we add a semi-column (';') in the output filter… "then", modx can't parse properly and the output is strange.... This behaviour needs to use twice a same setting value... see example. ### Step to reproduce create snippet "echoinput" ``` <?php return $input; ``` create a resource "test bug" with a void template and the following content: ``` [[echoinput?&input=`[[++mail_smtp_hosts]] [[echoinput:eq=`0`:then=`test1:[[++mail_smtp_hosts]];`:else=`test2:[[++mail_smtp_hosts]]`?input=`0`]]`]] ``` ### Observed behavior result: ``` `localhost [[echoinput:eq=`0`:then=`test1:localhost ``` ### Expected behavior ``` localhost test1:localhost; ``` If you remove the ';' in the then filter like `test1:[[++mail_smtp_hosts]]` instead of `test1:[[++mail_smtp_hosts]];` then everything is parsing ok.... If you add a blank after the name of the setting variable "[[++mail_smtp_hosts ]]" , like ``` [[echoinput?&input=`[[++mail_smtp_hosts ]] [[echoinput:eq=`0`:then=`test1:[[++mail_smtp_hosts]];`:else=`test2:[[++mail_smtp_hosts]]`?input=`0`]]`]] ``` The parsing is ok too ! ### Related issue(s)/PR(s) can not say... ### Environment MODX Revolution 2.8.3-pl, Apache/2.4.38 , 10.3.31-MariaDB, Firefox/chrome/...

github.com/modxcms/revolution

Notempty Output Filter cuts off value if it includes snippet properties

opened 10:17PM - 16 May 22 UTC

newswanger

bug

## Bug report ### Summary Not empty output filter will truncate the value if i…t includes a snippet with properties. ### Step to reproduce Add the following to a template: [[*content:notempty=\`[[!Wayfinder? &startId=\`0\` &level=\`1\`]]\`]] ### Observed behavior Rendering this in a browser will display: [[!Wayfinder? &startId= ### Expected behavior The complete snippet tag should be returned from the filter and processed. This filter does work MODX 2.8.4 ### Environment MODX 3.0.1, apache2 , mariaDB