Joomla! Issue Tracker | Joomla! CMS #11288 - folderlist form field returning full paths on Windows (issue #11102)

Closed
23 Feb 2017
Medium
Build: staging
# 11288
Diff
cheesegrits:issue_11102

Failure

Failure continuous-integration/travis-ci/pr The Travis CI build failed Details
Success JTracker/HumanTestResults Human Test Results: 7 Successful 0 Failed. Details

User tests: Successful: Unsuccessful:

cheesegrits
24 Jul 2016

Pull Request for Issue #11102 .

Issue is that folderlist on Windows now returns full paths instead of just folder names.

Summary of Changes

Fix for JFolder::_items() to use DIRECTORY_SEPARATOR instead of / when building full paths, and to rtrim the separator before concatenating file to path, to avoid double separators.

Fix for JFormFieldFolderList, to clean the path, which will then correctly match the path coming back from JFolder::folders(), and trim DIRECTORY_SEPARATOR instead of /.

Testing Instructions

Add a folderlist field to a form in the backend. I used com_categories. Edit ./administrator/components/com_categories/models/forms/category.xml. Around line 246, in the 'basic' fieldset, add ...

            <field
                directory="/administrator/components/com_categories/"
                label="Folder Test"
                name="folderlist_test"
                description="A test for the folderlist form field"
                type="folderlist"
                recursive="true"
            />

Open the J! backend, and edit any existing Category (under the Content menu). Go to the Options tab. You will see a new dropdown, "Folder Test". On a UN*X machine, it'll be just the folder names. On a Windows machine, it will be full absolute paths.

Apply the PR.

Reload the page. The folder dropdown will now just be folder name.

Votes

# of Users Experiencing Issue

3/3

Average Importance Score

5.00

9586e6a 24 Jul 2016

Fix for folderlist issue #11102

793db3a 24 Jul 2016

Ooops, remove test changes to XML

cheesegrits - open - 24 Jul 2016

cheesegrits - change - 24 Jul 2016

Status

New

⇒

Pending

joomla-cms-bot - change - 24 Jul 2016

Category

Libraries

⇒

Fields Libraries

wronax - comment - 29 Jul 2016

Is this PR need more checks to go into 3.6.1?

brianteeman - comment - 29 Jul 2016

yes

wronax - comment - 29 Jul 2016

What I need to test it? I'm new on GitHub. Could you please send me any tutorial?

brianteeman - comment - 29 Jul 2016

See https://docs.joomla.org/Testing_Joomla!_patches

On 29 July 2016 at 12:06, wronax notifications@github.com wrote:

What I need to test it? I'm new on GitHub. Could you please send me any
tutorial?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#11288 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8ZqcdV_7vZCx2gqXcrzfMG9h-INVks5qad6_gaJpZM4JTrkH
.

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

hardiktailored - test_item - 29 Jul 2016 - Tested successfully

hardiktailored - comment - 29 Jul 2016

I have tested this item ✅ successfully on 793db3a

Tested on Windows-7 and Ubuntu 12.04 machine. Before it was showing full path 'var/www/..../', after applying patch it resolved issue on both the machine. Now showing folder name only.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11288.}

a622f45 29 Jul 2016

Use check() instead of clean() in folderlist

kevinscheithauer - test_item - 1 Aug 2016 - Tested successfully

kevinscheithauer - comment - 1 Aug 2016

I have tested this item ✅ successfully on a622f45

I have tested this issue @icampus PBF with the proposed solution and can confirm that the fix worked. Tested on Windows 7 with Firefox. I edited the com_categories component with the folderlist field in /models/forms/category.xml. It showed the full path before the fix and applying the patch resolved the issue for me (showed folder names only).

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11288.}

hardiktailored - test_item - 1 Aug 2016 - Tested successfully

hardiktailored - comment - 1 Aug 2016

I have tested this item ✅ successfully on a622f45

Tested on Windows 7 and Ubuntu 12.04 (Chrome, Firefox).

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11288.}

hardiktailored - comment - 1 Aug 2016

Check results below:

Before patch:

After patch:

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11288.}

brianteeman - comment - 1 Aug 2016

Thanks for testing - seems that it works but it is failing the unit tests
@wilsonge is this a false positive? Do the unit tests need updating? Or is there a bugger problem with this PR

mehmetalipamukci - test_item - 1 Aug 2016 - Tested successfully

