? ? ? ? Pending

User tests: Successful: 0 Unsuccessful: 0

avatar zero-24
zero-24
27 Apr 2020

Pull Request for Issue #27761

Summary of Changes

  • Implement a new loading=lazy attribute for images on content via a plugin that can be disabled.
  • Implement a default enabled loading=lazy option that can be overwritten by extension devs useing the options array.

As discussed here: https://volunteers.joomla.org/departments/production/reports/1226-production-dept-meeting-april-21-2020

Allow progressive features
Joomla's current policy is that we only accept changes or features that work for everything that meets our minimum requirement (e.g. PHP versions, browsers, database versions etc..)

The motion goes beyond this by allowing progressive features. This allows us to take advantage of new technology for those that use it as long as there is no adverse effect to those that don't (this should also take into account the support/documentation affects on such conditional features). As an example we could look at "lazy loading" or at "same site cookies". This motion means that a PR or ISSUE should not be closed or rejected on the argument that it IS a progressive feature. The acceptance of such an issue or feature is still subject to the usual and accepted scrutiny and discussion.

We are now even official allowed to introduce featues like this here that don't break existing sites and just improve the overall expiricece for people using modern browsers.

Feature

This PR sets the lazyloading attribute to all images that passes the onContentPrepare Event or HTMLHelper::image to allow modern browsers to lazyload the images.

More information about the loading attribute:

Testing Instructions

  • install 4.0
  • setup one article with images and atd it to the frontend
  • apply this PR
  • discover install the plugin
  • enable the plugin
  • check the frontend
  • see that it not has the lazyloading attribute
  • disable the plugin
  • see that the loading attribute is not set

Expected result

Joomla by its core can set the loading=lazy attribute to images.

Actual result

Joomla can not set that loading=lazy attribute.

Documentation Changes Required

A doc page will be produced on this, I have just added the label and assigned it to me.

Beyond this PR

There is a backport of this feature / plugin for Joomla 3 here: GitHub JED

Special thanks

Thanks @SniperSister and @felixarntz for your support on that PR.

avatar zero-24 zero-24 - open - 27 Apr 2020
avatar zero-24 zero-24 - change - 27 Apr 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 27 Apr 2020
Category SQL Administration com_admin Postgresql Language & Strings Installation Libraries Front End Plugins
avatar zero-24 zero-24 - change - 27 Apr 2020
Labels Added: ? ? ?
avatar viocassel
viocassel - comment - 27 Apr 2020

But what about loading=lazy for iframe?
For example: <iframe src="video-player.html" loading="lazy"></iframe>

avatar zero-24
zero-24 - comment - 27 Apr 2020

But what about loading=lazy for iframe?

Sounds interesting. Will take a closer look into the implementation my main concern would be that nowdays you should not use iframes that much any more right? Do you know wether there are possible side affects by adding that loading to iframes?

avatar felixarntz
felixarntz - comment - 27 Apr 2020

@viocassel @zero-24 While I think loading=lazy on iframe tags will come in handy, I think that should be revisited separately. While some browsers already support it, it's not part of any approved specification yet, so I think it would be a bit early. But following up on this in a separate effort definitely sounds like a good idea.

avatar zero-24
zero-24 - comment - 27 Apr 2020

Thanks for you input on that @felixarntz extending the plugin is easy when there is a final specification out for sure.

avatar dgrammatiko
dgrammatiko - comment - 28 Apr 2020

But what about loading=lazy for iframe?

Please, please don't use experimental features:
https://twitter.com/zcorpan/status/1242408072129187847

avatar C-Lodder
C-Lodder - comment - 29 Apr 2020

Will this not conflict with https://extensions.joomla.org/extension/photos-a-images/images/imagelazyloading?

E.g if:

A user has a Joomla 3 site
Installs this extension from JED
Updates to J4
avatar zero-24
zero-24 - comment - 29 Apr 2020

No it is a different extension name.

avatar zero-24
zero-24 - comment - 29 Apr 2020

plg_content_imagelazyloading vs plg_content_imagelazyload ;-)

avatar luisorozoli
luisorozoli - comment - 30 Apr 2020

I have tested this item successfully on d36505f

Tested successfully in Launch Joomla in J4. :)


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

avatar luisorozoli luisorozoli - test_item - 30 Apr 2020 - Tested successfully
avatar Quy
Quy - comment - 30 Apr 2020

I have tested this item successfully on d36505f


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

avatar Quy Quy - test_item - 30 Apr 2020 - Tested successfully
avatar Quy Quy - change - 30 Apr 2020
Status Pending Ready to Commit
avatar Quy
Quy - comment - 30 Apr 2020

RTC


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

avatar zero-24
zero-24 - comment - 30 Apr 2020

Thanks for all the feedback and the tests beeing done here ?

avatar laoneo
laoneo - comment - 1 May 2020

I'm really in favor to use the loading attribute. But why adding another plugin? We can just change it in the layouts to use the loading attribute and then it can be overridden when required. Every plugin has negative impact on page load so we should choose it wisely. Just my 2 cents here.

avatar zero-24
zero-24 - comment - 1 May 2020

By using a plugin everyone can disable that feature and we are not hardcoding a feature in com_content as well as this works for all extensions that fire the onContentPrepare event.

avatar laoneo
laoneo - comment - 1 May 2020

This is not an argument, otherwise we can write for everything a plugin. Our database is bloated enough, we should try to reduce it and the opposite.

