Project

General

Profile

Actions

Bug #81597

closed

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

Added by Michael Stopp almost 7 years ago. Updated over 5 years ago.

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

100%

Estimated time:
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...).


Files

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

Related issues 2 (0 open2 closed)

Related to TYPO3 Core - Task #66995: ObjectAccess::getProperty(Path) performance needs improvementClosed2015-05-16

Actions
Has duplicate TYPO3 Core - Bug #81898: Extbase: List IRRE-children does not work, if using the property attribute of f:form.textfieldClosed2017-07-17

Actions
Actions #1

Updated by Daniel Goerz over 6 years 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.

Actions #2

Updated by Daniel Goerz over 6 years 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.

Actions #3

Updated by Daniel Goerz over 6 years ago

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

Updated by Daniel Goerz over 6 years ago

Issue was introduced here: #66995

Actions #5

Updated by Gerrit Code Review over 6 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 https://review.typo3.org/53525

Actions #6

Updated by Daniel Goerz over 6 years 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.

Actions #7

Updated by Gerrit Code Review over 6 years 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

Actions #8

Updated by Daniel Goerz over 6 years ago

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

Updated by Gerrit Code Review over 6 years 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

Actions #10

Updated by Gerrit Code Review over 6 years 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

Actions #11

Updated by Anonymous over 6 years ago

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

Updated by Daniel Goerz over 6 years ago

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

Updated by Benni Mack over 5 years ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF