... of course this should not be viewed as a real PR, but as a base for testing and see what we can do about this matter.

Travis failed for minor things, but I'm not going to fix those (right now) to underline that this is first rough test absolutely not conceived for production

Now it's time to take some sleep, but then I'll start polishing things, especially by factoring into the methods modifications that are now done on the calling side.

I will also revert all 3rd party libraries and that "critical" restore.php

In the meanwhile if someone want to test... more then welcome!!
PRs against this branch are welcome too.

I agree with Michael here. Smaller PRs are A LOT better. I've been working hard on getting the unittests to work properly on windows, but due to issues with my hardware, I currently can't test that fully. (My laptop is old and busted and overheats when I run the whole test suite at once...)

@mbabker and @Hackwar, please read my comment #5569 (comment) where I explain why I'm trying to do this...

Guys, I have a Windows system (Windows 7 Ultimate Edition) that is sitting underutilized: I also have an entry in no-ip.com to make it globally reachable and I can set-up TeamViewer as a service on it.

Then I can contribute the system to "the cause" and give access to who may be interested...

I understand where you are coming from, but there is no default way of handling this. This starts with the issue, that there are more than one developer out there that don't know the difference between a filesystem path and a URL. I also don't need to change all our constants and path handling in the case that someone wants to do a path comparison. Developers will have to pay attention to this, quite simply.

I currently have enough on my plate to go through the year until September. I guess I wont work on something like this before then.

@PhilETaylor Hi Phil! I've read #4433 and honestly my feeling is that the comparison between JPATH_BASE and JPATH_ADMINISTRATOR (mind you: those are two constants...) was failing only because they were defined in inconsistent way.

I'd really appreciate if you could test this "proof of concept" PR with your "problematic" Windows installations and see how it behaves...

@smz "Windows 7 Ultimate Edition" is not an acceptable hosting solution on which the development of Joomla Core should even be considered!

"Windows 7 Ultimate Edition" is not an acceptable hosting solution on which the development of Joomla Core should even be considered!

I understand, but that's what I have and can contribute...

@Hackwar

I currently have enough on my plate to go through the year until September. I guess I wont work on something like this before then.

No, no! That's not what I was asking for! My offer for the "Windows test platform" was generic, for whomever may be interested testing whatever he/she needs for whatever reasons...

I still hold that we should NOT be trying to reinvent the wheel, we should be using built in PHP constants whereever possible, like DIRECTORY_SEPARATOR, - Its only when we move away from that and start playing with this that bugs in OUR code appear.

Just like @mbabker said:

DIRECTORY_SEPARATOR is for FILE SYSTEMS and not URLS
forward slash as a default is for URLS

The problem is that even now, and historically, and in MANY places in CORE and 3rd Party EXTENSIONS the two have been confused.

It is not a quick fix to identify and fix each and every extension and line in the core.

It is not a quick fix to separate paths from URLs now.

All JPATH_* constants in Joomla SHOULD USE DIRECTORY_SEPARATOR as they are FILE PATHS NOT URLS.

However, even Joomla itself doesnt use them correctly as proved in the original #4433 #4433 (comment)

ANY CHANGE to what is currently in the core will mean EVERY extension developer testing EVERY extension use of JPATH_* constants and EVERY filepath/url building/routing will need to be tested...

So I conclude, this is a MASSIVE job... and should not be committed to lightly...

@PhilETaylor I totally disagree on all your lines:

The problem has never been the confusion between URLs and paths: the problem is that we sometimes have to make comparisons between file paths and if you look at the code you will find that Joomla is full of constructs like:

$newpath = $thispath . "/somedir_or_some_file";

$thispath may have / or \ as the directory separator depending on the underlying OS and $newpath may become a mixed-style path which is then more difficult to treat when you do comparisons.

This is what triggered the favicon bug, and, as I said in #5559 (comment), either we go bug hunting for every string comparison between paths, or (easier IMHO), we standardize on an internal representation of paths, call it a day and live happy for the rest of Joomla life.

I should point out that I already tried standardizing on one path type in both 1.5 and 1.6 and both attempts failed because of a host of reasons. It sounds simple, but unfortunately it definitely is not. Standardizing will mean at least as much work as checking for all path comparisons. Besides that, you will force Joomla to do unnecessary work on each pageload for paths which are not compared.

We go bug hunting, that is the correct course of action. In truth, the mixed slash comparison should only be an issue if you're doing something like this:

$windowsPath = 'C:\xampp\www\joomla\administrator\index.php';
$joomlaBuiltPath = 'C:\xampp\www\joomla\administrator/index.php';

if ($windowsPath != $joomlaBuiltPath) {
    echo 'Does not match, problem!';
}

