Bug #81597

property attribute of form input field viewhelper breaks f:for each loop

Added by Michael Stopp 5 months ago. Updated 4 months ago.

Status:
Resolved
Priority:
Must have
Assignee:
Category:
Fluid
Target version:
Start date:
2017-06-16
Due date:
% Done:

100%

TYPO3 Version:
8
PHP Version:
7.0
Tags:
Complexity:
Is Regression:
Yes
Sprint Focus:

Description

While migrating a project with a large Fluid form from 6.2 to 8.7, I noticed that some of my loops (e.g. to iterate over child objects) would only print the first child element. This code used to work in 6.2:

<f:for each="{film.director}" as="director" iteration="i">
    {director.uid}: <f:form.textfield property="director.{i.index}.firstName" /><br>
</f:for>

Let's say the output for this was:

12: John
37: Anna

In 8.7 the output for the same code is:

12: John

I first thought it was a problem with the iterator variable. But even if I do this (which doesn't make sense of course), it doesn't work (i.e. there will only be one iteration of the loop):

<f:for each="{film.director}" as="director" iteration="i">
    {director.uid}: <f:form.textfield property="director.0.firstName" /><br>
</f:for>

When I avoid the property attribute, it does work:

<f:for each="{film.director}" as="director" iteration="i">
    {director.uid}: <f:form.textfield name="tx_cudist_myfilms[film][director][{i.index}][firstName]" value="{director.firstName}" /><br>
</f:for>

I only got the property attribute to work inside a loop, when I used the VHS iterator viewhelper:

<v:iterator.for from="0" to="1" iteration="i">
    {film.director.{i.index}.uid}: <f:form.textfield property="director.{i.index}.firstName" /><br>
</v:iterator.for>

FYI: I first encountered this problem in the context of a viewhelper for FAL upload fields. My viewhelper is based on Helmut Hummel's sample project (https://github.com/helhum/upload_example) for file uploads. When I commented out the call to $this->getValueAttribute(); in the viewhelper class, the f:for loop wouldn't break anymore (but it obviously broke the viewhelper...).

object_access.tar.gz (2.24 KB) Daniel Goerz, 2017-07-15 18:52


Related issues

Related to TYPO3 Core - Task #66995: ObjectAccess::getProperty(Path) performance needs improvement Resolved 2015-05-16
Duplicated by TYPO3 Core - Bug #81898: Extbase: List IRRE-children does not work, if using the property attribute of f:form.textfield Closed 2017-07-17

Associated revisions

Revision 84131835 (diff)
Added by Daniel Goerz 4 months ago

[BUGFIX] Clone ObjectStorage in ObjectAccess

The ObjectStorage Object passed to iterator_to_array() ends up
with the pointer at the last item, so that any ongoing iteration
through the ObjectStorage wrapping the call of ObjectAccess is
interrupted.
This patch ensures that the ObjectStorage is cloned before converted
to an array so that the original object keeps it original iteration
state.

Resolves: #81597
Relates: #66995
Releases: master, 8.7
Change-Id: Ied025ff616e100cc5eb5dedd5b7b6a95293ddfcf
Reviewed-on: https://review.typo3.org/53525
Tested-by: TYPO3com <>
Reviewed-by: Markus Klein <>
Reviewed-by: Claus Due <>
Reviewed-by: Sascha Rademacher <>
Tested-by: Sascha Rademacher <>
Reviewed-by: Benjamin Kluge <>
Tested-by: Benjamin Kluge <>
Reviewed-by: Susanne Moog <>
Tested-by: Susanne Moog <>

Revision a8d502ac (diff)
Added by Daniel Goerz 4 months ago

[BUGFIX] Clone ObjectStorage in ObjectAccess

The ObjectStorage Object passed to iterator_to_array() ends up
with the pointer at the last item, so that any ongoing iteration
through the ObjectStorage wrapping the call of ObjectAccess is
interrupted.
This patch ensures that the ObjectStorage is cloned before converted
to an array so that the original object keeps it original iteration
state.

Resolves: #81597
Relates: #66995
Releases: master, 8.7
Change-Id: Ied025ff616e100cc5eb5dedd5b7b6a95293ddfcf
Reviewed-on: https://review.typo3.org/53533
Tested-by: TYPO3com <>
Reviewed-by: Susanne Moog <>
Tested-by: Susanne Moog <>

History

#1 Updated by Daniel Goerz 4 months ago

Claus shared some thoughts regarding this issue on slack, I quote it here:

this can probably be explained by pointers and Iterator, combined with not cloning the iterator and using foreach() to determine which object you reference when you use `0` as index in an ObjectStorage

logic flow would be: 1) you start iterating outside, 2) you try to access the property with numeric index, 3) ObjectAccess::getPropertyPath does iteration of your ObjectStorage to find the member, 4) pointer is left at the end of the ObjectStorage(Iterator) and the loop terminates early.