avatar C-Lodder
C-Lodder - comment - 1 May 2020

@zero-24 I can't imagine why anyone would need to disable lazy loading

avatar zero-24
zero-24 - comment - 1 May 2020

I have just explained why we did it as plugin.

What would be you proposal that includes non-core extensions and you can easy disable?

As for the layouts we can and should do it for sure. But i did not want to include them here. Also the layouts would not cover the article content right?

avatar laoneo
laoneo - comment - 1 May 2020

Same as @C-Lodder, why you want to disable? This is something a 3rd party plugin should do but not core. In core we should render all img tags with the loading attribute, to fulfill a nowadays standard.

avatar laoneo
laoneo - comment - 1 May 2020

What would be you proposal that includes non-core extensions

That's their job to be up to date with latest standards. Core should adopt to best practices and then 3rd party should follow. We don't have to babysit the whole eco system.

avatar zero-24
zero-24 - comment - 1 May 2020

@zero-24 I can't imagine why anyone would need to disable lazy loading

Personal taste. I does not nessesery make sense to always lazyload all images right? There is a reasone that it is not enabled by default on the browsers too.

avatar zero-24
zero-24 - comment - 1 May 2020

What would be you proposal that includes non-core extensions

That's their job to be up to date with latest standards. Core should adopt to best practices and then 3rd party should follow. We don't have to babysit the whole eco system.

Hmm fair argument, but to me this just results into an inconsistend behavior that is not transparet nor understandable by the general user right?

avatar brianteeman
brianteeman - comment - 1 May 2020

@zero-24 I can't imagine why anyone would need to disable lazy loading

  • They might already have a solution for lazy loading
  • They might want their site to load the same on all browsers
avatar C-Lodder
C-Lodder - comment - 1 May 2020

@brianteeman It's already supported by major browser and by the time J4 is released, Safari would have fixed the webkit bug and other mobile browsers would have caught up.

This plugin is a solution but it's using a regex on the content (not exactly good practice for page loading)

avatar brianteeman
brianteeman - comment - 1 May 2020

I was answering the question not commenting on the implementation ;)

avatar laoneo
laoneo - comment - 1 May 2020

but to me this just results into an inconsistend behavior that is not transparet nor understandable by the general user right?

When J4 arrives on the horizon, then hopefully all are using the loading attribute. For the ones which don't then they will be 3rd party plugins to the rescue.

Just a side not. In your implementation, the intro image of articles are not covered.

avatar zero-24
zero-24 - comment - 1 May 2020

Just a side not. In your implementation, the intro image of articles are not covered.

Right as mention we only run on the content here for now. The changes to the layouts are planed too but for now this here takes care of the user supplied content that passes onContentPrepare and HTMLHelper::image

This plugin is a solution but it's using a regex on the content (not exactly good practice for page loading)

Just to be sure it is not a regex that runs over all content of the page just the article content.
And how would you add the lazyload thing for the article content when not with a regex?

We can just change it in the layouts to use the loading attribute and then it can be overridden when required.

That article content would not covered by an just update the layouts solution right?

As mention I agree that we should update the layouts too but I still think we have to take care about the actual content right? That is something even the extension developer should not have to reinvent on their own right?

avatar HLeithner
HLeithner - comment - 1 May 2020

I would see the lazyloading for content in the responsibility of the editor.

avatar zero-24
zero-24 - comment - 1 May 2020

Why would we want to dublicate the code three times and why should every editor have to do it on its own? And this would also make it impossible to switch off lazyloading later right?
Why should one editor handle it different than the other and why should we bound that behavior to a setting in the editors?

And also that would not cover content that is not inserted via Editor right?

avatar sanek4life
sanek4life - comment - 1 May 2020

I would see the lazyloading for content in the responsibility of the editor.

5,000 articles have already been created on my website. How do you imagine doing this? Manually edit 5,000+ images in these articles?

I do not make sites with 20-30 pages, my site has 5000+ articles.

avatar sanek4life
sanek4life - comment - 1 May 2020

Why would we want to dublicate the code three times and why should every editor have to do it on its own? And this would also make it impossible to switch off lazyloading later right?
Why should one editor handle it different than the other and why should we bound that behavior to a setting in the editors?

And also that would not cover content that is not inserted via Editor right?

There is a solution for this question: to exclude the addition of this attribute for those images in which this tag was manually added (on or off, does not matter). I mean, eliminate duplication of this attribute. I have not tested it plugin yet, but it would be nice if the plugin could see this.

avatar HLeithner
HLeithner - comment - 1 May 2020

I would see the lazyloading for content in the responsibility of the editor.

5,000 articles have already been created on my website. How do you imagine doing this? Manually edit 5,000+ images in these articles?

I do not make sites with 20-30 pages, my site has 5000+ articles.

so you think it's better that at each site impression (uncached) every article get a regex to fix an images which maybe exists instead of execute one migration script? It's even worse on category views, for each article the regex get executed.

And also that would not cover content that is not inserted via Editor right?

And of course our image helper has to be updated (I mean it should support custom attributes anyway).

Also I think your approach miss plugin, module, template output

avatar ReLater
ReLater - comment - 1 May 2020

I would see the lazyloading for content in the responsibility of the editor.

And then we have it hardcoded in the db for any f'ing image? Bad solution.

avatar sanek4life
sanek4life - comment - 1 May 2020

