4.0 HTMLHelper - lazyloading

[HaJuSi-23]

Steps to reproduce the issue

on call an error on line 693 is the result.
Looks like "lazy" dosn't exist.
MfG HaJuSi

Expected result

Actual result

System information (as much as possible)

Additional comments

[HLeithner]

can you post the complete error message please?

[SharkyKZ]

My guess it's referring to Warning: illegal string offset 'loading' because $attribs can be a string. Don't rush fixing this because the PR that added this code should just be reverted.

[HaJuSi-23]

Warning: Illegal string offset 'loading' in /mnt/web101/....../htdocs/ligatest/libraries/src/HTML/HTMLHelper.php on line 693

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30615.}

[dgrammatiko]

because the PR that added this code should just be reverted

Any reason why? I'm really curious to know why Joomla shouldn't support something that magically improves perforrmance

[HaJuSi-23]

Joomla! Version 4.0.0-beta4-dev
PHP Version 7.4.9
Identity user
Response 200
Template atum
Database
ServermysqlVersion5.6.42-logCollationutf8_general_ciConn Collationutf8mb4_general_ci
'------------------------'
$_GET
array:2 [ "option" => "com_joomleague" "view" => "projects" ]
$_POST
[]
$_SESSION
array:2 [ "PHPDEBUGBAR_STACK_DATA" => [] "joomla" => "TzoyNDoiSm9vbWxhXFJlZ2lzdHJ5XFJlZ2lzdHJ5Ij...
$_COOKIE
array:3 [ "wf_browser_dir" => "" "554c794a85683f20e9080da206be01cf" => "av7picjj52tsnv0ehigliege...
$_SERVER
array:46 [ "HTTP_HOST" => "www.ligatest.ksv-nesselblatt.de" "REMOTE_ADDR" => "31.18.238.55" "T...
'--------------'
session
array:4 [ "timer" => array:3 [ "start" => 1599887952 "last" => 1599888711 "now" => 159...
registry
array:3 [ "data" => array:1 [ "com_joomleaguesportstypes" => "1" ] "initialized" => false ...
user
array:20 [ "id" => 290 "name" => "Administrator" "username" => "admin" "email" => "webmaster...
application
array:1 [ "queue" => [] ]

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30615.}

[SharkyKZ]

Any reason why? I'm really curious to know why Joomla shouldn't support something that magically improves perforrmance

Supporting it and forcing it by default are different things.

[dgrammatiko]

forcing it by default

You do realize that all browsers support it and it’s a platform feature, right? So yeah you re right why force it when you can be the the worst (in terms of performance) cms out there...

PS. Just to be clear here I'm talking about loading=lazy for the content images (eg editor images and intro/fulltext), about HTMLHelper::image I guess since there is an option to pass any attribute the default forcing of the attribute is probably wrong, so in this case I will agree on reverting the forced attribute there

[HaJuSi-23]

Dump HTMLHelper Line 691
"path-> " int(0)
"file-> " "/administrator/components/com_joomleague/assets/images/projects.png"
"attributes-> " NULL

All requested images exist at the requested place.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30615.}

[schultz-it-solutions]

Sorry, I somehow did not see this issue here, when I opened
#30647

My approach for solving this would probably be to change the methods parameters from
public static function image($file, $alt, $attribs = null, $relative = false, $returnPath = 0)
to
public static function image($file, $alt, $attribs = array(), $relative = false, $returnPath = 0)

[SharkyKZ]

Some more tinkering would be needed because $attribs can be a string. Anyways, if there's no objection to removing forced loading attribute, let's do that instead.

[HLeithner]

Would it make sense to deprecate string support in the future?

[HaJuSi-23]

I changed it to "$attribs=array()", but no improvement.
I'm now dump backward, to find out what's happen with "$attribs", why it's "NULL"

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30615.}

[schultz-it-solutions]

Together with the change of the methods parameters, this should do the trick (might be optimized code-wise):

	// Default to lazy you can disable lazyloading by passing $attribs['loading'] = 'eager';
	if (is_array($attribs) )
	{
		if (!isset($attribs['loading']))
		{
			$attribs['loading'] = 'lazy';
		}
	}
	else
	{
		$loading = stripos($attribs, 'loading');
		if ($loading === false) {
			$attribs .= " loading='lazy' ";
		}
	}
	return '<img src="' . $file . '" alt="' . $alt . '" ' . trim((\is_array($attribs) ? ArrayHelper::toString($attribs) : $attribs)) . '>';

[schultz-it-solutions]

As per the discussion about "forcing" this, you would have to invoke yet another configuration parameter to give the site owner the possibility to decide. I am not sure the additional complexity (new config field and language strings. Probably "active" information about this new feature to all Joomla webmasters - otherwise this might be overlooked easily) is worth the trouble.

I had a similar discussion three years ago (see #17827 ) where it was decided to NOT INVOKE additional configuration settings.

[zero-24]

Would it make sense to deprecate string support in the future?

That would be the way to go in my opinion given that the options array allow us to extend the code without breaking anything.

For now I'm closing this issue here as this is also fixed with #30748 by adding an is_array check for now :)

[richard67]

New PR is #30790. Please test.