• Paul

    (@paultgoodchild)


    Hi,

    First up, great plugin and many thanks for your continued support with it!

    You have function in the plugin: https_cleanup

    And you filter the URL only in the admin screen. Would it make more sense to add this filter by default to the loading on all images on the widgets? Currently this is not the case. So I simply added the code:

    add_filter( 'image_widget_image_url', array( $this, 'https_cleanup' ) );

    after line 36 in your main class. I’d love for this to be either an option to set, or just default in the core code.

    Is this do-able?

    Many thanks!
    Paul.

    https://www.ads-software.com/extend/plugins/image-widget/

Viewing 15 replies - 1 through 15 (of 16 total)
  • Honestly, I’ve never used the https aspect of this plugin. All the work on that to date has been user contributed. But I’m pretty sure that we’ve had some back and forth on specifically what you’re talking about. Since I’ve never used it and don’t know the impacts I can’t speak intelligibly about why it didn’t work.

    I think if you search the forum you’ll find the previous conversations on the matter.

    Actually, here’s the last round of talk on this topic:

    https://www.ads-software.com/support/topic/plugin-image-widget-force_ssl_admin-results-in-https-links-to-image

    I’d have to poke at it again but it sounds like that actually was already implemented. Maybe just in the wrong place in the code?

    Thread Starter Paul

    (@paultgoodchild)

    Hey Peter,

    Actually, the last poster on that thread implemented exactly what I did. He added the line that I referred to above at the end of Tribe_Image_Widget()

    This works perfectly. So here basically is the fully Tribe_Image_Widget() with the added line at the end:

    function Tribe_Image_Widget() {
    	$this->loadPluginTextDomain();
    	$widget_ops = array( 'classname' => 'widget_sp_image', 'description' => __( 'Showcase a single image with a Title, URL, and a Description', 'image_widget' ) );
    	$control_ops = array( 'id_base' => 'widget_sp_image' );
    	$this->WP_Widget('widget_sp_image', __('Image Widget', 'image_widget'), $widget_ops, $control_ops);
    	add_action( 'admin_init', array( $this, 'admin_setup' ) );
    
    	add_filter( 'image_widget_image_url', array( $this, 'https_cleanup' ) );
    }

    This will basically ensure that on the front-end display, the image display reflects whether it’s HTTPS or no.

    Hope that helps!
    Cheers,
    Paul.

    In the meantime, users can just drop in the SSL Insecure Content Fixer plugin, which handles this automatically for this plugin.

    Hi peeps!

    @webaware… I love that you have a plugin that fixes this. ??

    @paul Goodchild – I LOVE easy fixes like that. I don’t really understand this problem well enough to feel confident fixing or testing it. That said, when I glance at the code, this fix looks a little concerning to me for one reason: We’re already running that filter if the media uploader is being used for media-upload.php / async-upload.php but NOT if it’s used on the widgets actual page. I’m concerned that adding / moving that filter to where you have it might break the uploader. Have you tested that fix?

    @peter Chester: there’s a plugin for everything (and if there isn’t a plugin for something, there soon will be!) ??

    Hi guys, I’ve removed the SSL function from the image widget entirely in the new version. Since I don’t really understand what the issue is, can someone explain to me if we need any SSL function at all and if so, what it should be? Here’s the thread about the upgrade:

    https://www.ads-software.com/support/topic/new-wordpress-35

    Here’s the actual download:
    https://downloads.www.ads-software.com/plugin/image-widget.zip

    And for those who really know what they’re doing:
    https://plugins.svn.www.ads-software.com/image-widget/trunk

    The issue is that when people load a page over SSL, and assets loaded by that page are loaded without SSL, the image isn’t loaded and the web browser warns the user that they cannot trust the page. That could mean lost customers for some websites due to people refusing to enter their credit card details on an otherwise safe SSL-encrypted connection.

    The simple workaround for people using your plugin is to have a different template for pages that require SSL and ditch your widget on that template. Of course, that might mean the site design breaks.

    Another workaround is to install my SSL Insecure Content Fixer plugin, which explicitly handles images loaded by your widget by hooking ‘image_widget_image_url’.

    Better would be for your plugin to have an option to do this itself, saving your users the bother of having to search for solutions, find my plugin or the WordPress HTTPS plugin (which might also fix it, haven’t tested), and installing a plugin to fix a plugin.

    If you’re happy with leaving the fix to my plugin, maybe just add a FAQ so that your users know what they need to do without having to embark on a search. It’s probably the easiest way to handle the issue, considering that you would need to add an option for it and some documentation, whereas anyone who needs the fix can simply install my plugin — if they know about it.

    Oh, and regards the issue of donations (from your “New WordPress 3.5” thread), I know what you mean! I think I’ve had a grand total of 2 donations and 2 sponsorships for additional features, across 6 plugins ?? so it’s lucky we don’t write free plugins for the donations!

    @moderntribe: your new version breaks the ‘image_widget_image_url’ filter, thus disabling the fix my plugin provides. My plugin is called, fixes the url, then your plugin discards it in get_image_html().

    @webaware – excellent point. I see what you mean about the filter being bypassed.

    Honestly, I’d love to have a function in here that seamlessly handles the https problem. I’m more than happy to credit you for it if you want to own that aspect.

    In any case, I’ll fix the filters because I’m a huge advocate for filters and definitely don’t want to release this with broken filters!

    I’m looking at this… Actually, one thing that’s changed is we’re actually phasing out the imageurl var. So that filter is not really all that practical. Instead we’re using WordPress’s built in function ‘wp_get_attachment_image()’ which uses ‘wp_get_attachment_image_src()’ which in turn uses ‘image_downsize()’.

    As such, you should be able to filter core which is probably better to make your code more generalized. I’m not sure what filter is best used in there though.

    Hmmm… after looking through image_downsize(), i don’t se a nice obvious place to lay a URL filter.

    Ugh.

    Not sure what to do here…. I can write this to effectively bypass the WordPress system or we can petition to get a filter added. Bypassing WP will be more reverse compatible and will enable us to continue to support that filter, but will inherently keep this code from being aligned with WP code.

    Thoughts?

    Actually… more broadly, it seems odd that the HTTPS problem wouldn’t already be addressed in the core image code. Any ideas on that?

    Here’s an idea:

    I want to phase out ‘imageurl’ in favor of dynamically getting the image url based on the attachment_id. In the interim, we could lean on the imageurl if it is set and once there is a better filter in place in wordpress we can reverse this and default to attachment id instead.

    What do you think?

    if ( !empty( $instance['imageurl'] ) ) {
    	// If all we have is an image src url we can still render an image.
    	$attr['src'] = $instance['imageurl'];
    	$hwstring = image_hwstring( $instance['width'], $instance['height'] );
    	$output .= rtrim("<img $hwstring");
    	foreach ( $attr as $name => $value ) {
    		$output .= " $name=" . '"' . $value . '"';
    	}
    	$output .= ' />';
    } elseif ( abs( $instance['attachment_id'] ) > 0 ) {
    	$output .= wp_get_attachment_image($instance['attachment_id'], $size, false, $attr);
    }

    G’day,

    Firstly, there is a filter I can use to force dynamically-loaded media images to use SSL: wp_get_attachment_url

    I’ll add that to my plugin, so that will fix this issue, and a few others besides. I hadn’t noticed that WooCommerce was already doing this on a site I built recently that used wp_get_attachment_image() quite a bit, so I thought it was already handling that, but I can now see that my SSL fixer plugin should do this too.

    I’m with you on phasing out the use of image URLs, when the image is stored in the media library. I actually do this for some other plugins (non-free) so that I can easily load the appropriate size of an image selected once, i.e. user picks image once, I display at full, thumb, ‘bottle’, ‘bottlethumb’ etc. as appropriate.

    The only caveat is that you might want to allow external images, not from the media library, and thus using ‘imageurl’.

    Also, please add your old filter to the ‘imageurl’, e.g.

    $attr['src'] = apply_filters('image_widget_image_url', $instance['imageurl'], false, $instance);

    I’ll get a new release of the SSL fixer out today, to fix attachment URLs.

    cheers,
    Ross

    Peter Chester

    (@peterchester)

    @webaware, this should be all kosher in the latest version of the image widget (4.0) that we launched today. Can you confirm?

    Thanks!

Viewing 15 replies - 1 through 15 (of 16 total)
  • The topic ‘[Plugin: Image Widget] SSL on image output’ is closed to new replies.