Pass elements through EE template tags to HTML form tags

[TomJaeger]

The idea of adding an ability to pass elements through EE template tags to HTML form tags recently came up in Slack. There has been a lot of conversation around it in Slack DM's as well as a public threaded conversation. This github issue is intended to bring the conversation fully public and add permanence to it. This has also been discussed as a public contributed addition to the upcoming 5.4 release :)

The code that would need to be modified is located in the following method.

ExpressionEngine/system/ee/legacy/libraries/Functions.php

Line 421 in 5db58a8

public function form_declaration($data)

We talked about adding a parameter named 'pass_through', and using the ee()->TMPL->fetch_param('pass_through') option to accept a pipe separated list of items that would then be added to the outputted HTML accordingly.

[litzinger]

My initial thoughts are there are 2 ways to approach this.

1. Make the updates necessary in the existing Functions file and don't mess too much with the existing code aside from making whatever changes also work with the hooks that are present in that method.

2. Take the opportunity to refactor this function into a new service. The only dependencies here are the hooks and the $this->get_site_index() call. Of course by dependencies I mean anything that method calls that is external to the method or class entirely. I don't know what it looks like, but it might be possible to make the hooks callbacks in the service, and then pass the site_index value into the service as a constructor arg, thus removing any external dependencies and making the new service unit testable. It's very difficult to unit test any methods that contain references to the ee() super object b/c you have to stub everything out. Possible, but a lot of work. I'd also recommend making a separate view file to build the form tag. Right now it is constructing the html string in the method, so there are no separation of concerns happening. If you make a new service, just call said service in the existing Functions->form_declaration() method for backwards compatibility. This approach has been taken in various other locations of the legacy directory.

[jcogs-design]

I'm happy to have a go at doing a version of "1" in the above post. As and when that seems to work, then I suggest at some later date someone migrates that code into the new service option outlined in "2" above.

[jcogs-design]

Based on our initial discussion in Slack one aspect that would benefit from a wider perspective is the choice of parameter format to use. The solution needs to support the addition of multiple additional attributes to the form tag (not including ARIA and event handler global attributes, HTML specifies 38 HTML attributes that can be added to a form tag - of which EE currently handles 9). So far four alternative parameter forms have been proposed:

• everything in a very long string including quoting - for example
  pass_thru="target='_blank' novalidate name='my form'"
• everything in a very long string with quoting done by EE - for example
  pass_thru="target=_blank novalidate name=myform"
• everything in a very long string with pipe separation - for example
  pass_thru="target='_blank'|novalidate|name='myform'"
• everything in two pipe separated parameters - for example
  pass_thru_attributes="target|novalidate|name" pass_thru_values="_blank||myform"

Question in each of these cases is how to manage attribute values containing spaces and / or meaningful characters (such as | ), and how to leave enough quote headroom for use of layout array variables to provide input (which require one level of quotes to drive the index value)? The dual parameter solution (one for attribute names, one for values) and / or the EE adds quotes version use the fewest quote levels.

[blue-dreamer]

Just throwing this in, the ability to add custom attributes should be made available for any front end form, Email module, Tell a friend, Comments, Channel form etc.

[jcogs-design]

Just throwing this in, the ability to add custom attributes should be made available for any front end form, Email module, Tell a friend, Comments, Channel form etc.

That's the idea. Plan would be to modify the EE function that all those forms use to generate their form tag - so all things using that function would automatically inherit the additional ability

[litzinger]

I don't know if I'd take the pass_thru="target='_blank'|novalidate|name='myform'" approach. Seems confusing, plus it can lead to odd quotation issues. Why not just pass the form tag attributes directly to the

tag? e.g.

{exp:some:form data-foobar="something" return="some/page" target="_blank"}

Renders out:

<form data-foobar="something" target="_blank">
    <input type="hidden" name="RET" value="some/page">

Everything is available in the ee()->TMPL->tagparams array, so the method calling form_declaration should know what it's valid/documented parameters are, so you can filter them out, then put what is left in ee()->TMPL->tagparams array directly onto the tag.

[jcogs-design]

@litzinger

Why not just pass the form tag attributes directly to the tag?

aside from making whatever changes also work with the hooks that are present

The idea of just passing the attributes directly makes sense.

