Bug #56084

t3editor is not usable any more

Added by Alexander Schnitzler over 1 year ago. Updated over 1 year ago.

Status:Resolved Start date:2014-02-18
Priority:Must have Due date:
Assigned To:Frans Saris % Done:

100%
Category:t3editor Spent time: 2.00 hours
Target version:6.2.0
TYPO3 Version:6.2 Is Regression:Yes
PHP Version: Sprint Focus:
Complexity:

Description

Regression has been introduced in #55371

The actual problem of the change is the following:

-            $ajaxObj->addContent('result', $this->fileData);
+            $fileData = array();
+            foreach ($this->fileData as $action => $result) {
+                foreach ($result as $files) {
+                    /** @var $file \TYPO3\CMS\Core\Resource\File */
+                    foreach ($files as $file) {
+                        $fileData[$action][] = $file->toArray();
+                    }
+                }
+            }
+            $ajaxObj->addContent('result', $fileData);

Don't know what that should have fixed as there is not a single unit test for the change but if you save a file edited with t3editor in the list module, $this->fileData is as such:

array(
  'editfile' => array(
    0 => true
  )
)

This results in $files being true instead of array in the second foreach loop. So the third foreach loop will not be executed and therefore $fileData is an empty array.

Related issues

related to Core - Bug #55371: Ajax handler TYPO3_tcefile::process is broken Resolved 2014-01-27
blocks Core - Task #55515: Add CSRF Protection for tce_file.php Resolved 2014-01-31

Associated revisions

Revision 6cc6c4f7
Added by Frans Saris over 1 year ago

[BUGFIX] Followup: Ajax handler TYPO3_tcefile::process is broken

Upload action was taken care of, but the ajax handler can be just
for all commands that ExtendedFileUtility->processData can handle.

This change checks the result set and flattens
data only when needed.

Resolves: #56084
Releases: 6.2, 6.1
Change-Id: Ic1a0bd9084b9eb206b9b53960890d22d2a9c56f5
Reviewed-on: https://review.typo3.org/27739
Reviewed-by: Alexander Schnitzler
Tested-by: Alexander Schnitzler
Reviewed-by: Markus Klein
Tested-by: Markus Klein

Revision 3800d8bd
Added by Frans Saris over 1 year ago

[BUGFIX] Followup: Ajax handler TYPO3_tcefile::process is broken

Upload action was taken care of, but the ajax handler can be just
for all commands that ExtendedFileUtility->processData can handle.

This change checks the result set and flattens
data only when needed.

Resolves: #56084
Releases: 6.2, 6.1
Change-Id: Ic1a0bd9084b9eb206b9b53960890d22d2a9c56f5
Reviewed-on: https://review.typo3.org/27863
Reviewed-by: Frans Saris
Tested-by: Frans Saris
Reviewed-by: Markus Klein
Tested-by: Markus Klein

History

#1 Updated by Alexander Opitz over 1 year ago

  • Assigned To changed from Alexander Opitz to Alexander Schnitzler

#2 Updated by Alexander Schnitzler over 1 year ago

  • Assigned To changed from Alexander Schnitzler to Frans Saris

Frans, can you please have a look at it?

#3 Updated by Frans Saris over 1 year ago

Hi Alexander,

            $fileData = array();
            foreach ($this->fileData as $action => $result) {
                if ($action === 'upload') {
                    foreach ($result as $files) {
                        /** @var $file \TYPO3\CMS\Core\Resource\File */
                        foreach ($files as $file) {
                            $fileData[$action][] = array_merge(
                                $file->toArray(),
                                array (
                                    'date' => BackendUtility::date($file->getModificationTime()),
                                    'iconClasses' => \TYPO3\CMS\Backend\Utility\IconUtility::mapFileExtensionToSpriteIconClass($file->getExtension()),
                                )
                            );
                        }
                    }
                } else {
                    $fileData[$action] = $result;
                }
            }
            $ajaxObj->addContent('result', $fileData);

This fixes the problem.
But I'm not sure if there are more 'actions' that return File objects like upload. When I made this change I search for other uses of the ajax api but couldn't find any. Do you know if there are more?

#4 Updated by Alexander Schnitzler over 1 year ago

Thanks but that really feels wrong. An API should be consistent and clear and not handle several different cases with if, else, and multiple foreach loops.

Let's dig into it and find out where these calls are used and what for. What do you think?

#5 Updated by Gerrit Code Review over 1 year ago

  • Status changed from Accepted 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/27739

#6 Updated by Frans Saris over 1 year ago

The result set can have multiple results.

[delete][] = boolean
[copy][] = File | Folder
[move][] = File | Folder
[rename][] = File | Folder
[newfolder][] = Folder
[newfile][] = File
[editfile][] = boolean
[upload][][] = File
[unzip][] = boolean

I have changed the processAjaxRequest() so it loops through the result set and flattens the possible values.

#7 Updated by Gerrit Code Review over 1 year 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/27739

#8 Updated by Gerrit Code Review over 1 year 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/27739

#9 Updated by Gerrit Code Review over 1 year ago

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

#10 Updated by Gerrit Code Review over 1 year ago

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

#11 Updated by Markus Klein over 1 year ago

I don't get the problem. I can edit text files as usual in the filelist module.

Edit: Got the problem. It's about the Save button.

#12 Updated by Gerrit Code Review over 1 year ago

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

#13 Updated by Gerrit Code Review over 1 year ago

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

#14 Updated by Gerrit Code Review over 1 year ago

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

#15 Updated by Gerrit Code Review over 1 year ago

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

#16 Updated by Gerrit Code Review over 1 year ago

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

#17 Updated by Gerrit Code Review over 1 year ago

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

#18 Updated by Frans Saris over 1 year ago

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

Also available in: Atom PDF