? Pending

User tests: Successful: Unsuccessful:

avatar HLeithner
HLeithner
4 Jan 2020

This PR easily breaks existing installations.

avatar HLeithner HLeithner - open - 4 Jan 2020
avatar HLeithner HLeithner - change - 4 Jan 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 4 Jan 2020
Category Front End Plugins
avatar brianteeman
brianteeman - comment - 4 Jan 2020

Could you please explain in more detail what breaks

avatar HLeithner
HLeithner - comment - 4 Jan 2020

Every template loading it's own jquery could break because if you don't have a override to not load the tooltip has an additional jquery loaded. I had this problem already with another bugfix release in plg_content_pagenavigation

avatar brianteeman
brianteeman - comment - 5 Jan 2020

Sorry but I dont see how the changes here could do that

avatar mbabker
mbabker - comment - 5 Jan 2020

The only side effect introduced by that PR is the potential load of jQuery and Bootstrap on the frontend in a new location. If you’re saying that is a problem then the frontend needs to be rewritten so that it ONLY generates semantic markup with zero JavaScript enhancement or make Joomla a headless CMS and tell people they’re SOL if looking for core to be usable out-of-the-box.

avatar HLeithner
HLeithner - comment - 5 Jan 2020

The concept is that I don't have to override each template of joomla and adding a javascript framework in the bugfix release is unexpected

avatar brianteeman
brianteeman - comment - 5 Jan 2020

but it doesnt break anything

avatar HLeithner
HLeithner - comment - 5 Jan 2020

it does if you don't use joomla jquery

avatar Bakual
Bakual - comment - 5 Jan 2020

