addAttributes method is confusing in Widget class

[Serhii-DV]

Widget class has two methods for adding attributes: addAttribute and addAttributes.

addAttribute method is working fine and it allows to add custom attributes to widget.

$widget->addAttribute('data-var1', 1);
$widget->addAttribute('data-var2', 2);

and it will generate widget with this attributes:

<select ... data-var1="1" data-var2="2">

But, addAttributes doesn't work as expected.

$widget->addAttributes(array(
   'data-var1' => 1,
   'data-var2' => 2
));

In this case no attributes will be generated

<select ...>

The problem is in that addAttributes methods invokes __set magick method and add unknowns attributes to $arrConfiguration value. addAttribute method directly adds attributes to protected $arrAttributes value. And getAttributes method takes attributes from $arrAttributes value not $arrConfiguration.

[leofeyer]

What would you suggest?

[Serhii-DV]

I think we could safely use addAttribute method for each array element in addAttributes and don't think about backward campability. Until now no one has pointed on this problem, so no one have used addAttributes method.

[leofeyer]

and don't think about backward campability

This can never be the solution!

[Serhii-DV]

Another solution. Add new method addConfigurations (in the same way as addAttributes) and use it in Widget::__constructor and Widget::parse methods instead of addAttributes.

And in addAttributes method check Contao version, something like that:

    public function addConfigurations($arrConfigurations)
    {
        if (!is_array($arrConfigurations))
        {
            return;
        }

        foreach ($arrConfigurations as $k=>$v)
        {
            $this->$k = $v;
        }
    }


    public function addAttributes($arrAttributes)
    {
        if (!is_array($arrAttributes))
        {
            return;
        }

        if (version_compare(VERSION, '3.3', '<') && version_compare(BUILD, '3', '<'))
        {
            $this->addConfigurations($arrAttributes);
        }
        else
        {
            foreach ($arrAttributes as $k=>$v)
            {
                $this->addAttribute($k, $v);
            }
        }

    }

[leofeyer]

The best solution IMHO is to adjust the addAttributes() method so it correctly identifies the attributes. This is already done by whitelisting all known attributes in the setter method, so we should just add a routine which adds everything starting with data- as attribute instead of as configuration value there.

[Serhii-DV]

But HTML 5 specification allows to add attribute with any name not only with data- prefix. I can, for example, add myvar="12321" attribute and it will be HTML 5 compatible.

[Serhii-DV]

For example, AngularJS uses custom HTML attributes like ng-model, ng-submit and etc.

[leofeyer]

Then we need to discuss this. @contao/developers

[ausi]

<select myvar="12321"></select> is not valid HTML5. Custom attributes have to start with data-, see: w3.org/html/wg/drafts/html/master/dom.html.

[fjacobi]

Yep, AngularJS is not valid by default - you have to use a directive to validate it.

[psi-4ward]

Whats the problem with invalid attributes? Imho are there no side effects and are heavily used by various JS widgets like Angular or Datepickers. For me we sould accept every attribute a developer pushes in!

[fjacobi]

I don't have any problems with invalid attributes.
I think every developer using this function should know if he wants validity or not - whitelisting is not a real option imho.

[leofeyer]

Please read the ticket – you are missing the point.

[leofeyer]

Changed in b6f5d08. I have added support for data- attributes as well as ng- attributes (although the latter is not valid HTML5).