Skip to content

Custom field "list of images" doesn't show directories properly#20294

Merged
rdeutz merged 4 commits intojoomla:stagingfrom
eshiol:patch-7
Feb 3, 2020
Merged

Custom field "list of images" doesn't show directories properly#20294
rdeutz merged 4 commits intojoomla:stagingfrom
eshiol:patch-7

Conversation

@eshiol
Copy link
Copy Markdown
Contributor

@eshiol eshiol commented May 4, 2018

Pull Request for Issue #16708. replace PR #17125

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.

@Septdir
Copy link
Copy Markdown
Contributor

Septdir commented May 4, 2018

@eshiol Maybe add trim?
Perhaps someone will write the path /images/path

$path = $this->directory;

if (!is_dir($path))
if (!is_dir($path) || is_dir(JPATH_ROOT . '/' . $path))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should return when if (!is_dir($path) && !is_dir(JPATH_ROOT . '/' . $path)) and not let it through as JFolder::folders will then return false anyway.

@brianteeman
Copy link
Copy Markdown
Contributor

@eshiol please can you update with the suggestion from @laoneo

Return if (!is_dir($path) && !is_dir(JPATH_ROOT . '/' . $path))
@ghost ghost added the J3 Issue label Apr 5, 2019
@ghost ghost removed the J3 Issue label Apr 19, 2019
@ghost ghost changed the title [fix] Custom field "list of images" doesn't show directories properly #16708 Custom field "list of images" doesn't show directories properly #16708 Apr 19, 2019
@ghost ghost changed the title Custom field "list of images" doesn't show directories properly #16708 Custom field "list of images" doesn't show directories properly Apr 19, 2019
@viocassel
Copy link
Copy Markdown
Contributor

I have tested this item ✅ successfully on 15bc344


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20294.

@anibalsanchez
Copy link
Copy Markdown
Contributor

The patch can't be applied.

The file marked for modification does not exist: libraries/joomla/form/fields/folderlist.php


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20294.

@brianteeman
Copy link
Copy Markdown
Contributor

@anibalsanchez again this is a Pull Request for staging not joomla 4

@jwaisner
Copy link
Copy Markdown
Member

I have tested this item ✅ successfully on 46b13ac


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20294.

@jwaisner
Copy link
Copy Markdown
Member

@viocassel Please test latest version of PR.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20294.

@viocassel
Copy link
Copy Markdown
Contributor

I have tested this item ✅ successfully on 46b13ac


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20294.

@viocassel
Copy link
Copy Markdown
Contributor

@jwaisner Done 🤘

$options = array();

$path = $this->directory;
$path = ltrim($this->directory, '/');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really needed?

@alikon
Copy link
Copy Markdown
Contributor

alikon commented Jan 28, 2020

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20294.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jan 28, 2020
@rdeutz rdeutz merged commit 10db709 into joomla:staging Feb 3, 2020
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Feb 3, 2020
@rdeutz rdeutz added this to the Joomla! 3.9.16 milestone Feb 3, 2020
@luistar15
Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.