Double encoding certain things in large-text custom fields

Started by venguard223, August 09, 2013, 12:30:59 AM

Previous topic - Next topic

venguard223

tfs asked me to look at this, where large text custom fields were re-encoding br tags unnecessarily.

I'd submit a pull request to Github about this but I'm wary of doing it because I see other changes that I don't like the look of. But first, the SMF 2.0 / SD 2.0 changes. These should be good for any current version out there.

Very simple, one change to Subs-SimpleDeskPost.php, line 846 onwards.

Code (find) Select
if (isset($field_values[$row['id_field']]))
{
if ($context['ticket_form']['custom_fields'][$loc][$row['id_field']]['type'] == CFIELD_TYPE_MULTI)
$field_values[$row['id_field']] = explode(',', $field_values[$row['id_field']]);
$context['ticket_form']['custom_fields'][$loc][$row['id_field']]['value'] = $field_values[$row['id_field']];
}


Code (replace) Select
if (isset($field_values[$row['id_field']]))
{
if ($context['ticket_form']['custom_fields'][$loc][$row['id_field']]['type'] == CFIELD_TYPE_MULTI)
$field_values[$row['id_field']] = explode(',', $field_values[$row['id_field']]);
if ($context['ticket_form']['custom_fields'][$loc][$row['id_field']]['type'] == CFIELD_TYPE_LARGETEXT)
$field_values[$row['id_field']] = un_preparsecode($field_values[$row['id_field']]);
$context['ticket_form']['custom_fields'][$loc][$row['id_field']]['value'] = $field_values[$row['id_field']];
}


Just add those two lines before the final line of that block.


Now for what's in Github. Github looks like it's been modified for SMF 2.1 - I don't know if it's properly compatible, but I imagine it probably is since the work's been by emanuele.

The relevant block in Github is:
if (isset($field_values[$row['id_field']]))
{
if ($context['ticket_form']['custom_fields'][$loc][$row['id_field']]['type'] == CFIELD_TYPE_MULTI)
$field_values[$row['id_field']] = explode(',', $field_values[$row['id_field']]);

// Large text boxes may need fixing.
if ($context['ticket_form']['custom_fields'][$loc][$row['id_field']]['type'] == CFIELD_TYPE_LARGETEXT)
{
require_once($sourcedir . '/Subs-Editor.php');

$field_values[$row['id_field']] = html_to_bbc($field_values[$row['id_field']]);
}

$context['ticket_form']['custom_fields'][$loc][$row['id_field']]['value'] = $field_values[$row['id_field']];
}


I can only imagine that the html_to_bbc call is for the benefit of this fix, and that it should be what I've suggested above, however I don't know if other changes to the custom fields were made that meant it does have HTML parsing (I never gave it HTML parsing, only BBC parsing) so I don't want to submit a pull request in case it's irrelevant, but this is certainly something to think about.

I've tested this change locally and it works for me (and I can't seem to break it) but I can't believe I didn't spot it before somehow, there aren't any comments from me in the source reflecting it, put it that way.

tfs

Seems to have fixed the issue on my live help desk.  Thanks Arantor!!!
A good tree cannot bring forth evil fruit, neither can an evil tree bring forth good fruit.


tfs

Found a glitch.  If there's a second custom field that has a *required entry (in addition to the large text box), when you hit the [Edit ticket] button and you get sent back to the ticket and are presented the "Override the custom fields requirements" checkbox... the damage is already done and the BR tags are there.

At that point you can edit the large text box again, removing the BR tags, check the "Override" check box and it works OK. 
A good tree cannot bring forth evil fruit, neither can an evil tree bring forth good fruit.

venguard223

I don't have time to look into it right now but I'll look at it when I get home (which will be tomorrow now)

tfs

Have a safe trip.  No hurry.  As it is now, 99% of it is fixed.  It's rare for us that there's a ticket with overridden fields.  My boss just happened to hit one just after I told him it was fixed.  LOL!
A good tree cannot bring forth evil fruit, neither can an evil tree bring forth good fruit.