? Failure

User tests: Successful: Unsuccessful:

avatar eshiol
eshiol
13 Jul 2017

Pull Request for Issue #16708

Summary of Changes

This patch fixes JFormFieldFolderList. See: #16708

Testing Instructions

#16708 This patch should fix this behavior

Expected result

Field "list of images" works correctly.

avatar eshiol eshiol - open - 13 Jul 2017
avatar eshiol eshiol - change - 13 Jul 2017
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 13 Jul 2017
Category Libraries
avatar zero-24
zero-24 - comment - 13 Jul 2017

This would always set the path to the root and not respect the configruation.

avatar eshiol
eshiol - comment - 13 Jul 2017

This sets the path to the directory passed via configuration starting from the root directory.

$path = $this->directory;
$path = JPATH_ROOT . '/' . $path;
avatar zero-24
zero-24 - comment - 13 Jul 2017

Ah looks like i have not looked close enough.

avatar zero-24
zero-24 - comment - 13 Jul 2017

Can you please include a issue description + test instructions?

avatar eshiol
eshiol - comment - 14 Jul 2017

I wrote a test component called com_pr17152
com_pr17125-3.7.3.3.zip
It creates a folder called "this is the right folder" in the folder components/com_pr17125 and a folder called "this is the wrong folder" in the folder administrator/components/com_pr17125.
In the configuration page you can choose the folder using a folder list field
<field name="file_path" type="folderlist" directory="components/com_pr17125" hide_none="true" hide_default="true" recursive="false" label="folder" description=""/>

Steps to reproduce the issue

Install the component com_pr17125
Go to Components > PR# 17125 > TEST

Expected result

The field should show the contents of the folder components/com_pr17125
image

Actual result

The field shows the contents of the folder administrator/components/com_pr17125
image

Testing Instructions

Install the patch PR# 17125
Go to Components > PR# 17125 > TEST

avatar joomla-cms-bot joomla-cms-bot - edited - 5 Nov 2017
avatar franz-wohlkoenig franz-wohlkoenig - change - 5 Nov 2017
Title
fix #16708
[fix] Custom field "list of images" doesn't show directories properly #16708
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 5 Nov 2017

@eshiol can you please update Test Instructions in first Comment?

avatar Quy
Quy - comment - 23 Apr 2018

@laoneo Would this be a valid fix to always look in the root directory for images? Currently, it is relative so if there is an images in administrator, then it will look there and not in the root.

avatar laoneo
laoneo - comment - 23 Apr 2018

Not in the folderlist field, otherwise we would have here a BC break. If you need that case, then I would do your own form field for the imegelist custom field.

avatar Quy
Quy - comment - 24 Apr 2018

@laoneo Please confirm to close this PR for a different solution.

avatar laoneo
laoneo - comment - 25 Apr 2018

@Quy if the author is willing to implement a form field for the image list custom field in this pr then we can work it out here. Otherwise it should be closed.

avatar laoneo
laoneo - comment - 25 Apr 2018

Why we have a BC break can be reproduced with the following steps:

  • Add the following code to the file administrator/components/com_content/config.xml after line 7
    <field name="file_path" type="folderlist" directory="help" hide_none="true" hide_default="true" recursive="false" label="Help" description=""/>
  • Open the Article Manager options

Actual result

The en-GB folder is shown:
image

Result with patch

Warning and the select box is empty
image

avatar eshiol
eshiol - comment - 26 Apr 2018

I'm not be able to change the code. I lost the branch.
This code should solve the BC break

		if (!is_dir($path) || is_dir(JPATH_ROOT . '/' . $path))
		{
			$path = JPATH_ROOT . '/' . $path;
		}

By removing is_dir($path) the behavior will change slightly but the function still works

		if (is_dir(JPATH_ROOT . '/' . $path))
		{
			$path = JPATH_ROOT . '/' . $path;
		}

If the path doesn't exist, in the first case the error is related to JPATH_ROOT . '/' . $path, as it is now;
image
in the second one, the error is related to $path
image

avatar Quy
Quy - comment - 30 Apr 2018

@eshiol I am sorry if this is not the right way. I would close this PR and submit a new PR with the latest changes.

avatar eshiol
eshiol - comment - 4 May 2018

replaced by #20294

avatar eshiol eshiol - change - 4 May 2018
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2018-05-04 07:35:20
Closed_By eshiol
avatar eshiol eshiol - close - 4 May 2018

Add a Comment

Login with GitHub to post a comment