Modern fragment foundation

[m-vo]

This PR is the foundation for our new core fragment controllers (PR upcoming). Our new fragments will also have new templates and a cleaned up template context, but there are BC layers in place to ease the transition.

Changes

Setup

• Fragments without a template specified now default to the new content_element/<type> and frontend_module/<type>. The template names will always be set directly in the RegisterFragmentsPass. If you are currently using fragment controllers and do not want to update your templates, just specify them explicitly.
• CTE/FMD Definitions made via config.php now have precedence. This allows a by-element/module fallback to the old system. It will also automatically work if someone registered a replacement for a core element/module.

Context

• We operate in two modes, modern and legacy (BC), depending on the template name: if it contains a / and exists as a Twig template we assume modern templates.
• For legacy templates, nothing changes, for modern templates the default context will look somewhat like this:

  $headlineData = StringUtil::deserialize($model->headline ?: '', true);
  $attributesData = StringUtil::deserialize($model->cssID ?: '', true);
  
  $template->setData([
      'type' => $this->getType(),
      'data' => $model->row(),
      'section' => $section,
      'properties' => $properties,
      'attributes' => (new HtmlAttributes())
          ->setIfExists('id', $attributesData[0] ?? null)
          ->addClass($attributesData[1] ?? '', ...$classes),
      'headline' => [
          'text' => $headlineData['value'] ?? '',
          'tagName' => $headlineData['unit'] ?? 'h1',
      ],
  ]);

Compiling a response

• You can now omit the template when calling AbstractFragmentController#render() ($view is now optional).
• In the abstract getReponse() method we now pass a new FragmentTemplate^*) (from the Twig namespace) instead of the legacy FrontendTemplate/FragmentTemplate. It's a simple container object that allows setting template data and calling getResponse() but the latter is simply a closure set by the AbstractFragmentController. Effectively we're then calling $this->render(). This is a huge plus, because now there is only one path no matter if you're using $template->getResponse() or $this->render():

  public function getResponse(FragmentTemplate $template, ContentModel $model, Request $request): Response {
      // variant using the template object
      $template->foo = 'bar';
      return $template->getResponse();
  
      // variant using the render method
      return $this->render(parameters: ['foo' => 'bar']);
  } 

• Modern fragments are Twig only, so for them the legacy template engine won't be used anymore. 🎉

^*) This does not break existing type hints because the new class still extends from the old one. But it sneakily disables calling parent methods, annotates them as @internal and throws an exception when trying to access them. People can now slowly migrate to the new type hint and in Contao 6 the base class can be dropped.

Todo

• rebase once Allow to json-serialize the HtmlAttributes class #4381 is merged
• move unrelated but still needed things out of the old template classes (maybe in a followup PR) see Move unrelated functionality out of legacy template engine #4403
• add some more tests

[ausi]

Some minor notes/nitpicks/questions.
Concept LGTM 🚀

[aschempp]

Two initial thoughts:

1. I think in legacy mode, we should continue to pass a FrontendTemplate instance to the getResponse() method. Maybe someone is using template methods, and I don't see an advantage to build it lazily?

2. all the protected methods that are now supposed to be made private/be removed for Contao 6 are intentionally splitting code/logic to be overridden by child controllers, for the case that e.g. a content element does not have a the default fields. So e.g. if my content element does not have headlines, I would override the addHeadlineToTemplate method to noop. Are we sure this feature should be dropped?

[m-vo]

all the protected methods that are now supposed to be made private/be removed for Contao 6 are intentionally splitting code/logic to be overridden by child controllers, for the case that e.g. a content element does not have a the default fields. So e.g. if my content element does not have headlines, I would override the addHeadlineToTemplate method to noop. Are we sure this feature should be dropped?

IMHO yes. I think the reasoning might depend on if the AbstractFragmentController is supposed to work for any table/model or if its limited to content elements and modules.

• If it's limited, I'm not seeing any benefit of the granular approach: These two scenarios always have a headline, attributes and so on because they always use the same tables/models that include these features. You're free to not use the data in your templates and your controller can even overwrite/drop any of the default context if needed. Do you have an example where you adjusted addHeadlineToTemplate()?

• In case it's not limited, you would need to implement addDefaultDataToTemplate() and set the whole default context. This looks fine to me - it's only a few lines of code. In fact, then, I would be in favor of dropping the method altogether and copying the code to both AbstractContentElementController and AbstractFrontendModuleController - because then there should not be an abstraction at all.

[Toflar]

Also 👍 on the concept, except for the questions raised by @aschempp. Let's see if he has a use case.

[aschempp]

If it's limited, I'm not seeing any benefit of the granular approach: These two scenarios always have a headline, attributes and so on because they always use the same tables/models that include these features. You're free to not use the data in your templates and your controller can even overwrite/drop any of the default context if needed. Do you have an example where you adjusted addHeadlineToTemplate()?

The following core content elements do not have a headline in the palette definition for example:

• html
• accordionStart
• accordionStop
• accordionSingle
• sliderStop
• toplink
• alias
• article
• teaser
• module

[m-vo]

The following core content elements do not have a headline in the palette definition for example: […]

True, but you would not remove the headline data for those, you would just not include the headline fragment in the template (or use another base template), no? Leaving it in for these elements also has the benefit, that I, as a user, can simply add the headline to the palette and template if I need it without having to create my own controller.

[m-vo]

I think in legacy mode, we should continue to pass a FrontendTemplate instance to the getResponse() method. Maybe someone is using template methods, and I don't see an advantage to build it lazily?

This change is intentional: Now the template type inside the getResponse() methods can be narrowed to FragmentTemplate and people can start migrating away from the now deprecated Template to the new FragmentTemplate in their existing controllers. Otherwise there would not be a smooth upgrade path.

Using template methods other than getResponse() sounds wrong to me. I would drop support for this in Contao 5. An exception could maybe be parse(), but we already discouraged the use of that because it did not set the correct headers.

[m-vo]

Before this PR $request->attributes->get('templateProperties', []) are merged into the template context before the abstract getResponse() is called. It's similar with the current state of the PR though it's conflict-free under a properties key.

But maybe we are missing the point in both the old and the new implementation of what this functionality tries to achieve. If it is setting (model) data, so that a controller will behave differently, I'd suggest the following changes (example for content elements, analogous for modules) in AbstractContentElementController:

• always detach the model before calling the abstract getResponse()
• merge the templateProperties into the model data
• maybe even allow ContentModel|array in __invoke

[ausi]

🎉

[ausi]

If it is setting (model) data, so that a controller will behave differently, I'd suggest the following changes

Agree. I think modifying the model data (after detaching it) is the correct way for this use case.
Type hint ContentModel|array is not possible for __invoke due to BC AFAIK.

[Toflar]

Looks excellent! Just some nitpicks but I'll approve the concept anyway.

[leofeyer]

The template structure needs to be adjusted before we merge this.

1. I don‘t like the _new subfolder. Is this only temporary?
2. We never use back_end (two words) in the code base. It is always backend.

[ausi]

❤️

[leofeyer]

Thank you @m-vo.

[bytehead]

This is great! ❤️