mehmetalipamukci - comment - 1 Aug 2016

I have tested this item ✅ successfully on a622f45

tested @icampus:
successfully tested following the given instructions for categories component.
i tested it also for another component (in my case newsfeeds component) and it works.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11288.}

wilsonge - comment - 2 Aug 2016

This isn't a false positive on the test - it seems like a genuine failure. But it's as a result of adding https://github.com/joomla/joomla-cms/pull/11288/files#diff-ec4b92ba2051d70bff7613c1a6199c5bR192 but that's just a sanity check - I'm not sure if it's a dodgy test or the sanity check is going to kill things on some (linux?) filesystems

pepperstreet - comment - 3 Aug 2016

Thought this would make it into Joomla 3.6.1 ?!

BTW, I have tested successfully with a 3rd-party extension Cobalt CCK by MintJoomla. Which uses the field "folderlist" in global configuration. There is a parameter for Community-Integration, which lists the respective subfolder-name.
OSX El Capitan - MAMP 3.4

brianteeman - comment - 3 Aug 2016

@pepperstreet it was not included because it is failing the unit tests

pepperstreet - comment - 4 Aug 2016

@brianteeman Thanks for your comment, but what is a "unit test"? (seriously)

wronax - comment - 4 Aug 2016

I'm also curious why it failed unit test. Is it possible that unit test is written incorrectly?

This bug is critical for many extensions and template frameworks, because they use folderlist field as a theme option what causes that no theme is loaded after saving extension/template settings and as a result website is messed up with no possibility fix it from Joomla back-end.

brianteeman - comment - 4 Aug 2016

@wilsonge provided the link above

Bakual - test_item - 9 Sep 2016 - Tested successfully

Bakual - comment - 9 Sep 2016

I have tested this item ✅ successfully on a622f45

Tested and it is still an issue which has to be fixed.

Can we do something about the failed unit tests ourself?

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11288.}

Globulopolis - test_item - 27 Sep 2016 - Tested successfully

Globulopolis - comment - 27 Sep 2016

I have tested this item ✅ successfully on a622f45

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11288.}

pepperstreet - comment - 19 Oct 2016

Still not in J! 3.6.3 ? Any news about it? How to get this merged? ;)

mbabker - comment - 19 Oct 2016

Fixing the unit test failure for starters. Either determining if the test failure is an indicator of an actual code issue or if it's a bad test. If it's a bad test, it needs to be rewritten to ensure the behavior is valid going forward.

There's more to getting code merged than just opening a pull request and asking for it to be merged ?

mo-matictec - test_item - 10 Nov 2016 - Tested successfully

mo-matictec - comment - 10 Nov 2016

I have tested this item ✅ successfully on a622f45

We have tested it successfully on our Windows Server 2012 R2 servers and PHP 7.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11288.}

ggppdk - test_item - 11 Nov 2016 - Tested successfully

ggppdk - comment - 11 Nov 2016

I have tested this item ✅ successfully on a622f45

This works, but needs to be rebased it has conflicts,
maybe after resolving conflicts, and after replacing check() with clean(),
test units will succeed ?

e.g. about conflicts it extends different class:

-class JFormFieldFolderList extends JFormAbstractlist
+class JFormFieldFolderList extends JFormFieldList

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11288.}

jeckodevelopment - comment - 17 Nov 2016

This PR has 7 successful tests, but it needs to make Travis "green".
So @cheesegrits can you please look at the conflicts?

mbabker - comment - 17 Nov 2016

It doesn't matter how many users have tested it, the unit tests still need to be fixed. If the unit tests have a bad behavior, the tests need to be rewritten. If the tests are working correctly, then this indicates there is an incorrect functional change and the patch needs to be adjusted.

CoalaWeb - comment - 27 Jan 2017

Any progress on updating this so it passes the unit tests?

wilsonge - comment - 23 Feb 2017

Closing this in favour of #14139 which should fix this issue and doesn't have unit test issues

wilsonge - change - 23 Feb 2017

Status	Pending	⇒	Closed
Closed_Date	0000-00-00 00:00:00	⇒	2017-02-23 01:30:36
Closed_By		⇒	wilsonge

wilsonge - close - 23 Feb 2017

Add a Comment

Older
Newer

Joomla! Issue Tracker - CMS

[#11288] - folderlist form field returning full paths on Windows (issue #11102)

Summary of Changes

Testing Instructions

Votes

Add a Comment