The hooks (form_declaration_modify_data, form_declaration_return) each get a copy of and can modify the array variable submited when the function was called. If we go with reading the tag attributes directly in the function (by consulting ee()->TMPL->tagparams), should the hooks be sent the original array variable (i.e. without any extra tagparams), or should they get a copy of the array variable and whatever else is in tagparams? Can see pros and cons for either course - inclination is to continue to send just the original array variable, but would value your thoughts.

[jcogs-design]

I've put a partial fix together based on @litzinger idea of simply passing through tagparams but hit an issue. Would value any guidance on a solution.

Here is the code so far - it reads the tag params, excludes any that are already being considered by the function, and constructs an element to add to the <form> tag with the remaining parameters found.

$_tagparams = array();
$_pass_thru = '';

// Cannot think of any circumstance where tagparams is not empty, but just in case... 
if ( ! empty(ee()->TMPL->tagparams))
{
    $_tagparams = ee()->TMPL->tagparams;
    
    foreach ($_tagparams as $key => $val) {
    
        // Ignore the parameter if $key is defined in $deft (and so already being processed by function)
        if (! array_key_exists($key, $deft)) {

            // Append this key / value pair to end of the $_pass_thru variable
            $_pass_thru .= ' '.$key.'="'.$val.'"';
        }
    }
}

// Construct the opening form tag - including appending any pass_thru parameters
$form  = '<form '.$data['id'].$data['class'].$data['name'].'method="post" action="'.$data['action'].'" '.$data['onsubmit'].' '.$data['enctype'].".$_pass_thru>\n";

The issue is that tagparams also includes other parameters used by the tag (e.g. channel= url_title= etc.) that we don't want to pass through. Is there some established EE method for excluding such parameters from the tagparam list? As you will see I'm excluding the parameters that are already being processed by the function using a simple test against the default value array for these parameters; it would be good if there was something equally simple to exclude the other parameters we don't want to pass through.

Since I don't know what functions are calling the form_declaration function and so what parameters they are possibly using (and for future proofing, we have to assume other functions might be defined in future with unknown named parameters) it does not seem sensible to simply construct a list of possible EE parameters to exclude, or to build a list of 'acceptable' attributes that we might pass-through based on HTML spec or whatever (since ARIA, event monitoring and some HTML attributes may change over time).

Any ideas?

If we can't find a way to exclude the EE tag functional parameters, I think we have to go back to thinking about a pass_thru= parameter of some kind.

[jcogs-design]

Have developed a version that I think reaches a reasonable compromise between the various issues / options. The current test version works reliably in all testing circumstances tried so far.

NB. Unknown issues with github prevent the posting of a branch here: if you read this and want to try before those issues are resolved DM me and I'll send you a copy of the code.

The version has the following characteristics:

