Skip to content

Commit

Permalink
[BUGFIX] Dynamically set 'clear' TypoScript flag
Browse files Browse the repository at this point in the history
Not too many people know and understand the details
of the 'clear constants' and 'clear setup' flags in
TypoScript sys_template records: It not only
resets any given tree coming from previous templates,
but also triggers inclusion of TypoScript from
globals. As such, it is important to set the flags
somewhere, usually for the first template row. This
is also the main difference between the "Create
template for a new site" and "Create an extension
template" buttons: "New site" sets the clear flags
to trigger globals inclusion.

The old TypoScript parser had a convenience solution
to still include globals in case no template record
did set the 'clear' flags properly.

The new parser did not do that. We got feedback from
multiple early v12 adopters on this, who stumbled
here: If the clear flags are not set, especially the
basic rendering definitions from fluid_styled_content
are missing. Since this issue can be pretty hard to
find, the patch changes the SysTemplateTreeBuilder to
set the clear flag on the first template row if no
other row sets it. As a result, this increases b/w
compat with the old parser and integrators can
essentially forget about this flag for casual simple
setups again.

Resolves: #99331
Related: #97816
Related: #98552
Change-Id: I839e0fd04ff432911ff59bc6ec2047eb1e53be3e
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/77010
Tested-by: core-ci <typo3@b13.com>
Tested-by: André Kraus <info@andrekraus.com>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Stefan Bürk <stefan@buerk.tech>
Reviewed-by: André Kraus <info@andrekraus.com>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
  • Loading branch information
