Bug #60135

Nested stdWraps cause multiple "function calls"

Added by Philipp Kitzberger over 5 years ago. Updated about 1 year ago.

Status:
Closed
Priority:
Must have
Assignee:
Category:
TypoScript
Target version:
Start date:
2014-07-07
Due date:
% Done:

100%

TYPO3 Version:
6.2
PHP Version:
Tags:
Complexity:
easy
Is Regression:
No
Sprint Focus:

Description

In case of nested stdWraps, the specified objects (like TEXT or USER) will be called multiple times even though the output is still only being rendered once. This sucks especially for userfuncs and REGISTER (the "famous" counter scenario).

page = PAGE
page {
  10 = TEXT
  10.value = page.10.value<br>
  10.append = TEXT
  10.append.value = page.10.append.value<br>
  10.stdWrap.append = TEXT
  10.stdWrap.append.value = page.10.stdWrap.append.value<br>
}

The output in the browser is as expected:

page.10.value
page.10.stdWrap.append.value
page.10.append.value

If you now modify the TEXT class (typo3/sysext/frontend/Classes/ContentObject/TextContentObject.php) and prepend the following to render():

var_dump($conf['value']);

you'll see that this object is acutally being called not 3 but 4 times! I guess it's also possible to track this behaviour with the admin panel.

Anyway, to render "page.10.stdWrap.append.value" its TEXT object is being called two times, which is one time too much, imho. If you wrap it within another stdWrap, it's even called 4 times.

Associated revisions

Revision 23b63c2b (diff)
Added by Markus Klein about 5 years ago

[!!!][BUGFIX] Avoid to call stdWrap twice

Using the recursive stdWrap will work on the stdWrap twice, although the
output is created only once.

Fix this by preventing the recursive call to stdWrap() if the
current function name is "stdWrap" as this will trigger a call to
stdWrap_stdWrap() later on anyway.

This will change rendering if LOAD_REGISTER or something similar
is used in a recursive stdWrap context.

Resolves: #60135
Releases: 6.3
Change-Id: I728f637b4e34f26b9cf6951f04667c8195638b3d
Reviewed-on: http://review.typo3.org/32466
Reviewed-by: Helmut Hummel <>
Tested-by: Helmut Hummel <>
Reviewed-by: Stefan Froemken <>
Tested-by: Stefan Froemken <>
Tested-by: Markus Klein <>

Revision a4897da0 (diff)
Added by Olaf Schmidt-Wischhöfer over 3 years ago

[!!!][BUGFIX] Evaluate "boolean /stdWrap" properties correctly

stdWrap sub-properties on boolean properties have to be evaluated
correctly.

Resolves: #75493
Related: #60135
Releases: master
Change-Id: I7ab418a97228beb49458fa43e5d0ce50aef83877
Reviewed-on: https://review.typo3.org/48008
Reviewed-by: Markus Klein <>
Tested-by: Markus Klein <>
Reviewed-by: Nicole Cordes <>
Tested-by: Nicole Cordes <>
Reviewed-by: Oliver Hader <>
Reviewed-by: Frank Naegler <>
Tested-by: Frank Naegler <>

History

#1 Updated by Markus Klein over 5 years ago

Interesting finding, thank you!

#2 Updated by Stefan Neufeind over 5 years ago

I can confirm multiple calls to a function. In my case there a userFunc is used:

something.stdWrap.postUserFunc = ...

That userFunc is called twice from within typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php. Once around row 2080 in the line

$conf[$functionName] = $this->stdWrap($conf[$functionName], $conf[$functionProperties]);

and then in 2103 with the call

$content = $this->{$functionName}($content, $singleConf);

#3 Updated by Gerrit Code Review about 5 years ago

  • Status changed from New to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/32466

#4 Updated by Markus Klein about 5 years ago

  • Category set to TypoScript
  • Assignee set to Markus Klein
  • Priority changed from Should have to Must have
  • Target version set to next-patchlevel
  • Complexity set to easy

The provided patch yields a performance increase of around 5 to 10% for the sample TS.
(ab -n 1000 and no_cache=1)

#5 Updated by Gerrit Code Review about 5 years ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/32466

#6 Updated by Gerrit Code Review about 5 years ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/32466

#7 Updated by Helmut Hummel about 5 years ago

Given the following TS:

page.1 = TEXT
page.1 {
  value = Counter:
  append = TEXT
  append.data = register:Counter
  stdWrap.append = LOAD_REGISTER
  stdWrap.append {
    Counter.cObject = TEXT
    Counter.cObject.data = register:Counter
    Counter.cObject.wrap = |+1    
    Counter.prioriCalc = 1
  }
} 

Output without patch:
Counter:2

Output with patch:
Counter:1

#8 Updated by Markus Klein about 5 years ago

Thanks for the example. But this is a clear example of a TS workaround to get a value of 2, because of this bug.
If the bug didn't exist, nobody would have written such a TS snippet. (It looks strange to add 1 and get 2 as a result.)

#9 Updated by Stefan Neufeind about 5 years ago

Just looking at the example I'm with Markus here. Imho you replicated that the fix actually fixed an unexpected behaviour.

#10 Updated by Gerrit Code Review about 5 years ago

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/32466

#11 Updated by Helmut Hummel about 5 years ago

Stefan Neufeind wrote:

Just looking at the example I'm with Markus here. Imho you replicated that the fix actually fixed an unexpected behaviour.

People work around unexpected behavior, especially in TypoScript. We will break such workarounds.

Anyway, if the release managers of 6.2 and 6.1 agree that this fix should go into these releases, I'm fine.

#12 Updated by Gerrit Code Review about 5 years ago

Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/32466

#13 Updated by Markus Klein about 5 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100

#14 Updated by Benni Mack about 1 year ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF