MaxServ / t3ext-fal_s3

S3 driver for FAL
GNU General Public License v2.0
13 stars 10 forks source link

[Bugfix] File list showing all files when number of folder exceeds number of items (iLimit / filesPerPage) shown #50

Closed maechler closed 5 years ago

maechler commented 5 years ago

Currently there is a bug when the number of folders (e.g. 42) exceeds the limit of items shown (e.g. 40) in the file list. TYPO3 handles this special case with setting $filesFrom=false; and $filesNum=false;. (Better would probably be $filesFrom=0; and $filesNum=0;, but that's just how it is.)

See https://github.com/TYPO3/TYPO3.CMS/blob/2c8df8f59068e1861606ff1dc0f35c693ffd5393/typo3/sysext/filelist/Classes/FileList.php#L303-L306

The way getFilesInFolder is implemented at the moment results in showing all files for this special case, because we pass null as length to array_slice:

https://github.com/MaxServ/t3ext-fal_s3/blob/b3a0fc62b24ef514d0cd03eee82b5027fb9e9126/Classes/Driver/AmazonS3Driver.php#L928-L932

This pull request fixes the issue and makes the fal_s3 driver behave like the local driver in the file list.

maechler commented 5 years ago

Thanks for having a look at this! Let's consider an example with 3 folders, 3 files and a limit of 2.

Current behaviour

Page 1: folder 1, folder 2, file 1, file 2, file 3 -> folders correct, files wrong Page 2: folder 3, file 1 -> both correct, because both are paginated Page 3: folder 1, folder 2, folder 3, file 2, file 3 -> folders wrong, files correct

Page one falsely shows all files, page three falsely shows all folders. This happens because on page one the parameters for from and num are set to false for files and on page three they are set to false for folders. My initial claim, that this is only an issue with more folders than items visible, is wrong. It is an issue as soon as there are folders and files in one folder and a pagination occurs. Because the folders falsely show up on later pages where only files should be.

Desired / new behaviour

Page 1: folder 1, folder 2 Page 2: folder 3, file 1 Page 3: file 2, file 3

maechler commented 5 years ago

@IchHabRecht The parameters should be int, however their type is never enforced (Or am I missing something?). As mentioned in my pull request description, the core sets them to false in the following cases:

  1. https://github.com/TYPO3/TYPO3.CMS/blob/2c8df8f59068e1861606ff1dc0f35c693ffd5393/typo3/sysext/filelist/Classes/FileList.php#L292-L295

  2. https://github.com/TYPO3/TYPO3.CMS/blob/2c8df8f59068e1861606ff1dc0f35c693ffd5393/typo3/sysext/filelist/Classes/FileList.php#L303-L306

Probably it would be better if the core set them to 0 instead of false in order to be consistent with the types.