? ? Pending

User tests: Successful: Unsuccessful:

avatar joomdonation
joomdonation
19 Feb 2017

Pull Request for Issue #14075 .

Summary of Changes

The FolderList field type (https://docs.joomla.org/Folderlist_form_field_type) provides a drop down list of folders (relative path) from a specified directory. Due to a bug in the code, the dropdown list contains full path of the folders on Windows instead of just the relative path. It causes an issue with Imagelist field type as described in issue #14075

Testing Instructions

1.Create article
2.Create custom field type "List of images"
3.Select images Directory, for example banners
4.On frontend try to edit this article
5. On frontend you get no list of images and if you try to edit article you get warning message
6. Apply patch
7. Edit the custom field, select a folder in Directory parameter again (so that the parameter is saved with correct relative path)
8. Edit the article in frontend again, confirm that the issue fixed.

Documentation Changes Required

None

avatar joomdonation joomdonation - open - 19 Feb 2017
avatar joomdonation joomdonation - change - 19 Feb 2017
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 19 Feb 2017
Category Libraries
avatar joomdonation
joomdonation - comment - 19 Feb 2017

@Bakual Seems you are using Windows. Could you please help checking this one?

avatar ggppdk
ggppdk - comment - 19 Feb 2017

@joomdonation there are 3 other PRs open for this already, so this is the 4th PR made

See here:
https://github.com/joomla/joomla-cms/pulls?utf8=%E2%9C%93&q=is%3Apr%20is%3Aopen%20FolderList%20

I had tested all of them, and i believe i posted successful test in the most complete of them (that works in all cases):
#11288

This PR should have the same problem as described here:
#12841 (comment)

avatar joomdonation
joomdonation - comment - 19 Feb 2017

Thanks for the information @ggppdk, I don't know why these PRs were not merged

However, exactly what's wrong with this simple fix? I tested and it worked well. If the folder contain sub-folders, the result might be sampledata/fruitshop , sampledata/parks... which still work OK on Windows.

avatar ggppdk
ggppdk - comment - 19 Feb 2017

However, exactly what's wrong with this simple fix?

Without retesting, if i remember correctly

if you add a folder path that uses unix separator / instead of windows seperator \ then this PR will not work

That is your PR will work in a WINDOWS environment with destination:
directory="myfolderL1\myfolderL2\myfolderL3"

but will not work if fo
directory="myfolderL1/myfolderL2/myfolderL3"

And it is not too uncommon can a site is moved from windows environment to unix,
and it should be possible to do without needing to reconfigure (relative) folderlists

I think you could make a rewrite of PR #11288

avatar wilsonge
wilsonge - comment - 19 Feb 2017

There are three versions of this that I know of #11288 , #12841 and #11524

This looks like a duplicate of the latter two?

avatar joomdonation
joomdonation - comment - 20 Feb 2017

That is your PR will work in a WINDOWS environment with destination:
directory="myfolderL1\myfolderL2\myfolderL3"

but will not work if fo
directory="myfolderL1/myfolderL2/myfolderL3"

Actually, it does work. Here is the screenshot of sample folderlist form field with directory="components/com_content" recursive="true"

folder_list

avatar joomdonation joomdonation - change - 20 Feb 2017
Labels Added: ?
avatar joomdonation
joomdonation - comment - 20 Feb 2017

@wilsonge This is duplicate of #11524. However, that PR was from long time ago, so I am afraid of people won't test it soon. So we will leave this PR opens so that it get tested. Then you can merge this one or #11524 . We just need the issue fixed.

avatar wilsonge wilsonge - change - 23 Feb 2017
The description was changed
avatar wilsonge wilsonge - edited - 23 Feb 2017
avatar wilsonge wilsonge - change - 23 Feb 2017
The description was changed
avatar wilsonge wilsonge - edited - 23 Feb 2017
avatar wilsonge wilsonge - change - 23 Feb 2017
The description was changed
avatar wilsonge wilsonge - edited - 23 Feb 2017
avatar wilsonge
wilsonge - comment - 23 Feb 2017

I've closed the other PR's in favour of this one and merged in the test instructions from #14075 :)

avatar joomdonation
joomdonation - comment - 23 Feb 2017

Thanks @wilsonge. Hope we will get enough testers for this PR, seems it is hard to get Windows testers.

avatar N6REJ N6REJ - test_item - 23 Feb 2017 - Tested successfully
avatar N6REJ
N6REJ - comment - 23 Feb 2017

I have tested this item successfully on c334f9c

without patch path is a mess, with patch directory listing is cleaned up nicely.


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

avatar photodude
photodude - comment - 23 Feb 2017

@N6REJ repeatable form fields are broken (and deprecated), you should use the Subform form field type as of J3.6 for repeatable field functions.

avatar N6REJ
N6REJ - comment - 23 Feb 2017

@photodude that doesn't make any sense.. subform is for stacking of fields & repeatable is not marked as deprecated in https://docs.joomla.org/Standard_form_field_types

avatar photodude
photodude - comment - 23 Feb 2017

@N6REJ it is deprecated look at the api or the code. the docs are a user maintained guide that is often a little behind https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/form/fields/repeatable.php#L18

avatar wilsonge
wilsonge - comment - 23 Feb 2017

Also it is deprecated in the docs - look at the repeatable form field page (and no it's not a change made just now but in July 2016) https://docs.joomla.org/index.php?title=Repeatable_form_field_type&diff=318910&oldid=317302 . Having said that even though deprecated this is a completely separate issue so please open a new issue for this and leave this one for the windows folder system issue :)

avatar alikon alikon - test_item - 25 Feb 2017 - Tested successfully
avatar alikon
alikon - comment - 25 Feb 2017

I have tested this item successfully on c334f9c


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

avatar zero-24 zero-24 - change - 25 Feb 2017
Milestone Added:
Labels Added: ?
avatar wilsonge wilsonge - change - 25 Feb 2017
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2017-02-25 12:19:27
Closed_By wilsonge
Labels
avatar wilsonge wilsonge - close - 25 Feb 2017
avatar wilsonge wilsonge - merge - 25 Feb 2017
avatar joomdonation
joomdonation - comment - 25 Feb 2017

Thanks all for your help :)

avatar peterhulst
peterhulst - comment - 22 Mar 2017

This modification was still not present in 3.7.0-beta3? Could it be merged?? Thaks!

avatar mbabker
mbabker - comment - 22 Mar 2017

This was merged after beta 3 was tagged so it wouldn't be included there. It'll be in the next tagged release.

Add a Comment

Login with GitHub to post a comment