Duplicate code and a feature request

  • Resolved mackzwellz

    (@mackzwellz)


    6 years, 8 months ago

    Trying to have my site not to serve mixed content under certain CDN provider’s Flexible SSL (successful result included in the code snippet below) I discovered that you seem to have a duplicated code in your plugin:

    ...
// Remove sizes and srcset attribute.
$img->removeAttribute( 'sizes' );
$img->removeAttribute( 'srcset' );

// TODO: add "} // End if()." and delete everything below until next TODO
// Get src value.
$src = $img->getAttribute( 'src' );

// Check if we have a src.
if ( '' === $src ) {
// Set the value from data-noscript as src.
$src = $img->getAttribute( 'data-noscript' );
} // End if().

// Set data-src value.
$img->setAttribute( 'data-src', $src );
} else {
// TODO: Stop here
// Get src attribute.
$src = $img->getAttribute( 'src' );
// TODO: Review new optional code: stripping HTTP and HTTPS from src URLs
// or could you even maybe make it toggleable via checkbox in admin panel?
$src = str_replace(['http:', 'https:'], '', $src);
			
			// Check if we do not have a value.
			if ( '' === $src ) {
				// Set the value from data-noscript as src.
				$src = $img->getAttribute( 'data-noscript' );
			} // End if().

			// Set data-src value.
			$img->setAttribute( 'data-src', $src );
// TODO: Delete the line below too.
}// End if().

    I would be happy to be proven wrong, but after my changes everything works as it did before, so consider it both a minor bug report and a minor feature request.

Viewing 3 replies - 1 through 3 (of 3 total)
  • Plugin Author Florian Brinkmann

    (@florianbrinkmann)

    Hi,

    oh, you are right, the else is not necessary there, thanks! Changed that (https://github.com/florianbrinkmann/lazy-loading-responsive-images/blob/master/src/Plugin.php).

    To the protocol thing: so you would like an option that removes the http or https from the image URL? Can you eplain what exactly is the problem that you have with the CDN providers?

    Thanks,
    Florian

    Thread Starter mackzwellz

    (@mackzwellz)

    Well, in certain cirumstances (site with Cloudflare Flexible SSL + HTTPS Rewrites and no own server SSL) data-src and <nosript> img src attribute URLs aren’t replaced, and on image load page turns into mixed SSL.
    Oh, and reviewing the code a bit more, I came up with a better place to put this string:

    * Replace images with placeholders in the content

...

// Strip protocol scheme from src and data-src
$replace = str_replace(['http:', 'https:'], '', $replace);

$content = str_replace( $search, $replace, $content );

    I’d suggest making it a checkbox option, or let users even choose – strip both HTTP and HTTPS, replace HTTP with HTTPS or do nothing. Or maybe you could make the plugin choose automatically (i.e. check site adress protocol scheme in wp options) – and make it a fourth option! (alright, this might be a bit over the top). Either way, the best practice right now is to always use HTTPS, but not everyone may be using it still obviously.

    Plugin Author Florian Brinkmann

    (@florianbrinkmann)

    Hey,

    thanks for explanation and your code, I will think about the best way to implement that feature!

    Best,
    Florian

Viewing 3 replies - 1 through 3 (of 3 total)
  • The topic ‘Duplicate code and a feature request’ is closed to new replies.

Tags