so you think it's better that at each site impression (uncached) every article get a regex to fix an images which maybe exists instead of execute one migration script? It's even worse on category views, for each article the regex get executed.

I think it would be nice if in Joomla there was a script for migrating all images in articles. How this is done for migrating articles in utf8mb4 (emoji symbols).

And of course our image helper has to be updated (I mean it should support custom attributes anyway).

Also I think your approach miss plugin, module, template output

I also think that it would be nice if you could add this attribute in the image editor for the article. I am not against that.

And also I think that it is necessary to leave this plugin for the CMS. Everyone can choose an acceptable path for him to add this attribute.

avatar zero-24
zero-24 - comment - 1 May 2020

There is a solution for this question: to exclude the addition of this attribute for those images in which this tag was manually added (on or off, does not matter). I mean, eliminate duplication of this attribute. I have not tested it plugin yet, but it would be nice if the plugin could see this.

There is no duplication happening as we are checking that whether there is a loading attribute already. Feel free to test it yourself.

And of course our image helper has to be updated (I mean it should support custom attributes anyway).

Just take a look at the diff of that PR here that change is already included :D

Also I think your approach miss plugin, module, template output

As mention this here is for all the user generated content that passes via onContentPrepare nothing more and nothing less ;) Fixing the layout files and template files is on the list too.

avatar HLeithner
HLeithner - comment - 1 May 2020

On question, if we make this default why isn't it default in the browser? I mean what's the drawbacks?

avatar sanek4life
sanek4life - comment - 1 May 2020

There is no duplication happening as we are checking that whether there is a loading attribute already. Feel free to test it yourself.

Great! If all this is available, then I think this is the best solution!

avatar zero-24
zero-24 - comment - 1 May 2020

On question, if we make this default why isn't it default in the browser? I mean what's the drawbacks?

That is the reason i would like to give the user and the developers the decision to disable it when they want to do it.

One example would be that you already have a lazyloading solution or you depend one the images to be loaded to calculate some stuff using JS.

avatar laoneo
laoneo - comment - 1 May 2020

Having this functionality in a plugin is not wrong. The plugin should just not be added to the core, especially when we have such ones already in the JED. You can migrate you articles with a SQL search and replace for example. Again, what core should do is to render it's own images with lazy loading, but with the correct HTML syntax and not doing some regex.

avatar laoneo
laoneo - comment - 1 May 2020

@zero-24 why do you want to add it to core, instead of taking it from the JED? I do not understand this attitude.

avatar zero-24
zero-24 - comment - 1 May 2020

@zero-24 why do you want to add it to core, instead of taking it from the JED? I do not understand this attitude.

The plugin in the JED is just intended as backport for 3.x the same I did with HTTPHeaders so the users don't have to wait for 4.x to be released and can use the same in 3.x where no new Features are added.

avatar zero-24
zero-24 - comment - 1 May 2020

Having this functionality in a plugin is not wrong. The plugin should just not be added to the core, especially when we have such ones already in the JED. You can migrate you articles with a SQL search and replace for example. Again, what core should do is to render it's own images with lazy loading, but with the correct HTML syntax and not doing some regex.

Well in the JED it is just there as backport. ;-)

It's own images? You mean the layouts and HTMLHelper right? That is done for HTMLHelper and will be done later for the layouts too.

So you think we should not care in core for the user content? Why what is the reason we should not do this? Just because I released the same code as backport plugin? That argument does not make sense to me?

avatar laoneo
laoneo - comment - 1 May 2020

So you think we should not care in core for the user content?

This is the wrong question. You should ask yourself, should core contain every functionality or should it be slim and not bloated and when you need something you take a plugin from the JED. Joomla is great because of it's eco system, but when you start to eliminate it, then it will move for me personally into the wrong direction.

And the performance impact should also be considered, the onContentPrepare event is triggered a lot during page load, doing there a regex all the time which can be fixed with a one time migration is just not worth the cost.

avatar zero-24
zero-24 - comment - 1 May 2020

So you think we should not care in core for the user content?

This is the wrong question. You should ask yourself, should core contain every functionality or should it be slim and not bloated and when you need something you take a plugin from the JED. Joomla is great because of it's eco system, but when you start to eliminate it, then it will move for me personally into the wrong direction.

I have proposed a plugin for native lazyloading not a backup component or a shop component for core right?

Are we still talking about this PR and plugin or do we have a more general discussion here?

And the performance impact should also be considered, the onContentPrepare event is triggered a lot during page load, doing there a regex all the time which can be fixed with a one time migration is just not worth the cost.

Well the one time migration would be an individual decision of the site owner taking into account any things that this individual site should be thinking of. I don't see that right now to be a core feature do you?

avatar zero-24
zero-24 - comment - 1 May 2020

This here makes sure that the most people can benefit from that new loading attribute improving the Performance of the site by using the native lazyloading of the browsers.

avatar brianteeman
brianteeman - comment - 1 May 2020

I think the issue is more about the method used to achieve this - the decision has been made already that joomla shall have this.

The possible issue I see with this plugin is that it obviously only impacts on images rendered where a content plugin functions. Wouldn't an approach that looks at all images on the page be better? Otherwise you may not really benefit from lazy-loading as you're only looking at some of the images

avatar HLeithner
HLeithner - comment - 1 May 2020