In theory, this should work:

$windowsPath = JPath::clean('C:\xampp\www\joomla\administrator\index.php');
$joomlaBuiltPath = JPath::clean('C:\xampp\www\joomla\administrator/index.php');

if ($windowsPath != $joomlaBuiltPath) {
    echo 'Does not match, problem!';
}

PHP functions such as file_exists() or copy() handle mixed separators fine. Direct comparisons are going to fail because \ != /. So, a vast majority of our existing code base functions just fine using the existing constructs. It is in places where direct string comparisons are being made where your efforts should be focused.

@smz If you disagree that we should be using built in PHP constants where ever possible then I'll end my conversation here...

You yourself are failing to see that Joomla itself does comparisons where it compares a FILE PATH with a URL PATH.

If FILE PATHS are constructed right, and FILE PATHS are compared with FILE PATHS then there is no problem

If URL Paths are constructed right, and URLS are compared with URLS then there is no problem

The root cause of this problem, my problem for IIS and your favicon problem is that we do not, initially, correctly define the paths correctly and then try to compare a URL path with a FILE PATH.

Im not going to argue about this. I'm happy to have my previous PR revoked, however they then introduces another bug.

Like I said, the correct way to address this is not to "call it a day" but to address the problem fully, which is a massive job,

As @mbabker states - the other solution is to find all comparisons and fix those with JPath::clean(), but again that is a bug fix and not a root correction :-) This would also have fixed the original issue my PR fixed, however using PHP Built in constants over hard coded slashes should also ways win :)

@mbabker Yes I agree, the only problem we have (or at least this is the only one I too foresee...) is when we do "textual paths comparisons".

