? Failure

User tests: Successful: Unsuccessful:

Use DIRECTORY_SEPARATOR constant instead of '/' when looking for a favicon.ico (codelines 582 ff), because this causes WINDOWS installations to be unable to find the favicon.ico

see tracker item #4222
#4222

avatar schultz-it-solutions schultz-it-solutions - open - 5 Sep 2014
avatar jissues-bot jissues-bot - change - 5 Sep 2014
Labels Added: ?
avatar Bakual
Bakual - comment - 5 Sep 2014

As far as I know, PHP is able to deal with paths also on Windows when using /.
Your proposed code would actually create wrong URLs in the end because it uses the backslash there as well when adding the favicon using $this->addFavicon(JUri::base(true) . DIRECTORY_SEPARATOR . $path . 'favicon.ico');. That doesn't look correct.

avatar schultz-it-solutions
schultz-it-solutions - comment - 5 Sep 2014

Well, at least on my (WAMP) system, PHP in fact fails to handle the original code correctly, as I described in the original post on the "old" JoomlaCode Tracker ( http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=34012&start=0 ) :

The codeline

$path = str_replace(JPATH_BASE . '/', '', $dir);

actually removes JPATH_BASE plus / from the $dir variable, if the former exists in the latter.

However, on windows systems (at least on mine), the $dir variable contains: C:\Webserver\httpdocs\xxx\administrator\ (please note the last backslash, that will be compared to a slash in the str_replace) - so this str_replace NEVER succeeds (this is why the C:/Webserver survives the str_replace).

Using DIRECTORY_SEPARATOR here solves the issue.

However, you might be right that the "addFavIcon" could fail, so maybe we should apply this fix ONLY on the incriminating codeline.

avatar Fedik
Fedik - comment - 5 Sep 2014

I guess, there need DIRECTORY_SEPARATOR only for check if (file_exists($icon)) ....
Part with $this->addFavicon should stay as is, because the browser work with / and not with \

avatar Bakual
Bakual - comment - 5 Sep 2014

I tried to reproduce it using XAMPP (as I have this installed) on Windows 8.1 and couldn't reproduce it.
This is what I get when I dump the variables:
$dirs: D:\xampp\htdocs\joomla\administrator/ and D:\xampp\htdocs\joomla\administrator/templates/isis/
JPATH_BASE: D:\xampp\htdocs\joomla\administrator
Favicon: /joomla/administrator/templates/isis/favicon.ico

After applying this PR I get /joomla/administrator\D:\xampp\htdocs\joomla\administrator/templates/isis\favicon.ico for the favicon which of course then breaks it.

So it seems to be not a Windows issue but more a WAMP issue?

avatar smanzi
smanzi - comment - 5 Sep 2014

I have no favicon issues using UniformServer 11_3_2_ZeroXI (Apache/2.4.10 with PHP 5.3, 5.4 and 5.5) under Windows 8.1
Applying this patch, instead, does break my favicons.


This comment was created with the J!Tracker Application at http://issues.joomla.org/.

avatar Bakual
Bakual - comment - 6 Sep 2014

Tested it as well with wampserver 2.5 and favicon is working fine here. So to me it looks more like an issue with you installation.
Did you by change edit the file already?
Because I wonder how that backslash ends up in your $dirs array.
Code which creates that array is:

$path = $directory . '/';
$dirs = array($path, JPATH_BASE . '/');

As you see the last slash is hardcoded in both places. You never should get a backslash there to begin with. Thus the real question is why you got a backslash at this specific place.

I'm closing this PR as so far nobody was able to confirm the issue and the PR actually breaks the favicon.

avatar Bakual Bakual - change - 6 Sep 2014
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2014-09-06 10:43:11
avatar Bakual Bakual - close - 6 Sep 2014

Add a Comment

Login with GitHub to post a comment