BC break with protected $Template property

[bytehead]

Affected version(s)

4.9.26

Description

The fix from #4075 has still a BC break in PageRegular:

[fritzmg]

Yes, we still need to decide whether accessing ->Template of PageRegular publicly should be allowed or not. In the past it was allowed, but probably an oversight. However, even if it was not intentional, the current state is inconsistent. Module and ContentElement allow you to publicly access the Template property. PageRegular and BackendMain do not.

[m-vo]

Can we allow it in 4.13 and ultimately drop it everywhere in 5.0?

[bytehead]

Before 4.9.24 you could actually access Template in PageRegular therefore we cannot change this in 4.x IMO.

[bytehead]

Or how would you add some properties to the page template?

[fritzmg]

Or how would you add some properties to the page template?

I would do it via the parseTemplate hook.

[ausi]

Can we make it a public member in Contao 4.9/4.13 and private in Contao 5.0?

[leofeyer]

Huh? I don‘t understand. I have merged the original fix, so why is this breaking now?

[bytehead]

Then this was also a BC break then.

[bytehead]

Can we make it a public member in Contao 4.9/4.13 and private in Contao 5.0?

Like discussed already: #3894 (comment) 😅

[aschempp]

jup, it must be public. Or probably just not be set at all. I noticed that too while reading the PR 😂

[bytehead]

I'm in for making it public, I doubt that people made the property protected or private by themself. But otherwise, why not leave it as it was and don't set it at all until 5.0? 🤷‍♂️

[leofeyer]

Creating dynamic properties is deprecated as of PHP 8.2, which is why this issue came up in the first place.

https://github.com/contao/contao/runs/4498623325?check_suite_focus=true

[bytehead]

True, I've already missed that again. So making it public would be the only (futureproof) way.

[aschempp]

How about adding a __get() method that return the template object? This would work in any case. If a developer did make it public in a child class that property would be accessed, and in any other case our __get() method would be called?

[fritzmg]

You mean: leave everything as it was before (i.e. reverting #4075 again 🙈) and only implement __get for Template for PageRegular? (May be also BackendMain)

[fritzmg]

There is another issue. We cannot revert #4075 because there are actually extensions who directly access $this->arrData['Template']: https://github.com/DERHAEUPTLING/contao-lazy-images/blob/9edd71b3e8d632ad2ad66329c519a8b3a9e0686a/elements/LazyContentImage.php#L26

[m-vo]

Creating dynamic properties is deprecated as of PHP 8.2, which is why this issue came up in the first place.

If we cannot guarantee BC otherwise, we could add #[AllowDynamicProperties] in this special case I think.

[leofeyer]

As discussed in the Contao call, we do want to add #[AllowDynamicProperties].

[ausi]

And use a protected property in Contao 5.0 and remove #[AllowDynamicProperties] then.