But as (or at least this is what I've being told...) PLT decided to standardize on / as the directory separator, to me it makes sense to enforce this as much as we can and (@Hackwar) there would not be much more overhead (look at the differences between JPath:clean() and JPath:ux_clean()...)

@PhilETaylor PHP constants are here for our convenience but nobody enforces us to use them, as it is demonstrated by the fact that current code is full of the '/' constant which is just like that... a constant, albeit not a "named constant".

The difference between JPath::clean() and JPath::ux_clean() is that someone's too lazy to call the former with the second parameter to force / separators

It really isn't needed to introduce that method or force every path everywhere to always use / separators. JPath::clean() serves the purpose of cleaning the file path so it's appropriate for the environment in use. It can also force paths to use a specific separator (really, you could drop a . in there and it wouldn't care). Does every file handling function in Joomla need to force the path to a specific separator? No. Are there places that do? Yes. Fix the specific use cases. You're taking a .50 caliber machine gun to a 9 millimeter handgun range here, it's overkill.

I'm no good at this talking crap.

I prefer to code.

#5580

Please find here a PR #5580 which reverts #4433 and implements a cleaning of the only place I can find where IIS was failing for me on a crappy IIS server configuration that initially had #4433 to fix.

Please test this PR #5580 and see if it fixes all your issues in life and makes everyone in the world happy...

The difference between JPath::clean() and JPath::ux_clean() is that someone's too lazy to call the former with the second parameter to force / separators

Yes, I agree!!
... but it was convenient for testing my "proof of concept"

You're taking a .50 caliber machine gun to a 9 millimeter handgun range here, it's overkill.

I like big bangs! Gimmie that LW50MG!!!

@PhilETaylor "talking crap"? Sorry but you're being un-polite here: is there any specific reason?

Please note that under Windows #5580 will generate mixed-style named constants itself. I will test and probably it will fix the issue at hand as we didn't had the issue before your #4433, but please remind that there is also my #5559 which fixes it in the way which probably @mbabker will most agree with.

@smz The problem before standardising on "/" was, that we had stuff like JPATH_BASE . DS . "something" . DS . "somethingelse" . DS . "evenmore" etc. and it was absolutely unreadable and also didn't make any difference. As @mbabker said, there is no difference for PHP if you use / or \ and thus it was decided in order for readability to get rid of the DS crap that we had. That is the standardising that the PLT is talking about. Its 9 less characters per separator and if you have a path with 10 separators, that means 90 less characters.
Just for the fun of it: Writing JPATH_BASE . '/libraries/cms/component/router/rules/menu.php' with the older DS separator style means that we go from 61 chars to 110. That means almost double the number of characters without any gain and in fact a detrimental effect (reduced readability).

Regarding performance: Yes, the 11 additional string concatenations from my example above are not a deal breaker here, but Joomla does have a performance issue from my perspective and we should strive to solve that instead of introducing even more performance drains, as small as they are. With that I mean that every loss in performance should be balanced by some gain for the users. I don't see that gain here.

To be clear - the "crap" was referring to my "talking" not to your words. Those that know me know I don't like to talk for days and days over a single line of code... I'm a do-er.

#5580 has been tested on the exact IIS server (thanks @RedEvo) that had issues and it now generates two strings that compare correctly so is a quick fix to revert the #4433 that has caused you at least two issues, and fixes the original issue the #4433 was designed to fix. Thus is a win win situation.

No shooting of developers is allowed.

@Hackwar Wait! When I was talking about introducing the JDS named constant (fixed at /), I absolutely didn't meant to come back to the horrors of DS and the concatenation of the various elements of a path. It was meant as a shortcut for situations like str_replace('\\', JDS); or other usages of the same kind...

To be clear - the "crap" was referring to my "talking" not to your words.

Ah, OK, sorry: I did took it the other way... (Ah, the difficulties of human interaction through a cold and inexpressive medium!)

@smz Have you tested #5580 ? Does it fix all the issues you had? If so I suggest thats merged and we park for now the larger massive job of revisiting all the other places of code that we could/should code correctly...

Have you tested #5580 ?

Not yet: gimmie some minutes...

Just a question: why don't you explode/implode with / instead of DIRECTORY_SEPARATOR for building JPATH_ROOT which then will be concatenated to "slash-qualified" paths?

Because I'm not touching what is/was not broken. I have simply reverted back to known good code. Code that used to work for you...

And furthermore the problem slash is NOT in JPATH_ROOT, the problem slash that #4433 fails on is the one hard coded before a folder name, like /administrator

@PhilETaylor
Sorry, dinner time here and my com_patchtester is acting up... will test later...

@PhilETaylor OK, tested #5580 with success. Please see comments there...

Now I would say this PR is ready for "prime time"...

I don't pretend it to be 100% bug free: no honest and decent programmer can sincerely affirm that of any piece of code (unless subjected to formal analysis).

There are 101 modified files in this PR, but much of that derives from the need of not changing the default directory separator in JPath::clean(). IMHO this would not be a problem: in all Unix and MacOS installations this would account to no difference, while in Windows installations it would have the desired effect of adjusting the directory separator to the proposed Unix style.

For not breaking B/C (a non issue in this case, IMHO) I introduced the JPath::ux_clean() method. As @mbabker correctly says this is a "lazy way of not adding the second parameter to JPath::clean()": I agree.

As for the rest it has just been a matter of:

• replacing all (actually most) DIRECTORY_SEPARTOR named constants to the unnamed '/' constant
• adding some (not many) JPath::ux_path() calls (JPath::ux_path() being a new method that does just a str_replace('\\', '/', $path))
• fixing very few cases where the code was unneeded or in error.

Personally I would:

• Accept the risk (0% in my opinion) of breaking some code in some very rare circumstances and change the behavior of JPath::clean(), and then revert all the modifications made changing all JPath::ux_clean() back to JPath::clean()
• Add the JDS named constant initialized to '/', just for the convenience of using it when needed (again and to make it clear: not to use it the way we used old DS to build paths piece by piece!)

My code works: I can't and don't want to force it down anybody's throat, but I suggest the community to take it into consideration.

Sorry, above I mentioned JPath:ux_path(), but forgot that it was existing only in my mind: in reality where needed I just use str_replace('\\', '/', $path)

... and I forgot to send a sincere and big Thank you! (bold intended) to @mbabker who provided constructive criticism and guidance in the development of a PR he doesn't agree with!

For reference, I've created a new PR #5586 which is a sister to this one: in it I took the liberty of modifying JPath::clean() and hence do not have the need for JPath:ux_clean() anymore

P.S.: to make it clear, #5586 does not substitute this PR. It is an alternative in case the modification to JPath::clean() is accepted, and also a lightweight version of this one for those who wants to check what really has been done, without the clutter of all the modifications from JPath::clean() to JPath::ux_clean()

PR dead (killed, actually...)

Travis failed but if I'm not getting wrong it is a problem in Travis itself:

Parse error: syntax error, unexpected ')' in /home/travis/build/joomla/joomla-cms/libraries/loader.php on line 163

Forget it... it was my error!

Didn't read all comments here but read the related Google Mail discussion.
I'm closing this PR since the risk of a B/C break is to big and this will never get tested well enough.

You're welcome to create a PR to fix the actual issue at hand.

On a sidenote: Please don't use GitHub as a discussion place. Keep your comments to a minimum. Others are going to read them and it's wasting time of the other contributors if there is no real information or update in a comment.

@Bakual

You're welcome to create a PR to fix the actual issue at hand.

#5566