Skip to content
This repository was archived by the owner on Oct 19, 2022. It is now read-only.
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions js/lazy-load.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,22 @@

function lazy_load_image( img ) {
var $img = jQuery( img ),
src = $img.attr( 'data-lazy-src' );
src = $img.attr( 'data-lazy-src' ),
srcset = $img.attr( 'data-lazy-srcset' ),
sizes = $img.attr( 'data-lazy-sizes' );

if ( ! src || 'undefined' === typeof( src ) )
return;

if ( 'undefined' !== typeof( srcset ) && srcset ) {
$img.removeAttr( 'data-lazy-srcset' );
$img.removeAttr( 'data-lazy-sizes' );
img.sizes = sizes;
img.srcset = srcset;
}

$img.unbind( 'scrollin' ) // remove event binding
.hide()
// .hide() // Flickering looks bad
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we remove the fadeIn below as well?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fade() should be ok. It did not hurt in the eyes. ;)

.removeAttr( 'data-lazy-src' )
.attr( 'data-lazy-loaded', 'true' );

Expand Down
19 changes: 13 additions & 6 deletions lazy-load.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ static function setup_filters() {
add_filter( 'the_content', array( __CLASS__, 'add_image_placeholders' ), 99 ); // run this later, so other content filters have run, including image_add_wh on WP.com
add_filter( 'post_thumbnail_html', array( __CLASS__, 'add_image_placeholders' ), 11 );
add_filter( 'get_avatar', array( __CLASS__, 'add_image_placeholders' ), 11 );
?>
<noscript><style type="text/css">.lazy { display:none; }</style></noscript>
<?php
}

static function add_scripts() {
Expand Down Expand Up @@ -62,7 +65,7 @@ static function add_image_placeholders( $content ) {

static function process_image( $matches ) {
// In case you want to change the placeholder image
$placeholder_image = apply_filters( 'lazyload_images_placeholder_image', self::get_url( 'images/1x1.trans.gif' ) );
$placeholder_image = apply_filters( 'lazyload_images_placeholder_image', self::get_url( 'images/1x1.trans.gif' ), $matches );

$old_attributes_str = $matches[2];
$old_attributes = wp_kses_hair( $old_attributes_str, wp_allowed_protocols() );
Expand All @@ -73,13 +76,17 @@ static function process_image( $matches ) {

$image_src = $old_attributes['src']['value'];

// Remove src and lazy-src since we manually add them
$new_attributes = $old_attributes;
unset( $new_attributes['src'], $new_attributes['data-lazy-src'] );
// Also retain the srcset, and sizes if present. The latter is to keep w3c validator happy (sizes requires srcset)
$lazy_srcset_attribute = ( ! empty( $old_attributes['srcset'] ) ) ? 'data-lazy-srcset="' . $old_attributes['srcset']['value'] . '"' : '';
$lazy_sizes_attribute = ( ! empty( $old_attributes['sizes'] ) ) ? 'data-lazy-sizes="' . $old_attributes['sizes']['value'] . '"' : '';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of creating the attribute here, we should just assign the value and build it as part of the HTML further down. Similar to how we're handling the src attribute. This helps us more clearly escape the attribute value.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we don't have a value for srcset and/or sizes?

Is it safe to have empty values?

// Remove src, lazy-src, srcset, lazy-srcset, sizes and data-lazy-sizes since we manually add them
$new_attributes = $old_attributes;
$css_class = $new_attributes['class'] ? $new_attributes['class']['value'].' lazy' : 'lazy';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should scope the classname a bit more to avoid conflicts (maybe lazy-load-img?)

unset( $new_attributes['src'], $new_attributes['data-lazy-src'], $new_attributes['srcset'], $new_attributes['data-lazy-srcset'], $new_attributes['sizes'], $new_attributes['data-lazy-sizes'], $new_attributes['class'] );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The indentation should be fixed here (should be using tabs, not spaces).


$new_attributes_str = self::build_attributes_string( $new_attributes );

return sprintf( '<img src="%1$s" data-lazy-src="%2$s" %3$s><noscript>%4$s</noscript>', esc_url( $placeholder_image ), esc_url( $image_src ), $new_attributes_str, $matches[0] );
$placeholder_url = strpos($placeholder_image, 'data:') == 0 ? $placeholder_image : esc_url( $placeholder_image );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can just pass allowed schemes into esc_url instead:

esc_url( $placeholder_url, array( 'http', 'https', 'data' ) );

return sprintf( '<img src="%1$s" class="%2$s" data-lazy-src="%3$s" %4$s %5$s %6$s><noscript>%7$s</noscript>', $placeholder_url , $css_class, esc_url( $image_src ), $lazy_srcset_attribute, $lazy_sizes_attribute, $new_attributes_str, $matches[0] );
}

private static function build_attributes_string( $attributes ) {
Expand Down