Problem mit empty und isset behoben

[iCodr8]

Ich habe ein Problem im DcaWizard entdeckt. Die empty() Funktion funktioniert nicht richtig, da die Klassenvariable $this->params übergeben wird. Dadurch findet kein merge mit $arrParams statt.

// Merge params
if (!empty($this->params) && is_array($this->params)) {
    $arrParams = array_merge($arrParams, $this->params);
}

Im Pull Request ist die Lösung für das Problem. Ich hoffe ihr könnt das in den Hotfix 2.1.1 übernehmen.

[aschempp]

This is actually a Contao core issue. The Widget class (or even BaseTemplate or System) should implement __isset(). We should simply remove the empty call and it will continue to work.

[Toflar]

Well, it was introduced by @iCodr8 himself ;-) (see https://github.com/terminal42/contao-dcawizard/commit/a5747b9f547fb89252d92483db2923c41a242c06). But I think we can still fix it in the dcaWizard. But yes, we should also copy the code for @leofeyer.

[leofeyer]

@Toflar What do I have to do?

[Toflar]

See what @aschempp wrote:

The Widget class (or even BaseTemplate or System) should implement __isset()`

Otherwise

if (empty($this->magicGetterProperty)) {

will not work. We had that problem in other parts of the application as well (Hybrid, ContentElement, Template etc.).

[iCodr8]

I think the Widget class needs the fix.

[leofeyer]

It does not seem trivial to implement though, because the getter might return anything. Can you please test the following approach:

public function __isset($strKey)
{
    return $this->$strKey !== null;
}

[leofeyer]

If the code above works, it could be added to the System class.

[aschempp]

I think we should add a real implementation:

class Widget 
{
    public function __isset($key)
    {
       return isset($this->arrConfiguration[$key]) || isset($this->arrAttributes[$key]) || parent::__isset($key);
    }
}

class System
{

    public function __isset($key)
    {
        return isset($this->arrData[$key]);
    }
}

I have not tested this!

[leofeyer]

It will not work, because the approach is much too simple.

E.g. the System class stores its values in $this->arrObjects, the File and Folder classes extend the parent method with function calls such as filectime(TL_ROOT . '/' . $this->strFile);, the User and Template classes use $this->arrData, the Widget class uses $this->arrConfiguration and $this->arrAttributes and so on.

A specific implementation for the Widget class is of course not a problem. But not for the System class.

[Toflar]

A specific implementation for the Widget class is of course not a problem.

:+1:

[aschempp]

@leofeyer I know there are multiple inheritances, but each of them should check it's getter/setter array! That would not only fix Widget but all classes!

[leofeyer]

Ok, so here is a list of all classes that need to be fixed (probably incomplete):

• Database
• Database\Statement
• DataContainer
• Date
• Email
• Environment
• File
• Folder
• Request
• System
• User
• Widget
• ZipReader

Adding the method is not trivial for at least half of them (e.g. Environment, File, Folder). Or do you already have an idea?

[aschempp]

I must say I have not really looked into it, but in theory it should just call isset() on the same storage as get and set does? Can you give an example of a problem?

[leofeyer]

For instance, we are not using a storage in the File or Environment class. Instead, we are making function calls.

https://github.com/contao/core/blob/master/system/modules/core/library/Contao/File.php#L176

[aschempp]

We don't need to support isset on them imho.

[Toflar]

So will this be fixed in the Contao Core @leofeyer? For the Widget class at least. Because if so, we can close this PR.

[leofeyer]

Yes, it will be fixed. But we should agree on how to fix it first :)

[Toflar]

Imho we should support it on Widget only for now.

[leofeyer]

I have created a ticket: https://github.com/contao/core/issues/7290

[aschempp]

The problem is fixed in dcawizard meanwhile (we're not checking for empty() anymore), but it should still be fixed in Contao core.