kylereicks / picturefill.js.wp

A WordPress plugin to use Picturefill.js for image loading.
106 stars 10 forks source link

srcset and sizes not being output after migration #43

Closed Ramblurr closed 10 years ago

Ramblurr commented 10 years ago

I did a migration from my dev to production server, which involved:

  1. Databse export -> import
  2. Change of Site urls
  3. Pruned un-attached images
  4. Regenerated thumbnails

However now picturefill.js.wp isn't working on pre-existing posts. If I upload a new attachment, it works. srcset and sizes just simply aren't output.

I've narrowed down the problem to picturefill.js.wp-master/inc/templates/image-template.php

if(false !== $view->image_attributes['attachment_id']){
    echo $view->get_sizes();
    echo ' srcset="' . $view->format_srcset($template_data) . '"';
  }

That if-block is never returning true, an so its never being entered.

Any idea what's up?

kylereicks commented 10 years ago

I've been using attachment id as a way to tell if the image is in the site database or from an external url. The problem in this case seems to be that id is not being returned for images that should have ids. The plugin gets the id by matching the url against the posts in the database (images are stored as posts). The method is url_to_attachment_id in class-model-image-picturefill-wp.php

    private function url_to_attachment_id($image_url){
      $original_image_url = $image_url;
      $image_url = preg_replace('/^(.+?)(-\d+x\d+)?\.(' . implode('|', $this->application_model->get_allowed_image_extensions()) . ')((?:\?|#).+)?$/i', '$1.$3', $image_url);
      $prefix = Picturefill_WP::$wpdb->prefix;
      $attachment_id = Picturefill_WP::$wpdb->get_col(Picturefill_WP::$wpdb->prepare("SELECT ID FROM " . $prefix . "posts" . " WHERE guid='%s';", $image_url ));
      if(!empty($attachment_id)){
        return $attachment_id[0];
      }else{
        $attachment_id = Picturefill_WP::$wpdb->get_col(Picturefill_WP::$wpdb->prepare("SELECT ID FROM " . $prefix . "posts" . " WHERE guid='%s';", $original_image_url ));
      }
      return !empty($attachment_id) ? $attachment_id[0] : false;
    }

It could be that there is a problem with the RegEx, but I would think that the problem would also arise in development. My best guess is that the urls of the images on the posts and pages, do not match the urls for the images as saved in the database. You will want to compare the "guid" column of the posts table for two images, one previously existing and one newly uploaded. I wonder if the older images will still have the url corresponding to the development server. Image urls are stored as absolute urls, and wouldn't be updated unless you've taken specific steps to do so.

And just to make sure, the images without srcset and sizes still output the original image src as a fallback, correct?

As always, thanks for taking the time to post your feedback.

Ramblurr commented 10 years ago

Alright now I'm completely stymied.

First, I take back what I said new attachments working, they in fact aren't. That was a mistake on my part.

Digging into url_to_attachment_id(), I found that the SELECT query was returning no results, like you predicted. The strange thing is if I change the code to:

      $query = Picturefill_WP::$wpdb->prepare("SELECT ID FROM " . $prefix . "posts" . " WHERE guid='%s';", $image_url );
      $attachment_id = Picturefill_WP::$wpdb->get_col($query);
      echo $query;

Then copy and paste the exact prepared query into my own mysql shell, I get a result:

mysql> SELECT ID FROM wp_posts WHERE guid='http://example.com/wp-content/uploads/2014/07/test.jpg';
+-----+
| ID  |
+-----+
| 699 |
+-----+

The query ran in the plugin code returns nothing, yet if I hardcode the query (by copy/pasting it into get_col(), it works! :confounded:

I have done an UPDATE on all guids in wp_posts to ensure they have the same siteurl as my prod server.

And just to make sure, the images without srcset and sizes still output the original image src as a fallback, correct?

Yea, this works, so at least an image is showing :)

Ramblurr commented 10 years ago

Alrighty, I've figured out what's causing the problem. The issue, ass it is with most wordpress issues, is a conflict of plugins.

I'm using the Wordpress HTTPS plugin to put my admin site on a separate subdomain which I have an SSL cert for (secure.example.com ). The main site is HTTP only. So when I'm editing posts and I insert media, it is inserted as https://secure.example.com/wp-content/uploads/2014/07/text.jpg, yet when the HTTPS plugin does its magic, its converted to http://example.com/wp-content/.....

While editing if I change the img src tag to use http://example.com instead of https://secure.example.com, then picturefill works as expected.

It turns out that Wordpress HTTPS is doing a mass search-replace of https://secure.example.com -> http://example.com before the page is sent over the wire, and it is doing this after picturefill's filters run. I'm a noob when it comes to WP's API and internals, so I'm not sure exactly where that search/replace is taking place, but it is.

Anyways, now that the problem is identified. I guess I could just always edit the included media to not use the secure url, but that is a small PITA. Do you think its possible to make them play nice with eachother?

Thanks for taking the time to look over this issue, I realize its not a bug in the plugin perse.

kylereicks commented 10 years ago

We can make the picturefill filter run earlier or later with the picturefill_wp_the_content_filter_priority filter. By default it is set to 11, right after wpautop runs. Try returning a high or low number to see if that makes any difference. I'll see about adding a few filters to try to make things more flexible on picturefill's end.

add_filter('picturefill_wp_the_content_filter_priority', 'new_priority');
function new_priority($old_priority){
//  return 1;
  return 1000;
}
kylereicks commented 10 years ago

I added a few filters that should make this kind of thing easier. With picturefill_wp_attachment_id_search_url we can filter the url as it is used to search for the attachment id. Here we are able to do a search and replace of the url string to get the correct id. Then, if needed, we can do a similar search and replace on the output url strings with the picturefill_wp_srcset_url filter and picturefill_wp_image_attribute_src filter.

Ramblurr commented 10 years ago

How would I go about using one of these filters?

kylereicks commented 10 years ago

In all three cases they filter image urls. picturefill_wp_attachment_id_search_url filters the original image url as it is being used to search for the attachment id, but it does not make any persistent changes to the image src data. picturefill_wp_srcset_url filters each of the urls in the srcset as they are being output to the view. picturefill_wp_image_attribute_src filters the output of the src attribute, if the src attribute is set to be included.

In all cases they can be edited like this:

add_filter('picturefill_wp_attachment_id_search_url', 'search_replace_url_secure_to_unsecure');

function search_replace_url_secure_to_unsecure($url){
  return str_replace('https://secure.example.com', 'http://example.com', $url);
}

This example replaces the secured url with the unsecured url so that we get the right attachment id (fingers crossed). You then may want to do the reverse for the src and srcset attributes.

add_filter('picturefill_wp_image_attribute_src', 'search_replace_url_unsecure_to_secure');
add_filter('picturefill_wp_srcset_url', 'search_replace_url_unsecure_to_secure');

function search_replace_url_unsecure_to_secure($url){
  return str_replace('http://example.com', 'https://secure.example.com', $url);
}

If I'm understanding the issue correctly, I think that should work things out.

Let me know how it goes.

Ramblurr commented 10 years ago

Gotcha, this works perfect! I ended up just using https sitewide, so your snippets worked without modification. Otherwise, since before the 'secure' subdomain was only for logged in admins, I might have had to wrap the str_replace in a is_ssl() or is_admin().

Here's my site by the way, if you want to see your handiwork in action: https://onvagrancy.com

kylereicks commented 10 years ago

I'm happy that everything is working. Thanks for the feedback.