? Success

User tests: Successful: Unsuccessful:

avatar dgt41
dgt41
21 Jan 2015

This PR ensures that all js and css added by Joomla templates follow the same rule

All templates that come with standard Joomla installation add some scripts and some stylesheets files. The problem is that not all of them use the same logic and this ends up with an array like this:
screen shot 2015-01-21 at 5 45 15

From processing point of view that is not a problem as the function in head.php will iron this! But why not echo some variable that is already in the memory and get everything in the same line?
So this is hathor before:

$doc->addStyleSheet('templates/system/css/system.css');

And after:

$doc->addStyleSheet($this->baseurl . '/templates/system/css/system.css');
B/C

None

Testing:

Just test that Beez, protostar, Hathor and Isis still working (css and js files get loaded)

avatar dgt41 dgt41 - open - 21 Jan 2015
avatar jissues-bot jissues-bot - change - 21 Jan 2015
Labels Added: ?
avatar dgt41
dgt41 - comment - 21 Jan 2015

Forgot to post the after image of the same array, so here it is:
screen shot 2015-01-21 at 8 29 43

So to make it clear: templates right now will have a js path like templates/isis/js/template.js
where it should be /administrator/templates/isis/js/template.js

avatar zero-24 zero-24 - change - 21 Jan 2015
Category Templates (admin) Templates (site)
avatar zero-24 zero-24 - change - 21 Jan 2015
Easy No Yes
avatar AlexdeBorba
AlexdeBorba - comment - 26 Jan 2015

Somehow this pulls sidebar and notification alerts on Isis too close to the Administrator navigation by removing the space in-between.

avatar brianteeman
brianteeman - comment - 26 Jan 2015

Thanks for testing. Can you upload a screenshot please

avatar AlexdeBorba
AlexdeBorba - comment - 26 Jan 2015

I am sorry for not adding one previously, totally skipped the step.
mozilla-2015-01-25 23-11-36

avatar dgt41
dgt41 - comment - 26 Jan 2015

@AlexdeBorba One quick question: if you scroll up and down the page restores that space?

avatar AlexdeBorba
AlexdeBorba - comment - 26 Jan 2015

No it doesn't, and I am using latest Joomla! 3.x version as well. Both on Chrome and Mozilla returns the same thing.

avatar dgt41
dgt41 - comment - 26 Jan 2015

@AlexdeBorba This only happens if you apply this patch? What if you clear the browser cache before and after applying this PR? It is very awkward, because all this patch is doing is setting the relative path e.g. from templates/isis/css/template.css to /administrator/templates/isis/css/template.css, so in theory this erratic behavior should never, ever happen!

avatar AlexdeBorba
AlexdeBorba - comment - 26 Jan 2015

Yes, it only happens when I apply this patch. I clean both browser as Joomla! cache prior to view any changes I made, and once I do this I can confirm the issue comes from this patch. After reversing everything goes back to normal.

Perhaps it could be an issue related to compressed caching, since I run optimization/compression on the website. Just a thought.

avatar dgt41
dgt41 - comment - 26 Jan 2015

@AlexdeBorba honestly if this PR was in any case wrong, the result should be no css and no js should load. So most probably the erratic behavior might come from a 3pd software, you mentioned that you are using something for compression, whitch one?

avatar AlexdeBorba
AlexdeBorba - comment - 26 Jan 2015

I am using as plugin, JCH Optimize with server-side compression by MaxCDN. Perhaps it occurs just at my end due to some CDN issue, as I had before issues with JS loads which are now resolved.

avatar dgt41
dgt41 - comment - 26 Jan 2015

@AlexdeBorba I will appreciate it if you can test it in a out of the box environment, without CDN, etc. Most of the times, CDN’s need to propagate the changes and this doesn’t happen immediately...

avatar AlexdeBorba
AlexdeBorba - comment - 26 Jan 2015

I have run it without CDN and compression and the result was the same, with or without it, the header pulls the main area toward the top.

avatar dgt41
dgt41 - comment - 26 Jan 2015

@AlexdeBorba This is quite weird. My setup seems fine here:
before
screen shot 2015-01-26 at 9 35 51

And after
screen shot 2015-01-26 at 9 34 07

As you can see, the after (thats with this PR applied) got the correct relative paths. Also if something was wrong with this PR most probably the relative path would be wrong and thus the page would be without that template.css and the template.js files…
Are you on the latest staging?

avatar AlexdeBorba
AlexdeBorba - comment - 26 Jan 2015

I will give it a second try later this week. Actually I am using the Joomla! Patch Tester Component to apply patches. But thanks for looking into it and don't stress, perhaps its some conflict I have over my setup.

avatar dgt41
dgt41 - comment - 26 Jan 2015

@AlexdeBorba Actually comparing your screenshot (take a look at the sidebars) with mine I can say that you use 3.3.6 (?) and this PR needs at least 3.4_dev. Also because you are using patch tester, the index.php files are replaced and therefore the whole mess.

avatar AlexdeBorba
AlexdeBorba - comment - 27 Jan 2015

Yes, you are right, thank you for the insight.

avatar Fedik Fedik - test_item - 27 Jan 2015 - Tested successfully
avatar Fedik
Fedik - comment - 27 Jan 2015

test
works good for me

avatar waader
waader - comment - 27 Jan 2015

@test success!

One question: should this also be done for the IE .css files eg. in hathor?
<link href="templates/<?php echo $this->template; ?>/css/ie8.css" rel="stylesheet" type="text/css" />

avatar waader waader - test_item - 27 Jan 2015 - Tested successfully
avatar dgt41
dgt41 - comment - 27 Jan 2015

@waader thanks for testing. The problem I faced in one of my plugins, was that _scripts and _styles arrays had different starting value for the core templates and I had to make some preg to unify them (pricy processing!). The links for the older IE on the other hand are directly injected in documents head, thus I didn’t touch them. But I guess we should stick to one way. I guess it would be better if we convert them to:
<link href="<?php echo $this->baseurl; ?>/templates/<?php echo $this->template; ?>/css/ie8.css" rel="stylesheet" type="text/css" />

avatar zero-24
zero-24 - comment - 27 Jan 2015

@dgt41

I guess it would be better if we convert them to:

Would you do this with this PR or with an own PR?

avatar dgt41
dgt41 - comment - 27 Jan 2015

Also this <script src="../media/jui/js/html5.js"></script> can also be written as:
<script src="<?php echo JUri::root(true); ?>/media/jui/js/html5.js"></script>

@zero-24 If you want to make a PR I will definitely test it ????

avatar zero-24 zero-24 - change - 27 Jan 2015
Status Pending Ready to Commit
avatar zero-24
zero-24 - comment - 27 Jan 2015

@zero-24 If you want to make a PR I will definitely test it :wink:

Here you go @dgt41 #5895

So i will mark this PR here as RTC :+1:


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

avatar brianteeman brianteeman - change - 28 Jan 2015
Labels Added: ?
avatar Kubik-Rubik
Kubik-Rubik - comment - 29 Jan 2015

Thank you for this optimization, @dgt41!

avatar Kubik-Rubik Kubik-Rubik - close - 29 Jan 2015
avatar zero-24 zero-24 - close - 29 Jan 2015
avatar Kubik-Rubik Kubik-Rubik - change - 29 Jan 2015
Status Ready to Commit Closed
Closed_Date 0000-00-00 00:00:00 2015-01-29 10:21:48
avatar zero-24 zero-24 - change - 14 Oct 2015
Labels Removed: ?

Add a Comment

Login with GitHub to post a comment