lolli42 committed Dec 10, 2022
1 parent 4ca6a0c commit 2c593c0
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ public function __construct(
* @param ServerRequestInterface|null $request Nullable since Request is not a hard dependency ond just convenient for the Event
*
* @todo: It's potentially possible to get rid of this method in the frontend by joining sys_template
* into the Page rootline resolving as soon as it uses a CTE.
* into the Page rootline resolving as soon as it uses a CTE: This would save one query in *all* FE
* requests, even for fully-cached page requests.
*/
public function getSysTemplateRowsByRootline(array $rootline, ?ServerRequestInterface $request = null): array
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,28 @@ public function getTreeBySysTemplateRowsAndSite(
$this->includedSysTemplateUids = [];

$includeTree = new RootInclude();
if (empty($sysTemplateRows)) {
return $includeTree;
}

// Convenience code: Usually, at least one sys_template records needs to have 'clear' set. This resets
// the AST and triggers inclusion of "globals" TypoScript. When integrators missed to set the clear flags,
// important globals TypoScript is not loaded, leading to pretty hard to find issues in Frontend
// rendering. Since the details of the 'clear' flags are rather complex anyway, this code scans the given
// sys_template records if the flag is set somewhere and if not, actively sets it dynamically for the
// first templates. As a result, integrators do not need to think about the 'clear' flags at all for
// simple instances, it 'just works'.
$atLeastOneSysTemplateRowHasClearFlag = false;
foreach ($sysTemplateRows as $sysTemplateRow) {
if (($this->type === 'constants' && $sysTemplateRow['clear'] & 1) || ($this->type === 'setup' && $sysTemplateRow['clear'] & 2)) {
$atLeastOneSysTemplateRowHasClearFlag = true;
}
}
if (!$atLeastOneSysTemplateRowHasClearFlag) {
$firstRow = reset($sysTemplateRows);
$firstRow['clear'] = $this->type === 'constants' ? 1 : 2;
$sysTemplateRows[array_key_first($sysTemplateRows)] = $firstRow;
}

foreach ($sysTemplateRows as $sysTemplateRow) {
$identifier = $this->getSysTemplateRowCacheIdentifier($sysTemplateRow);
Expand All @@ -136,9 +158,7 @@ public function getTreeBySysTemplateRowsAndSite(
$includeNode->setRoot(true);
}
$clear = $sysTemplateRow['clear'];
if (($this->type === 'constants' && $clear & 1)
|| ($this->type === 'setup' && $clear & 2)
) {
if (($this->type === 'constants' && $clear & 1) || ($this->type === 'setup' && $clear & 2)) {
$includeNode->setClear(true);
}
$this->handleSysTemplateRecordInclude($includeNode, $sysTemplateRow, $site);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
"pages"
,"uid","pid","sorting","deleted","t3_origuid","t3ver_wsid","t3ver_state","t3ver_stage","t3ver_oid","title","slug"
,1,0,256,0,0,0,0,0,0,"FunctionalTest","/"
,2,1,256,0,0,0,0,0,0,"Sub","/sub"
"sys_template"
,"uid","pid","sorting","deleted","hidden","starttime","endtime","t3_origuid","root","clear","include_static_file","constants","config","basedOn","includeStaticAfterBasedOn","static_file_mode"
,1,1,256,0,0,0,0,0,1,0,"","foo=fooValue","","",0,0
,2,2,256,0,0,0,0,0,0,0,"","bar=barValue","","",0,0
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
"pages"
,"uid","pid","sorting","deleted","t3_origuid","t3ver_wsid","t3ver_state","t3ver_stage","t3ver_oid","title","slug"
,1,0,256,0,0,0,0,0,0,"FunctionalTest","/"
,2,1,256,0,0,0,0,0,0,"Sub","/sub"
"sys_template"
,"uid","pid","sorting","deleted","hidden","starttime","endtime","t3_origuid","root","clear","include_static_file","constants","config","basedOn","includeStaticAfterBasedOn","static_file_mode"
,1,1,256,0,0,0,0,0,1,0,"","","foo=fooValue","",0,0
,2,2,256,0,0,0,0,0,0,0,"","","bar=barValue","",0,0
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,68 @@ public function twoPagesTwoTemplates(): void
self::assertSame('barValue', $ast->getChildByName('bar')->getValue());
}

/**
* @test
*/
public function twoPagesTwoTemplatesWithoutClearForConstantsStillLoadsFromGlobals(): void
{
$this->importCSVDataSet(__DIR__ . '/Fixtures/SysTemplate/twoPagesTwoTemplatesNoClearForConstants.csv');
$GLOBALS['TYPO3_CONF_VARS']['FE']['defaultTypoScript_constants'] = 'globalsConstant = globalsConstantValue';
$rootline = [
[
'uid' => 2,
'pid' => 1,
'is_siteroot' => 0,
],
[
'uid' => 1,
'pid' => 0,
'is_siteroot' => 0,
],
];
/** @var SysTemplateRepository $sysTemplateRepository */
$sysTemplateRepository = $this->get(SysTemplateRepository::class);
/** @var SysTemplateTreeBuilder $subject */
$subject = $this->get(SysTemplateTreeBuilder::class);
$includeTree = $subject->getTreeBySysTemplateRowsAndSite('constants', $sysTemplateRepository->getSysTemplateRowsByRootline($rootline), new LossyTokenizer());
self::assertEquals($includeTree, unserialize(serialize($includeTree)));
$ast = $this->getAst($includeTree);
self::assertSame('fooValue', $ast->getChildByName('foo')->getValue());
self::assertSame('barValue', $ast->getChildByName('bar')->getValue());
self::assertSame('globalsConstantValue', $ast->getChildByName('globalsConstant')->getValue());
}

/**
* @test
*/
public function twoPagesTwoTemplatesWithoutClearForSetupStillLoadsFromGlobals(): void
{
$this->importCSVDataSet(__DIR__ . '/Fixtures/SysTemplate/twoPagesTwoTemplatesNoClearForSetup.csv');
$GLOBALS['TYPO3_CONF_VARS']['FE']['defaultTypoScript_setup'] = 'globalsKey = globalsValue';
$rootline = [
[
'uid' => 2,
'pid' => 1,
'is_siteroot' => 0,
],
[
'uid' => 1,
'pid' => 0,
'is_siteroot' => 0,
],
];
/** @var SysTemplateRepository $sysTemplateRepository */
$sysTemplateRepository = $this->get(SysTemplateRepository::class);
/** @var SysTemplateTreeBuilder $subject */
$subject = $this->get(SysTemplateTreeBuilder::class);
$includeTree = $subject->getTreeBySysTemplateRowsAndSite('setup', $sysTemplateRepository->getSysTemplateRowsByRootline($rootline), new LossyTokenizer());
self::assertEquals($includeTree, unserialize(serialize($includeTree)));
$ast = $this->getAst($includeTree);
self::assertSame('fooValue', $ast->getChildByName('foo')->getValue());
self::assertSame('barValue', $ast->getChildByName('bar')->getValue());
self::assertSame('globalsValue', $ast->getChildByName('globalsKey')->getValue());
}

/**
* @test
*/
Expand Down

0 comments on commit 2c593c0

Please sign in to comment.