this would probably also have been worse if the pointer was actually reset after iteration (there are several threads about this problem. TL;DR pointer reset works one way for arrays and another for Iterators)

the reason you don't see it when using VHS iterator is you avoid the outer iteration and instead access each member directly (causing a foreach($objectStorage) on each access, but resulting in a working loop)

if that's right then fixing it becomes a pretty complicated issue

you might be tempted to use iterator_to_array($objectStorage)[$index] but that wouldn't really be efficient

you might also be tempted to `clone $objectStorage` in ObjectAccess but that could easily cause unwanted side effects for objects defining `__clone`

my straight-forward recommendation would be to be aware of this pointer thing and design your code accordingly. It would also apply if you attempt the same procedure in PHP directly.

#2 Updated by Daniel Goerz 4 months ago

To give some more information: Claus is right. The same ObjectStorageObject is iterated inside its first iteration again, thus exiting the first (outer) loop after the first iteration. The same thing happens obviously if you write the following:

$objectStorage = $object->getObjectStorageProperty();
foreach ($objectStorage as $object) {
  // This would be the loop of the f:for ViewHelper
  echo 'outer loop';
  foreach ($objectStorage as $object) {
    // This would be the loop of in ObjectAccess to find the property value
    echo 'inner loop';
  }
}

This will give you only one 'outer loop'. Suggested solution is to clone the $objectStorage within the objectAccess class (where the second loop happens) which was tested successfully but not pushed yet, due to possible sideffects. It would also be possible to clone the $objectStorage before the loop in the f:for ViewHelper but this might lead to even more unwanted sideeffects.

A workaround for now is to convert the objectStorage to an array ($objectStorage->toArray()) before passing it to f:for ViewHelper.

#3 Updated by Daniel Goerz 4 months ago

  • Related to Task #66995: ObjectAccess::getProperty(Path) performance needs improvement added

#4 Updated by Daniel Goerz 4 months ago

Issue was introduced here: #66995

#5 Updated by Gerrit Code Review 4 months 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 https://review.typo3.org/53525

#6 Updated by Daniel Goerz 4 months ago

Added an extension (object-access) to verify (and understand) the Issue. SImply install it. The plugin is added to page.50. PLease adjust if this does not fit your system configuration.

#7 Updated by Gerrit Code Review 4 months ago

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

#8 Updated by Daniel Goerz 4 months ago

  • Assignee set to Daniel Goerz
  • Target version set to next-patchlevel
  • PHP Version set to 7.0
  • Is Regression set to Yes

#9 Updated by Gerrit Code Review 4 months ago

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

#10 Updated by Gerrit Code Review 4 months ago

Patch set 1 for branch TYPO3_8-7 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/53533

#11 Updated by Anonymous 4 months ago

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

#12 Updated by Daniel Goerz 4 months ago

  • Duplicated by Bug #81898: Extbase: List IRRE-children does not work, if using the property attribute of f:form.textfield added

Also available in: Atom PDF