I have nothing against this functionality in core but I think core shouldn't ship 100 plugins. It think it would fit better to the template layer like our set html5 function as Service or similar don't know the best position yet. But yeah it's maybe wrong to have it in the editor (beside the fact that many people use an editor that generates styling).

We should think about a system/server/whatever to help template developers with such forward feature helpers.

avatar HLeithner
HLeithner - comment - 1 May 2020

the decision has been made already that joomla shall have this.

That's not true, it's used as example but doesn't mean it's accepted.

avatar brianteeman
brianteeman - comment - 1 May 2020

the decision has been made already that joomla shall have this.

That's not true, it's used as example but doesn't mean it's accepted.

I stand corrected

avatar sanek4life
sanek4life - comment - 1 May 2020

The possible issue I see with this plugin is that it obviously only impacts on images rendered where a content plugin functions. Wouldn't an approach that looks at all images on the page be better? Otherwise you may not really benefit from lazy-loading as you're only looking at some of the images

I think that it would be a good solution if the developer of this plugin made an option:

  1. add an attribute only for articles
  2. add an attribute for all images on the site page.
avatar sanek4life
sanek4life - comment - 1 May 2020

I’ve just thought that this plugin would really need not only for articles, but for the whole site, for the entire page of the site, and here's why:

  1. on many sites there are comments (and in the comments images, user avatars, etc.)
  2. it can also be used for other components - for example, for a forum (Kunena), etc.
  3. it can be used for the site logo and other elements of the site template.
  4. modules with images, image sliders

@zero-24 It really would be great if your plugin allowed to add this attribute for the entire page of the site, for each image (and not just for article).

avatar zero-24
zero-24 - comment - 2 May 2020

I’ve just thought that this plugin would really need not only for articles, but for the whole site, for the entire page of the site, and here's why:

  1. on many sites there are comments (and in the comments images, user avatars, etc.)

When they don't pass the content via onContentPrepare the extension dev opt-out of such preperations and than it is the responsible of the extensions provider as pointed out above.

  1. it can also be used for other components - for example, for a forum (Kunena), etc.

Yes Kunena and others can do that too.

  1. it can be used for the site logo and other elements of the site template.

That is what will be done by the upcomming layout changes.

  1. modules with images, image sliders

Image sliders should absoluty do that on itself as issues with JS that tries to check the Image that is not loaded yet etc should be checked by the extension developer.

@zero-24 It really would be great if your plugin allowed to add this attribute for the entire page of the site, for each image (and not just for article).

I don't see that to be a core plugin. The main problem is the Performance issue as mention above for running over all images on the site vs just over the images in articles and possible issues with other extensions depending on the images to be loaded.

avatar dgrammatiko
dgrammatiko - comment - 2 May 2020

FWIW this can be done in the media field plugin or for the intro/fulltext images in the respected layout. Actually since the images are always handled by that Form Field it SHOULD be done there.
THERE IS NO NEED FOR A PLUGIN for such implementation.

Eg:

