Lazy loading in 5.5 causes flashing of custom logo in Firefox

[demetris]

That is, when navigating from post to post or from page to page using, for example, the site menu links, the logo flashes sometimes — not always but more often than not.

Happens with Twenty Twenty as well as a custom theme I am working on.

Happens with Firefox 80 (Beta channel) and Firefox 81 (Nightly channel) on Windows 10.

Does not happen with Chrome 84 on Windows 10.

Does not happen if I disable lazy loading using wp_lazy_loading_enabled.

Do other people see the same behaviour?

If yes, I imagine this is something that could or will be fixed in Firefox, but maybe we could do something in the meantime.

For example, we could disable lazy loading for the logo image when calling wp_get_attachment_image() from get_custom_logo() — unless we have reasons to use lazy loading for the custom logo.

Cheers!

[johnbillion]

Moving to 5.5.1 for investigation.

[khag7]

I'm having trouble reproducing. WP 5.5, Twenty Twenty, no plugins. Maybe I'm not sure exactly what I'm looking for.

@demetris can you reproduce it on a clean install? Can you show a screen recording of what you're seeing? Or anyone else able to reproduce this?

[demetris]

Thanks for having a look, Kevin.

Here is a screencast of what I am seeing:

​https://vimeo.com/449575076

Using Firefox 80 (Beta channel) I click 17 times to navigate from page to page. The flashing happens 3 times. Usually, it happens more often.

The layout shift along with the flashing is, I think, a by-product of the issue I am seeing. It does not happen if the logo dimensions are specified in the stylesheet.

Here is the test website:

​https://0.kikizas.net/

It is on WordPress 5.5 and has two active plugins: GitHub Updater and Two Factor.

And here is a plugin I just made (can be installed with GitHub Updater) to quickly disable native lazy loading in WordPress 5.5:

​https://github.com/demetris/disable-native-lazy-loading

[demetris]

Forgot to add mention: @khag7

[khag7]

I guess I noticed that and thought it was just the page redrawing.

It seems like the layout is shifting from before the image is loaded to after.

Ticket #50367, titled "Avoid layout shifting due to images not having dimension attributes" seems to describe a similar situation. In that ticket, they describe the necessity for lazy loaded images to have a height and a width defined. Our site logo has that (at least on Twenty Twenty) but it also has CSS styles which affect the height and the width. So if an image is larger than what the CSS allows for, maybe there's some issue?

Before I go too far down that path, though, should we just consider not lazy loading the site logo? If the logo is at the top, one of first thing shown in the vast majority of themes, it's not going to be a performance impact to just load it normally. If it fixes this bug, great. Is there a downside?

EDITS:

1. Another thing I notice. Your site logo on your test site is 576px square. The one I was testing with was 120x90, and I am not having any issue. When I try a larger image on my site, I am having the issue. If you look at the HTML on your test site, the height and width attributes of the image are showing 576 for each, but the CSS prevents it from showing that large. Not sure why this would affect anything involving lazy loading, but I'm observing that the flashes occur if lazy loading is enabled for the site logo and if height and width are at that size.

2. Everything works fine if I disable lazy loading in firefox (about:config and then find the flag called dom.image-lazy-loading.enabled and disable it). Or you can disable it in wordpress with add_filter('wp_lazy_loading_enabled', '__return_false'); -- either way, disabling lazy loading from server or client fixes the problem.

3. As far as disabling lazy loading for just the logo, this works:

   add_filter( 'get_custom_logo_image_attributes', function( $attributes ){
   	$attributes['loading'] = 'eager';
   	return $attributes;
   });
   

4. Related: #50425 - where we were warned against lazy-loading images that are likely to be in the initial viewport.

[demetris]

Replying to khag7:

Thanks for investigating and for confirming!

Before I go too far down that path, though, should we just consider not lazy loading the site logo? If the logo is at the top, one of first thing shown in the vast majority of themes, it's not going to be a performance impact to just load it normally. If it fixes this bug, great. Is there a downside?

I think the same.

It seems to me the best thing we can do is to just stop adding the loading attribute to the custom logo.

We can do that by adding 'loading' => false to the array of attributes we pass to wp_get_attachment_image() in get_custom_logo().

As it is now, we do something that is not needed and that we get no benefit from doing. That is, we ask the browser to decide whether to lazy load an image that:

1. We know should be in the initial viewport

2. We know should also be in the browser cache for subsequent visits and subsequent views

The only thing we get in return is that we trigger a jarring UX in Firefox.

[rebasaurus]

Disable lazy loading as default

[demetris]

Disable lazy loading for custom logo, with explanatory comment

[demetris]

Thanks for the quick patching, @rebasaurus

The only thing I would do differently is include a comment, so that people in the future know the reason for the special treatment.

I am attaching a patch with explanatory comment as a second option.

[audrasjb]

Just noting we’re not really/only fixing the issue for Firefox, but rather because custom logos are probably more often located at the top of the screen and most of the time don't need to be lazy loaded.

For theme authors who want to re-add the lazy load, they can use the get_custom_logo_image_attributes filter.

[audrasjb]

Otherwise, the patch is functional on my side, thanks

[audrasjb]

Media: Use false value for loading attribute to disable lazy loading by default on get_custom_logo()

[audrasjb]

In 50933.1.diff, let's use an @since comment to explain the change in the function’s changelog.

[desrosj]

In 48870:

Media: Disable lazy-loading for custom logos by default.

Custom site logos are most commonly displayed above the fold, so lazy-loading is unnecessary.

Props demetris, khag7, johnbillion, rebasaurus, audrasjb.
Fixes #50933.

[desrosj]

Reopening for backport.

[desrosj]

In 48871:

Media: Disable lazy-loading for custom logos by default.

Custom site logos are most commonly displayed above the fold, so lazy-loading is unnecessary.

Props demetris, khag7, johnbillion, rebasaurus, audrasjb.
Merges [48870] to the 5.5 branch.
Fixes #50933.

[SergeyBiryukov]

In 48874:

Tests: Update unit tests to account for lazy-loading being disabled for custom logos by default.

Follow-up to [48870].

See #50933.

[SergeyBiryukov]

In 48875:

Tests: Update unit tests to account for lazy-loading being disabled for custom logos by default.

Follow-up to [48870].

Merges [48874] to the 5.5 branch.
See #50933.

[SergeyBiryukov]

In 48878:

Tests: Update one more test to account for lazy-loading being disabled for custom logos by default.

Follow-up to [48870], [48874].

Props desrosj.
See #50933.

[SergeyBiryukov]

In 48879:

Tests: Update one more test to account for lazy-loading being disabled for custom logos by default.

Follow-up to [48870], [48874].

Props desrosj.
Merges [48878] to the 5.5 branch.
See #50933.