• prior functionality is maintained as-is for compatibility purposes;
• any attribute value passed through to function by calling function is given priority over any other new function added by these mods;
• if id= name= class= or onsubmit= are specified in the calling tag they will be processed by the existing functionality of this function (even if the function controlling the tag itself ignores the parameters) and will be included in the <form> tag created by it
• additional attributes can be specified in a new parameter pass_thru_attributes=; if the user wants to specify more than one then they can be added via a piped list (e.g cats|dogs)
• since attributes cannot contain spaces or a variety of other illegal characters, attribute names are stripped of anything other than alphadash characters (so this attribute! becomes thisattribute
• values for the additional attributes can be specified in a new parameter pass_thru_values=
• if no values are specified, the attributes are passed through simply as tokens inline with HTML protocol (e.g. novalidate)
• if the user wants to specify values for multiple attributes, these are added via a similar piped list (e.g. are vicious and mean|are kind and loving). To add a null value within a list of values simply put two pipe characters together
• if the pass_thru_attributes= parameter contains an attribute also specified either in the array variable supplied to the function by the calling function, or a parameter set explicitly in the tag, it is ignored unless it is class. Multiple instances of class are simply collated into a single class= attribute in the <form> tag.

So by way of illustration the following EE tag:

{exp:channel:form 
    channel="articles" 
    url_title="eric" 
    class="stuff" 
    id="test" 
    pass_thru_attributes="favourite biscuit|drinks|cake|name|id|class"
    pass_thru_values="chocolate||strawberry cream|kitty|cat|and nonsense" 
    return="{current_path}"
}

renders output like this (method and action attributes have been removed and the tag output reformatted for clarity) (note: favourite biscuit is concatenated, drinks is passed as a token, the second id attribute specified is ignored, class values are combined)

<form 
    id="test" 
    class="stuff and nonsense" 
    favouritebiscuit="chocolate" 
    drinks
    cake="strawberry cream" 
    name="kitty"
>

The revised code includes a small bugfix (in the original no default value for $data['name'] is set).

The use of the two-parameter solution is not ideal but seems only way to work around the lack of certainty about what constitutes possible parameters that might appear in the tagparams, and so the difficulty of excluding those that are not relevant to this pass_thru activity.

It might also be that a more abbreviated name for the parameters might be appropriate - suggestions would be welcomed.

Finally all this will require changes to the documentation of tags that trigger EE forms to explain the new tags, and also the new way of treating id= name= class= and onsubmit= parameters.

Look forward to any comments etc.

[litzinger]

I'm still not a fan of the pass thru option. The issue I have with it is that it introduces a new pattern for tag usage, which is to separate key/value pairs into 2 different parameters. I've seen similar stuff from 3rd party add-ons, but I don't think the core should natively support it. It's confusing, especially to new users, who have to split up the keys and values into 2 different parameters. It also means you have to count to make sure your keys and values line up, which isn't intuitive, and it prevents you from easily conditionally adding a parameter to the tag, e.g.

{exp:channel:form 
    channel="articles" 
    url_title="eric" 
    class="stuff" 
    id="test" 
    {if segment_2}data-attribute="foo"{/if}
}

I held back on mentioning this in my initial feedback, but maybe this is something that warrants introducing Symfony's OptionsResolver to EE. I've been using this in my add-ons for awhile now and it's great to be able to provide a list of parameters, and valid values for those parameters, and if the user enters an invalid parameter or value they get an error message informing them of the problem, instead of EE silently ignoring it and the user is frustrated and doesn't know why their tag isn't working. form_declaration() could be given an optional 2nd parameter, which is an OptionsResolver object. The channel:form tag would create the object with it's default parameter values, pass the OptionsResolver object to form_declaration(), which can then separate the extra custom parameters that the user wants to add from the ones required for the form tag.

[jcogs-design]

I concur with the awkwardness of the two tag solution, and the desirability of the 'simply chuck in a parameter and if it is not used we'll pass it on to the HTML tag' usability model. But as per previous notes, the trade off is between doing something that works without opening up the rest of EE code for modifications (vis example code provided should I ever find out how to get it into the repo for review) and something that works better but needs potentially much greater levels of invasion into the EE code.

As per my original comments, tracking down the (apparently) 52 instances where this function is called, working out for each how to modify the calling code to provide the appropriate list of params to side-step and testing it all is a fairly substantial activity. Going further and working out how to insert Symfony objects into the flow is yet another thing - as is creating a new Service Class and so on.

The need to have (if nothing else) the ability to pass ARIA parameters through to forms is something that could be used to improve EE sites now. I think the danger is that we opt for the complex solution and then find that it never arrives. I think there is a case for doing something that works and if need be to litter the documentation with caveats that someone is working on a better (Service Class / Symfony / whatever) solution for use in EE6.

[litzinger]

What are examples of other attributes that would be added? You mentioned aria attributes... what about adding an option to accept any parameter prefixed with role, aria-, or data-, and just pass them along directly to the <form> tag. This would reduce the scope a bit and probably lean toward a simpler implementation.

[jcogs-design]

The form documentation details 11 possible attributes that can be set specifically for the tag, plus noting that it also accepts all global attributes. There are 27 global attributes (of which, as you note two support multiple forms aria-* and data-*) and the same page notes that there are the 63 possible event handler attributes that could legitimately be included too. I have not checked for double counting between the various lists, or removed the (I think 8) attributes currently supported by the function. But we are looking at around 100 not including aria-* or data-* variants (and of course the possibility that new attributes might be defined in future). HTH

[jcogs-design]

Maybe this is something that warrants introducing Symfony's OptionsResolver to EE.

Had a read through of the documentation and can see that it is an interesting way to replace the code already in the function for setting default values. Cannot see how, though, it helps with the work to identify what each calling function might also require (or optionally use) in the tagparams: without that list of parameters to watch out for / ignore, the 'just pass it all through' option fails - as all we want to pass through are the parameters that are not used elsewhere by EE for its own purposes. The alternative option (defining all possible legitimate HTML Form attributes and only accepting those) is also problematic (e.g. presumably that list will need to be updated as HTML spec changes).

So while I see the nicety of having symfony elements deployed, I cannot see how it would actually help with this issue (of working out what parameters to ignore). Am I missing something?