Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUGFIX] Fix contentAs feature when used on Layout level #469

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

smichaelsen
Copy link

<f:render section="MySection" contentAs="content">some content</f:section> does not work when called from a Layout because the content variable is not passed to the section in that case.

@coveralls
Copy link

coveralls commented Jul 31, 2019

Coverage Status

Coverage decreased (-0.07%) to 97.091% when pulling 68db27f on smichaelsen:contentas-in-layout into ae2d3d7 on TYPO3:master.

@mbrodala
Copy link
Contributor

Thanks for the report and fix. Would you be able to add a functional test case for this? You can have a look at the existing tests.

@smichaelsen
Copy link
Author

Thanks for your feedback, Mathias. I fear I can't add a test at the moment (deadlines and stuff). Would that block my PR from being merged?

@mbrodala
Copy link
Contributor

Of course not. I'll have a look at this.

@mbrodala mbrodala force-pushed the contentas-in-layout branch from a1631a9 to 4ea540e Compare July 31, 2019 13:46
@mbrodala mbrodala requested a review from NamelessCoder July 31, 2019 13:46
@mbrodala
Copy link
Contributor

@NamelessCoder I've added a test for this now which looks basically correct but still fails. Can you have a look?

Copy link
Contributor

@NamelessCoder NamelessCoder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two main issues need to be addressed:

  1. There is copy/paste content in the example PHP file doc comment and it should not assign a "layout" variable.
  2. More importantly, the way these variables are assigned will "bleed" them after the section is rendered because they are assigned not in a copy of the variable provider, but in the root instance from which copies are derived. This puts the variables in all sections rendered after, makes the variable available to the layout file as well and overwrites any identically named variable that was passed to the template being rendered.

The second point is rather complex, I know. Solving it kind of implies that we must also clone the rendering context when rendering sections-in-template from a layout (same as in the "rendering template" case) and this has other side effects as well because the template no longer has access to the original rendering context.

You could experiment with removing the case I annotated. We might be able to live with the side effects...

@@ -221,6 +221,9 @@ public function renderSection($sectionName, array $variables = [], $ignoreUnknow
if ($this->getCurrentRenderingType() === self::RENDERING_LAYOUT) {
// in case we render a layout right now, we will render a section inside a TEMPLATE.
$renderingTypeOnNextLevel = self::RENDERING_TEMPLATE;
foreach ($variables as $key => $value) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the dangerous line. It operates on the not-cloned RenderingContext retrieved from getCurrentRenderingContext so assigning variables here, bleeds the variables to other sections and the layout's subsequent code.

Copy link
Contributor

@NamelessCoder NamelessCoder Aug 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

        $renderingContext = $this->getCurrentRenderingContext();
        $renderingTypeOnNextLevel = $this->getCurrentRenderingType();
        if ($this->getCurrentRenderingType() === self::RENDERING_LAYOUT) {
            // in case we render a layout right now, we will render a section inside a TEMPLATE.
            $renderingTypeOnNextLevel = self::RENDERING_TEMPLATE;
            if (empty($variables)) {
                $variables = $renderingContext->getVariableProvider()->getAll();
            }
        }
        $variableProvider = $renderingContext->getVariableProvider()->getScopeCopy($variables);
        $renderingContext = clone $renderingContext;
        $renderingContext->setVariableProvider($variableProvider);

`<f:render section="MySection" contentAs="content">some content</f:section>`
did not work when called from a Layout because the `contentAs` variable was
not passed to the template section in that case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants