Am I the only one having issues with favicon (both frontend and backend) with the latest staging code? I don't see it anymore or I see it randomly. I also don't see a request (GET) for it while monitoring network activity...
Have we changed something at this regard?
Have you tried to enter the path to the favicon directly and refresh it?
Just tested in a completely fresh install of FF, still can't reproduce the issue :/
curious... I have several local virtual hosts and the only one where I see this issue is the one using 3.4.0-dev...
I'll do further testing, but in the meanwhile let's leave this issue open (maybe at low priority if someone want to change it: I can't) and see if someone else pops out with the same issue...
... I also seems to remember that something has been done to favicon with this rev, but I can't remember exactly what... (my poor neurons... )
hehe :) Yeah, just leave it open
@smz Did you try with different browsers? Do you have any mod_rewrite directives in .htaccess regarding templates folders? Does clear cache of browser not help?
Hi Richard!
(logging off for some hours...)
From memory we don't overwrite the favicon anymore with an update. But I'd have to check to be sure.
By default, the browser will look in the root folder of the website for a favicon.
However Joomla automatically creates the correct links by checking the template and root folder for the file and creating the proper link in the document header. See https://github.com/joomla/joomla-cms/blob/f6c93e0294ba6e75b9604366f71bcf6b66970c04/libraries/joomla/document/html/html.php#L583
I think we have a real issue.
First of all I looked in the very obvious place (where I didn't before, shame on me!), the actual generated HTML, where the problem is evident:
<link href="/D:/UniServerZ/vhosts/smztest/templates/protostar/favicon.ico" rel="shortcut icon" type="image/vnd.microsoft.icon" />
Then, thanks @Bakual that pointed me to the right code: please use it for reference while reading here below.
I dumped some variables and found that:
$directory = D:/UniServerZ/vhosts/smztest\templates
JPATH_BASE = D:/UniServerZ/vhosts/smztest
and hence the code to remove JPATH_BASE from $directory:
$path = str_replace(JPATH_BASE . '/', '', $dir); <--- $dir == $directory
cannot work because D:/UniServerZ/vhosts/smztest/
!= D:/UniServerZ/vhosts/smztest\
Tracing back where $directory is populated I landed to _fetchTemplate()
where we have:
$directory = isset($params['directory']) ? $params['directory'] : 'templates';
Here... I'm a bit lost because I cannot understand where that 'directory' parameter comes from.
In any case it is the cause as it is set to D:/UniServerZ/vhosts/smztest\templates
.
Substituting the above line of code in _fetchTemplate()
with the following one solved the issue:
$directory = isset($params['directory']) ? str_replace('\\', '/', $params['directory']) : 'templates';
PR is on its way...
This really sounds like another issue with your uniserver setup that no one else can replicate.
Until anyone else can replicate ir then it definitely is not a serious issue.
PR is #5566
@brianteeman As you are raising a specific hypothesis, the onus of the proof is on you.
Well as no one else has yet confirmed the issue then sorry but no its not.
I have tested on all platforms available to me and cannot replicate
On 30 Dec 2014 23:51, "Sergio Manzi" notifications@github.com wrote:
@brianteeman https://github.com/brianteeman As you are raising a
specific hypothesis, the onus of the proof is on you.—
Reply to this email directly or view it on GitHub
#5559 (comment).
I traced back code and data, and demonstrated that a wrong parameter is stored in Joomla.
The conditions bringing to this can be many: OS version, PHP version, MySQL version, libraries version, who knows, maybe even the CPU...
Your hypothesis of Uniform Server being the cause is just that, an hypothesis, until proofed. If you insist on it without proofing it will become FUD.
On my side I also have the fact that at least other 5 virtual servers in the same Uniform Server installation do not present the issue.
This is one of the reasons I objected to the change of using DIRECTORY_SEPARATOR. Code in Joomla expects '/' as directory separator. The platform difference should not be introduced to Joomla. That would require to deal with it at all places instead on only on input!
@sovainfo It must be that somewhere, someone, builds that parameter by a concatenation of this kind:
$directory = JPATH_BASE . DIRECTORY_SEPARATOR . 'templates';
but then we try to strip out JPATH_BASE . '/'
and we of course fail.
I'm still trying to find where the error is in the first place ("the original sin"), but sanitizing $params['directory']
on load can't do any harm, for sure!!
The standard includes/defines.php is doing that. look at the pr's that changed it. The correct code used to be '/templates', '/cache' ....
See #4433
On 31 December 2014 at 00:47, sovainfo notifications@github.com wrote:
The standard includes/defines.php is doing that. look at the pr's that
changed it. The correct code used to be '/templates', '/cache' ....—
Reply to this email directly or view it on GitHub
#5559 (comment).
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
So... was this an Uniform Server issue?
Now I'm calling in PLT to decide which is the correct fix: either revert #4433 (IMHO the right thing to do...) OR go bug hunting for every instance where we concatenate '/' to a path and change it to DIRECTORY_SEPARATOR.
In any case my PR is probably useless as it is some other str_replace(
\, '/', $whatever);
spread around for good measure...
The PLT, facing an obvious case where that PR caused a regression, can always change its mind...
AFAIK in PHP there is actually no need to use DIRECTORY_SEPARATOR as all file functions will account for specific local OS differences and apply the needed modifications automatically.
PLT, think of this: how many 3rd party extensions you risk to break with that PR? Talk about B/C...
Please also note that under Windows JPATH_BASE
defined as __DIR__
, all __DIR__
, and all __FILE__
instances are converted by PHP with paths with normal slashes /
and adding DIRECTORY_SEPARATOR to those will create "mixed style" paths which are IMO a really-bad-bad-thing.
If there are edge cases under IIS where this is causing issues, those edge cases should be solved specifically in the context of IIS
The server environment Phil was able to create the original issue on proves
that PHP does not always handle / separators well. In the case of that PR,
the proposed change fixed an edge case without breaking anything obvious at
the time. Circumstances have changed since then.
There are known path handling issues in the Joomla code base. Even the
unit test suite fails on Windows because of this. So our options at the
framework and API levels are to fix the issues or discontinue support for
Windows. And the latter is not an option.
On Tuesday, December 30, 2014, Sergio Manzi notifications@github.com
wrote:
PLT, think of this: how many 3rd party extensions you risk to break with
that PR? Talk about B/C...—
Reply to this email directly or view it on GitHub
#5559 (comment).
Also this is not entirely a IIS versus Apache issue on Windows. When I ran
a Windows system for testing, I would have test failures running our test
suite on XAMPP.
On Tuesday, December 30, 2014, Michael Babker mbabker@flbab.com wrote:
The server environment Phil was able to create the original issue on
proves that PHP does not always handle / separators well. In the case of
that PR, the proposed change fixed an edge case without breaking anything
obvious at the time. Circumstances have changed since then.There are known path handling issues in the Joomla code base. Even the
unit test suite fails on Windows because of this. So our options at the
framework and API levels are to fix the issues or discontinue support for
Windows. And the latter is not an option.On Tuesday, December 30, 2014, Sergio Manzi > wrote:
PLT, think of this: how many 3rd party extensions you risk to break with
that PR? Talk about B/C...—
Reply to this email directly or view it on GitHub
#5559 (comment).
There may be some other bugs hidden somewhere where using DIRECTORY_SEPARATOR does indeed solves the issue, but that doesn't automatically and "per se" means that DIRECTORY_SEPARATOR is the good choice...
Not saying it is either. But in general, I'm saying we need to address
areas in our code where there are issues on Windows when it comes to
filesystem operations.
On Tuesday, December 30, 2014, Sergio Manzi notifications@github.com
wrote:
There may be some other bugs hidden somewhere where using
DIRECTORY_SEPARATOR does indeed solves the issue, but that doesn't
automatically and "per se" means that DIRECTORY_SEPARATOR is the good
choice...—
Reply to this email directly or view it on GitHub
#5559 (comment).
I absolutely agree! But first let's decide which is the correct way to go, standardize on it, and proceed. I don't want to be too pushy and repeat myself too often, but again: look at what PHP itself does...
We've basically standardized on /
in our code. There are places where that has caused issues, so let's address those issues now.
Also, just to make clear since I still see people mentioning my name with the group, I'm not PLT anymore and try not to get involved in decision making matters very much now. I'm just another (loud) voice in the crowd
got it! ... but you surely can bring PLT attention on this...
Really, I think the odds of breaking 3rd party templates are quite high here...
Sorry but I made false assumptions here above: under Windows __DIR__
and __FILE__
returns path with the backslash separator (\
)
I've been tricked by the fact that JPATH_BASE
(which has forward slashes) is defined as define('JPATH_BASE', __DIR__);
in /administrator/index.php, but for frontend (/index.php) it is defined as define('JPATH_BASE', str_replace('\\', '/', __DIR__));
... OMG!
I'm about trying to make global code modifications getting rid of DIRECTORY_SEPARATOR, standardizing on /
for separator and check all contexts where __DIR__
and __FILE__
are used... I'll let you know if I can produce something working...
Status | New | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-01-02 20:17:35 |
Closed_By | ⇒ | brianteeman |
Closed_Date | 2015-01-02 20:17:35 | ⇒ | 2015-01-02 20:17:36 |
Labels |
Added:
?
|
Hmm can't seem to reproduce this