Afaik JQuery as well as tooltips are loaded using a JTHML function and thus should be overrideable. So if you did the JQuery and/or tooltip replacement the proper way, then this doesn't break anything at all.
If your template however manipulated the head of the document and this way removed the core JQuery(which it shouldn't), then it may have unforeseen issues.

In the end I agree with Brian and Michael that this shouldn't be reverted. It's not really a new feature which would mandate to go into a minor release. Also, the rendered markup isn't part of our backward compatibility promise as per https://developer.joomla.org/development-strategy.html. We excluded that at the time especially in mind of issues like this one.

avatar brianteeman
brianteeman - comment - 5 Jan 2020

Can you please provide a more detailed real word example of how this breaks a site. I have tested this on several sites that do not use joomla jquery without issue

avatar HLeithner
HLeithner - comment - 5 Jan 2020

It's not impossible that I'm doing it wrong so what would be the prefered method not getting additional js loaded (jquery and tooltip) without overriding each layout? Or don't wanting to get jquery loaded at all?

avatar brianteeman
brianteeman - comment - 5 Jan 2020

I am sure there are a million ways to do it but this is what I do

On my main site I do

function RemoveJavascript()
{
	$removeJs = array(
		'media/jui/js/jquery.min.js',
		'media/jui/js/jquery.js',
		'media/jui/js/jquery-noconflict.js',
		'media/jui/js/jquery-migrate.min.js',
		'media/jui/js/jquery-migrate.js',
		'media/system/js/tabs-state.js',
		'media/jui/js/bootstrap.min.js',
		'media/jui/js/bootstrap.js',
		'media/system/js/mootools-core.js',
		'media/system/js/core.js',
		'media/system/js/caption.js',
	);

	$document = JFactory::getDocument();

	$scripts = $document->_scripts;

	foreach ($removeJs as $signature)
	{
		foreach($scripts as $script => $scriptdata)
		{
			if(stristr($script, $signature))
			{
				unset($scripts[$script]);
			}
		}
	}

	$document->_scripts = $scripts;
}


and then if I want to use bs from a cdn or something completely different I do something like


	<script src="//code.jquery.com/jquery-1.10.2.min.js"></script>
	<script src="//code.jquery.com/jquery-migrate-1.2.1.min.js"></script>
	<script src="//netdna.bootstrapcdn.com/bootstrap/3.0.3/js/bootstrap.min.js"></script>

Or if I want to use bootstrap locally just a newer version then I override the files in media/jui/js/ by creating a template override folder as follows containing files with the same name
templates/<templatename>/js/jui/

avatar HLeithner
HLeithner - comment - 5 Jan 2020

I'm doing something similar in a plugin onBeforeCompileHead but got a problem on a change in the plg_content_pagenavigation

avatar brianteeman
brianteeman - comment - 5 Jan 2020

i would say that the problem is in your code then. I tested this on three different sites all of which are not using joomla jquery in different ways and had no problems at all

avatar Bakual
Bakual - comment - 5 Jan 2020

If you want to get rid of tooltips or use a different implementation, you can write a plugin which overrides the JHtml::tooltip function. I don't know exactly how it has to be done, but I'm sure I've read explanations from Michael once.
The same should be possible for JQuery as it is loaded with a JHtml function as well (in core, 3rd party is a different topic).

avatar dgrammatiko
dgrammatiko - comment - 5 Jan 2020

@brianteeman @HLeithner couple of things here as I see that there are some comments that are totally wrong here:

  • Multiple CDNs will make your site slower!. The reason is that the time for resolving the DNS, do the TLS handshake and establishing a connection is not negligible, it's around 300-500ms per domain. Also ALL NEW BROWSERS RUN SITES IN TOTAL ISOLATION meaning that the cached jQuery from Joomla.org is not available to myawesomesite.com...

  • The way to override static assets is by putting the override in the template css/js/images folder.

  • Removing an asset requires you to create an HTMLHelper override plugin that will not add the asset in question. Of course this is quite a ride for any frontender and thus Joomla is pretty much a no-go for anyone that doesn't want to write some 1000's lines of code to achieve what is a no brainer in other systems.

  • [EDIT] Last but not least LAYOUTS are not part of the B/C promise according to the docs

avatar HLeithner
HLeithner - comment - 5 Jan 2020

i would say that the problem is in your code then. I tested this on three different sites all of which are not using joomla jquery in different ways and had no problems at all

I didn't tested this PR if it breaks, I only know that the last PR adding a tooltip has broken many of my sites.

@dgrammatiko I don't use cdn ;-)

Last but not least LAYOUTS are not part of the B/C promise according to the docs

It's not necessary needed to do it ;-)

I will test this later, but a good solution to this would be interesting (overriding jhtml correctly). And simply overriding jquery doesn't work for me because I use jquery 1.12 and 2.x (I know not necessary any longer but I still have customers requiring legacy browsers...)

avatar dgrammatiko
dgrammatiko - comment - 5 Jan 2020

@dgrammatiko I don't use cdn ;-)

Well, you might not but Brian shared some snippet above which is totally outdated. Since Spectre and Meltdown ALL browsers run each site in isolation, meaning the cache is per site, meaning that no asset will ever be shared across different domains, even if those assets point in the same CDN (the cache bucket is per site).
I just wanted to point this out as people might have wrong assumptions on how browsers really work these days and blindly copy paste those lines...

PS This could be educational: https://youtu.be/yOcgGSCrn-c?t=789

avatar ReLater
ReLater - comment - 6 Jan 2020

Simply remove the line JHtml::_('bootstrap.tooltip'); if you think that's a B\C break. All other changes are not.

avatar ReLater
ReLater - comment - 6 Jan 2020

BTW. I hate these ugly BS tooltips ;-)

avatar chmst
chmst - comment - 6 Jan 2020

If it is about the tooltip - remove it. There are situations when they are useful or necessary but not here

avatar HLeithner
HLeithner - comment - 8 Jan 2020

Merged #27436 as compromise.

avatar HLeithner HLeithner - change - 8 Jan 2020
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2020-01-08 21:20:54
Closed_By HLeithner
Labels Added: ?
avatar HLeithner HLeithner - close - 8 Jan 2020

Add a Comment

Login with GitHub to post a comment