if (Joomla.selectedFile.url) {
if (!isElement(editor) && (typeof editor !== 'object')) {
Joomla.editors.instances[editor].replaceSelection(`<img src="${Joomla.selectedFile.url}" alt=""/>`);
} else if (!isElement(editor) && (typeof editor === 'object' && editor.id)) {
window.parent.Joomla.editors.instances[editor.id].replaceSelection(`<img src="${Joomla.selectedFile.url}" alt=""/>`);
} else {
editor.value = Joomla.selectedFile.url;
fieldClass.updatePreview();
}

converted to:

      if (Joomla.selectedFile.url) {
        if (!isElement(editor) && (typeof editor !== 'object')) {
          Joomla.editors.instances[editor].replaceSelection(`<img loading="lazy" src="${Joomla.selectedFile.url}" alt=""/>`);
        } else if (!isElement(editor) && (typeof editor === 'object' && editor.id)) {
          window.parent.Joomla.editors.instances[editor.id].replaceSelection(`<img loading="lazy" src="${Joomla.selectedFile.url}" alt=""/>`);
        } else {
          editor.value = Joomla.selectedFile.url;
          fieldClass.updatePreview();
        }

and

<img src="<?php echo htmlspecialchars($images->image_fulltext, ENT_COMPAT, 'UTF-8'); ?>"

to:

		<img loading="lazy" src="<?php echo htmlspecialchars($images->image_fulltext, ENT_COMPAT, 'UTF-8'); ?>"

and finally

<img src="<?php echo htmlspecialchars($images->image_intro, ENT_COMPAT, 'UTF-8'); ?>"

to

				<img loading="lazy" src="<?php echo htmlspecialchars($images->image_intro, ENT_COMPAT, 'UTF-8'); ?>"

Maybe you can convert this plugin to bing the attribute to migrated/upgrated sites, eg run through the db and add the missing attribute

avatar dgrammatiko
dgrammatiko - comment - 2 May 2020

@wilsonge please don't merge this and request the changes I proposed. You don't need to do this in the runtime...

avatar dgrammatiko
dgrammatiko - comment - 2 May 2020

Anyways here: #28903 is what I think is the proper solution for introducing this attribute

avatar wilsonge
wilsonge - comment - 2 May 2020

I have to say overall I'm slightly concerned that with all these implementations we are kinda in trouble over this note in the HTML 5 Spec:

Developers are encouraged to specify an intrinsic aspect ratio via width and height attributes on lazy loaded images, even if CSS sets the image's width and height properties, to prevent the page layout from shifting around after the image loads.

Which we're obviously not doing (nor are we particularly able to do as we choose not to have a DB store of the images to keep performance - and even if we did have a store probably retrieving all that data out the DB would then ruin the performance gain from lazy loading anyhow). The reason I see this as an issue is we're adding this attribute to all images wherever they appear on the page (i.e. in category blog views even for the visible images we'll be lazy loading)


In terms of all the fighting here about plugins: I've merged @dgrammatiko 's pull request. That means that all new images added by media manager as well as all intro + fulltext images will be lazy loaded. I've also merged the HTMLHelper change from this PR: 9cd8cb3 which means images loaded via the PHP Helper will also be lazy loaded.

Someone (@dgrammatiko or @zero-24 ?) should probably add the custom fields plugins to the list of things we should fix - both media field and the imagelist field.

Maybe you can convert this plugin to bing the attribute to migrated/upgrated sites, eg run through the db and add the missing attribute

I definitely don't want J4 to touch end user data on upgrade. It's too risky - I'd much rather have users update their own sites or via a dedicated tool like DB Replacer where it's at their own risk not core's.

Images that are loaded e.g. in templates aren't covered by the plugin event here. So all this leaves us with is basically old content from previous versions of Joomla (old articles and old custom content modules (which have the plugin event enabled) and any images that are added through TinyMCE directly - largely images from outside of Joomla). And we have to decide if we want to regex through all these images or not.

I'm conscious that it's not just old data that needs a one off upgrade but also things added through the tinymce media default image handling which is an edge use case but not the most uncommon thing ever. As a result my gut is that this plugin is shipped in core but only enabled by default on Joomla 3 upgrades. On new Joomla 4 installs it's present but disabled by default. Alongside that make it a system plugin that runs through the entire page - at least then it's a one off run and covers all content on the site - because of where onContentPrepare is triggered - that probably makes it more performant.

avatar dgrammatiko
dgrammatiko - comment - 2 May 2020

Which we're obviously not doing (nor are we particularly able to do as we choose not to have a DB store

Still doable, the media field just before injecting the image tag has both the sizes (height and width) and some more extra info, so extracting this and putting in into the ratio attribute is within our reach. Ref: https://drafts.csswg.org/css-images/#intrinsic-aspect-ratio

I will try to do a PR for all the remaining fields and another one for the ratio part

avatar laoneo
laoneo - comment - 2 May 2020

It is absolutely fine to have such plugins in JED but not in core.

avatar richard67
richard67 - comment - 6 May 2020

@wilsonge Reading the long discussion above: Shall we remove RTC here?

avatar zero-24 zero-24 - change - 19 May 2020
Labels Added: ?
avatar zero-24
zero-24 - comment - 19 May 2020

As just discussed with @wilsonge this plugin is kept as content plugin but disabled on new installs. By default it is enabled on upgrades.

avatar laoneo
laoneo - comment - 20 May 2020

Is this discussion available for the public? Sorry but I do not agree here!! @dgrammatiko has already implemented it in the editor and the layouts, so we do not need a plugin which get executed on every request to change the HTML output. It can be done in a one time task.

avatar laoneo
laoneo - comment - 20 May 2020

Do a post install script or whatever, but please no content or even a system plugin. It is the first thing I do after installation, to disable all content plugins which are not needed, because it has a performance impact.

avatar dgrammatiko
dgrammatiko - comment - 20 May 2020

@laoneo I think what they try to achieve is 0 hustle upgrades from J3.
If you ask me if this is the way to do it I'll answer, as I did in a comment above: NO.
That's the reason I already started a tiny component that will do the upgrade in the J3: https://github.com/ttc-freebies/com_addlazyloading
and also another simple plugin that ports my PRs to J3: https://github.com/ttc-freebies/joomla-3.x-images-lazy-loading

All OSS all for free, I just need some time to finish those.

avatar zero-24
zero-24 - comment - 20 May 2020

Is this discussion available for the public? Sorry but I do not agree here!! @dgrammatiko has already implemented it in the editor and the layouts, so we do not need a plugin which get executed on every request to change the HTML output. It can be done in a one time task.

It was a voice discussion via Google meet. I can sum it up on request.

There is no content plugin running on all content with 4.x as mention above just on upgrades. And it is not hard build into the system and you are free to disable the pluigins. Similiar we all do for the fields when not needed for the site.

We don't want to touch user generated content and update it from the database for obvius reasons.

avatar zero-24
zero-24 - comment - 20 May 2020

Do a post install script or whatever, but please no content or even a system plugin. It is the first thing I do after installation, to disable all content plugins which are not needed, because it has a performance impact.

There is no system plugin planed as this has a higher potenzial of breaking 3rd party components.

You should always disable stuff you don't need. Similiar is true or fields when you don't need them on that site.

avatar dgrammatiko
dgrammatiko - comment - 20 May 2020

We don't want to touch user generated content and update it from the database for obvius reasons.

Joomla doesn't have to do anything. The community is already fixing this properly,, read my comment above...

avatar zero-24
zero-24 - comment - 20 May 2020

@laoneo I think what they try to achieve is 0 hustle upgrades from J3.
If you ask me if this is the way to do it I'll answer, as I did in a comment above: NO.
That's the reason I already started a tiny component that will do the upgrade in the J3: https://github.com/ttc-freebies/com_addlazyloading
and also another simple plugin that ports my PRs to J3: https://github.com/ttc-freebies/joomla-3.x-images-lazy-loading

All OSS all for free, I just need some time to finish those.

Awesome. As for the core i don't want to touch the user generadted content. But would you mind me mention your component in the upcomming docs? Explaining how to use it and ghan disable the core plugin?

avatar zero-24
zero-24 - comment - 20 May 2020

We don't want to touch user generated content and update it from the database for obvius reasons.

Joomla doesn't have to do anything. The community is already fixing this properly,, read my comment above...

Seems our commets crossed please read my commet just before that one here in response to yours ;-)

avatar laoneo
laoneo - comment - 20 May 2020

We don't want to touch user generated content and update it from the database for obvius reasons.

What for reasons?

But then you want to do some modifications on every request. That's definitely the wrong approach.

avatar brianteeman
brianteeman - comment - 20 May 2020

this plugin is kept as content plugin but disabled on new installs. By default it is enabled on upgrades

Surely the other way around?

avatar zero-24
zero-24 - comment - 20 May 2020

this plugin is kept as content plugin but disabled on new installs. By default it is enabled on upgrades

Surely the other way around?

Why? On Upgrades old content exists so when you Upgrade from 3.x to 4.x you need the plugin for the old content.

For 4.x new installs this is not required any more. For that reason the plugin is disabled by default for new installs.

avatar zero-24
zero-24 - comment - 20 May 2020

We don't want to touch user generated content and update it from the database for obvius reasons.

What for reasons?

But then you want to do some modifications on every request. That's definitely the wrong approach.

I have to disagree here I don't think it is the right approach to run a oneway regex over all content with out the user having a choice. For numberus reasons:

  • think of large sites with a lots of content
  • just remember it is not all about content onContentPrepare is also trigged in other places.
  • how to move back when for some reason a site breaks or the owner of the site dont likes lazyloading for sone reason? With a one way the site is broken with the plugin you can just disable the plugin.

I agree for an idividual site it makes sense to use the tools that Dimitris bis working on and than disable the core plugin. For that reason i would like to mention that tools on the upcomming docs page. But i don't see the core doing that for all sites running joomla.

avatar dgrammatiko
dgrammatiko - comment - 20 May 2020

For 4.x new installs this is not required any more. For that reason the plugin is disabled by default for new installs.

You do realise that this makes no sense, right?
If Joomla 4 doesn't need the plugin neither the upgrades from J3 should need a plugin. What they need is an upgrade in their content table. Joomla doesn't want to do that, fairly acceptable.
But trying to patch something with a workaround is not either a solution, further more an acceptable one.
What is needed is something that will patch the content table: https://github.com/ttc-freebies/com_addlazyloading (shortly this will be available, together with a back port of lazy loading for J3 all done with overrides)

PS I'm not attacking @zero-24 here! I even attributed his code in my component: https://github.com/ttc-freebies/com_addlazyloading/blob/a591095e7cb71a8b7c31bac9ef5dfd123cb4aebc/src/admin/models/lazy.php#L103 but the concept here is totally wrong. Fix the root, don't put bandaids...

avatar zero-24
zero-24 - comment - 20 May 2020

For 4.x new installs this is not required any more. For that reason the plugin is disabled by default for new installs.

You do realise that this makes no sense, right?
If Joomla 4 doesn't need the plugin neither the upgrades from J3 should need a plugin. What they need is an upgrade in their content table. Joomla doesn't want to do that, fairly acceptable.
But trying to patch something with a workaround is not either a solution, further more an acceptable one.
What is needed is something that will patch the content table: https://github.com/ttc-freebies/com_addlazyloading (shortly this will be available, together with a back port of lazy loading for J3 all done with overrides)

PS I'm not attacking @zero-24 here! I even attributed his code in my component: https://github.com/ttc-freebies/com_addlazyloading/blob/a591095e7cb71a8b7c31bac9ef5dfd123cb4aebc/src/admin/models/lazy.php#L103 but the concept here is totally wrong. Fix the root, don't put bandaids...

That is awesome and no worry i dont take that personal. I'm happy that we have a discussion to get the best ot for all users.

As mention in my last reply to allon i outlined my reasons to not do a one time convertion on upgrades by the core. But i'm happy to document your approach as recommend for large sites.

But at the same time I don't want to limit that feature to sites that install your componet and clean up the content tables. The ofther sites have on plugin more running a small regex to detect images. When you don't want that there are your component to take care of that.

avatar zero-24
zero-24 - comment - 20 May 2020

Ps @dgrammatiko thanks for mention me in your code but we might should mention the people from the intial code written as mention in the intial PR. As this is based on the work they did not on mine. I just did the joomla version out of that ;-)

avatar brianteeman
brianteeman - comment - 20 May 2020

@zero-24 sorry I forgot about the other changes that have been merged

avatar HLeithner
HLeithner - comment - 20 May 2020

I'm still seeing no reason for this plugin, if as already mentioned making the voodoo in every request is more bad then having an "enhance you database content" button and rewrite the database content with the same regex used in the content plugin.

And this can also be 3rd party or a seperate joomla project/plugin.

avatar zero-24
zero-24 - comment - 20 May 2020

I'm still seeing no reason for this plugin, if as already mentioned making the voodoo in every request is more bad then having an "enhance you database content" button and rewrite the database content with the same regex used in the content plugin.

And this can also be 3rd party or a seperate joomla project/plugin.

You can disable the voodoo when you want to and it is only enabled by default for upgraded sites.

As for the Upgrade your database we have the component mention by Dimitris. That will be added to the docs page about lazyloading. I dont think we should ship such a componet with the core nor dublicate the work Dimitris is donig there already.

Both approches are already 3rd party pluigins. The approache of this PR here is to do the easy part in the core to benifit all users of Joomla without breaking the sites. And even when we would break the sites it is an easy switch to turn it off.

avatar zero-24
zero-24 - comment - 20 May 2020

As for that performance argumente would it be an compromise to add a switch to disable the plugin to load on the article blog and categorie blog views. Maybe even disabled by default?

avatar laoneo
laoneo - comment - 20 May 2020

@zero-24 you say it. If somebody wants that change, then use a 3rd party extension as @dgrammatiko did. If you really want to do something on upgrade, then add a post install message, that the content should be extended with the lazy loading attribute as this is the new standard in core. Then point to the various plugins in JED.

avatar dgrammatiko
dgrammatiko - comment - 20 May 2020

Maybe even disabled by default?

Tobias the argument is that J4 is becoming extremely heavy, iirc it's 30% more plugins compared to J3... So people objecting here probably are more into a lightweight core, with proper API, instead of questionable implementations for any user case. Joomla cannot possibly solve all the possible user cases but still tries to do so. This is not sustainable, the whole thinking needs to shift to lighter, less opinionated codebase. Joomla tries to do so much and fails miserably on most of them...

avatar brianteeman
brianteeman - comment - 20 May 2020

Thinking out aloud.

I have a news site with 300 images (1 per article) which I am upgrading to j4
I am upgrade to j4 and now this plugin runs on all pages
I continue to add new content and these images will be lazy-load by default
2 years later my site has grown to 10000 images (1 per article) but I still have to run this plugin on every page just in case someone accesses those original images

Now that I have thought this through I realise that there is no need at all for this plugin. All new content will be lazy-load without it and the old content if I really want to make it lazy-load I can update the image or do some db magic. But I dont have to and can leave it all alone as well.

So if I finally understood this correctly I agree with @HLeithner and this is not needed in core at all.

avatar zero-24
zero-24 - comment - 20 May 2020

@zero-24 you say it. If somebody wants that change, then use a 3rd party extension as @dgrammatiko did. If you really want to do something on upgrade, then add a post install message, that the content should be extended with the lazy loading attribute as this is the new standard in core. Then point to the various plugins in JED.

So we agree now on the approach. Great. ?

avatar Bakual
Bakual - comment - 20 May 2020

I also don't see why this would be needed. Even for upgrades, the site will work just fine without that lazy loading attribute.
Could it be more performant? Probably. But those who care, can also solve it themself already.

For those who don't care, the lazy loading doesn't matter and the impact it has will be nothing since they likely have far bigger issues to solve.

avatar laoneo
laoneo - comment - 20 May 2020

So we agree now on the approach. Great

On which approach? To inform the user after Joomla got upgraded to 4? Yes sure, you can add there a whole summary of performance optimization suggestions. I do not care. What I care is that there is no need for a plugin.

avatar HLeithner
HLeithner - comment - 20 May 2020

This is not sustainable, the whole thinking needs to shift to lighter, less opinionated codebase. Joomla tries to do so much and fails miserably on most of them...

I think that's not true. Anyway...

I'm in favor for a Joomla managed component that does such "optional" things on upgrade, I'm sure there are more things that can be done after upgrading to j4 a more or less migration component (even if we don't need it for j4 upgrade). Which could be installed by a post upgrade message or by pointing to JED.

On the other side we see how good com_weblinks is maintained...

avatar zero-24
zero-24 - comment - 20 May 2020

Maybe even disabled by default?

Tobias the argument is that J4 is becoming extremely heavy, iirc it's 30% more plugins compared to J3... So people objecting here probably are more into a lightweight core, with proper API, instead of questionable implementations for any user case. Joomla cannot possibly solve all the possible user cases but still tries to do so. This is not sustainable, the whole thinking needs to shift to lighter, less opinionated codebase. Joomla tries to do so much and fails miserably on most of them...

This has nothing todo with this plugin than right? It is more a general thing that should be discussed in in a different issue i guess.

Btw is that 30% a real number or just a feeled number? Yes we have more plugins but for the site load we have the following right now:

Content 3.x 10; 4.x 11 (with this plugin included)
System 3.x 18 4.x 21

As for the system plugins it is that most of the new of them are not running on all sites or have sesible defaults to skip early on the site nor have a that bad impact.

What other pluigins influence the inital page load?

Sure we added a lot for media manager and webservices but they usually not run on events that Influence the intial page load right?

avatar dgrammatiko
dgrammatiko - comment - 20 May 2020

This is not sustainable, the whole thinking needs to shift to lighter, less opinionated codebase. Joomla tries to do so much and fails miserably on most of them...
I think that's not true. Anyway...

I wouldn't make such a comment without backing it with some hard data, and yes Joomla is failing hard to deliver competitive sites: https://joomla-needed-fixes.netlify.app/data/ (this is all of the showcase sites with most of them still running on HTTP instead of HTTPS, having a blank page for more than 3 seconds before anything is painted, etc)

Sorry that's the hard truth ?

Btw is that 30% a real number or just a feeled number

Oh yes, it's very real, count ALL the plugins.

avatar zero-24
zero-24 - comment - 20 May 2020

I also don't see why this would be needed. Even for upgrades, the site will work just fine without that lazy loading attribute.
Could it be more performant? Probably. But those who care, can also solve it themself already.

For those who don't care, the lazy loading doesn't matter and the impact it has will be nothing since they likely have far bigger issues to solve.

Yes that is the point what speaks against moving that to the most sites by default? I guess you all saw that post of the motivation behind lazyloading and that images are the things that are consuming the most resouces on pageload as well as on the servers.

So i would like to turn the questions why should we not do that?

As it improves the Performance for all sites with nearly zero costs for us nor the site owner.

The reality is most of them don't care about all that that's the reason i think we should come up with good defaults thag can be disabled by the people that do have issues with it.

So the general people can stop claming 'joomla is bad for performance because they dont do layzloading' like they do for other things we dont do.

It is something that is build in native in the browsers not a JS lib with a gazillion depentecys right?

avatar dgrammatiko
dgrammatiko - comment - 20 May 2020

are the things that are consuming the most resouces on pageload as well as on the servers

NO, you're wrong here. The problem is the excessive CSS (bootstrap and font awesome) and all the JS that is not deferred. These are RENDER BLOCKING meaning the visitor of the site stairs on a blank screen until these are downloaded, parsed and executed. Images are not render blocking thus the only negative effect is their size, which by the way loading=lazy is not solving

avatar zero-24
zero-24 - comment - 20 May 2020

According to HTTPArchive, images are the most requested asset type for most websites and usually take up more bandwidth than any other resource. At the 90th percentile, sites send about 4.7 MB of images on desktop and mobile. That's a lot of cat pictures.

https://web.dev/native-lazy-loading/

This is what i'm talking about. @dgrammatiko

We don't fix all issues with it for sure but we can make a good step in the right direction for all users of Joomla.

avatar dgrammatiko
dgrammatiko - comment - 20 May 2020

We don't fix all issues with it for sure

The loading=lazy can hardly be assumed as a solution to the size images. For that you need srcset and a plugin like: https://ttc-freebies.github.io/plugin-responsive-images/
Which by the way I wrote it 3 years ago with the hope that the GSOC would have brought it in the core (I didn't do that due to lack of time, I was already at that point involved in way too many sub-projects). What they did instead was to try, again, to fulfil a niche market with adaptive images which is a total failure from workflow perspective (upload the image, then go edit it, then use it, and then nobody will follow such stupid workflow in 2020 ?).
In short, what I stated before: Joomla tries to solve every possible user case and it fails miserably...

avatar zero-24
zero-24 - comment - 20 May 2020

We don't fix all issues with it for sure

The loading=lazy can hardly be assumed as a solution to the size images. For that you need srcset and a plugin like: https://ttc-freebies.github.io/plugin-responsive-images/
Which by the way I wrote it 3 years ago with the hope that the GSOC would have brought it in the core (I didn't do that due to lack of time, I was already at that point involved in way too many sub-projects). What they did instead was to try, again, to fulfil a niche market with adaptive images which is a total failure from workflow perspective (upload the image, then go edit it, then use it, and then nobody will follow such stupid workflow in 2020 ?).
In short, what I stated before: Joomla tries to solve every possible user case and it fails miserably...

Agree what about starting an RFC on your proposed pluigin and with your permission we build a core plugin based on yours. You don't have to do the coding when you don't want to. But starting the discussion would help. I for example was not aware of that plugin.

avatar wilsonge wilsonge - change - 30 May 2020
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2020-05-30 14:46:09
Closed_By wilsonge
avatar wilsonge wilsonge - close - 30 May 2020
avatar wilsonge wilsonge - merge - 30 May 2020
avatar wilsonge
wilsonge - comment - 30 May 2020

I've had a chat with @zero-24 about this offline and have agreed to merge this (it was my idea to only have it enabled for existing installs). I do understand the cons of performance here. And definitely we should be advocating for people to fix this properly in the database. But I don't think it's realistic to believe that the majority of people will do this either.

avatar dgrammatiko
dgrammatiko - comment - 30 May 2020

@wilsonge even the googler devs were against this: GoogleChrome/lighthouse-stack-packs#44 (comment)

Also do you realise that I already have:

  • proper back port for J3 (plugin)
  • proper db update (component)

You want to have them under Joomla because you don't trust me? Fork them, their OSS

avatar wilsonge
wilsonge - comment - 30 May 2020

No it was the google devs suggestion. Tobias was the one interacting with them so I think he knows best what they were suggesting

avatar dgrammatiko
dgrammatiko - comment - 30 May 2020

@wilsonge probably it's my bad English but even Tobias responds that the plugin will be the first on the search. So it wasn't their suggestion at any point
Screenshot 2020-05-30 at 18 57 33

avatar Bramfield
Bramfield - comment - 10 Aug 2023

Sounds interesting. Will take a closer look into the implementation my main concern would be that nowdays you should not use iframes that much any more right? Do you know wether there are possible side affects by adding that loading to iframes?

How can this be implemented for an iframe?


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

avatar zero-24
zero-24 - comment - 10 Aug 2023

That plugin has been reverted out of the core before the final release was made.

Please find here more details about lazy loading iframes:

https://web.dev/iframe-lazy-loading/

Add a Comment

Login with GitHub to post a comment