Silverstripe-framework: HTMLText doesn't honour the `shortcodes` options

[maxime-rainville]

Affected Version

SS 4.4, but probably all the other ones as well

Description

HTMLText supports a shortcode options, but doesn't honour it. I think that's because the the model.yml injector settings take precedence over the $db settings.

https://github.com/silverstripe/silverstripe-framework/blob/ed7aaff7da61eefa172fe213ec25e35d2568bc20/_config/model.yml#L27-L30

Steps to Reproduce

In a DataObject db definition, add an HTML field like this:

<?php
use SilverStripe\ORM\FieldType\DBHTMLText;
use SilverStripe\ORM\DataObject;
use SilverStripe\Forms\HTMLEditor\HTMLEditorField;

class Dummy extends DataObject
{
    private static $db = [
        'NoShortCode' => 'HTMLText(array("shortcodes"=>false))'
    ];

    public function getCMSFields()
    {
        $fields =  parent::getCMSFields();
        $fields->addFieldToTab(
            'Root.Main',
            new HTMLEditorField('NoShortCode', 'NoShortCode', $this->NoShortCode)
        );
        return $fields;
    }
}

Edit the content of NoShortCode via a WYSIWYG and add an image.
Get the content of NoShortCode rendred in a template.

Expected results: The short code should be outputted as-is in plain text.
Actual results: The short code is converted to an image.

If you define your DB field using the FQNS of HTMLText, it works:

private static $db = [
     'NoShortCode' => DBHTMLText::class . '(array("shortcodes"=>false))'
];

[kinglozzer]

Does defining your property with HTMLFragment achieve the same effect?

private static $db = [
     'NoShortCode' => 'HTMLFragment'
];

If so, it may be easier to deprecate the array("shortcodes"=>false) approach

[maxime-rainville]

We use the same gimmick for HTMLVarchar, which default to shortcode => false. Only because, it's not explicitly define in the Injector YML, so 'NoShortCode' => 'HTMLVarchar(255,array("shortcodes"=>true))' works just fine.

I don't think it's a big issue because DBHTMLText::class works just fine and provide a suitable workaround ... and because it's pretty unusual to want to disable shortcodes in a HTMLText field.

[tractorcow]

This is also a problem at the Injector level; When providing constructor arguments, these can be provided not only at the string level (string arguments in the injector specifier, e.g. Injector::inst()->get("HTMLText(['shortcodes' => true])") but also provided by injector directly. E.g. Injector::inst()->get('HTMLText', ['shortcodes' => true]).

Injector resolves this by joining both lists of arguments together. You would end up with the shortcodes argument being passed as the second and third argument (after the field $name).

The real fix here is to safely document and capture incorrect arguments being passed into HTMLText (or other field types), and to encourage / document the use of HTMLFragment where user arguments are necessary.

[michalkleiner]

Just found a related issue. HTMLVarchar misses the whitelist functionality that HTMLText has, even though it's mentioned in the docblock of the setOptions method.
The problem seems to be that the _HTML_ fields don't inherit from the same ancestor where this kind of functionality would reside.

[maxime-rainville]

@michalkleiner That's sounds like a distinct problem. Would you mind raising it a separate issue?

[michalkleiner]

Extracted into #9104