17

I am currently in the course of migrating site content from an old pre 4.1 site to a new setup and hitting an issue with the rounding error issue of #18532 and the corresponding fix.

To summarize this fixed a long standing rounding misbehaviour on the side of WordPress:

Imagine we upload an image with 693x173 and scale it to a width of 300:

  • pre 4.1: 300x74
  • post 4.1: 300x75

The issue

Generally this doesn't cause any issues because existing files and <img> aren't touched.

But when you regenerating thumbs or importing attachments from a WXR file they get generated differently in the filesystem leaving all <img> in post_content dead.

Looking for a solution

I have been thinking of various solutions:

Going back to the bad old times

Changeset 30660 introduced a new filter wp_constrain_dimensions which can be used to just plug the old behaviour from before 4.1 back in. This does fix the issue. But I am wondering if this might cause issues later on and generally I'd like to have the fix so although this works I'd deem it non-ideal.

The Times They Are a-Changin'

So this leaves us with another goal: Clean up the DB and replace all references to the old files with references to the new files. The question I am actually asking here now is how to do this. I am looking for an effective and generally-applicable solution as I suspect this issue does and will affect a lot of people

My current idea is this:

  1. Import, regenerate or whatever which leaves us with the new files and broken tags.
  2. Create a list A from all resized files in the filesystem or alternatively getting this information from the database
  3. Parse this list and create a second list B with filenames all offset by one pixel as it would look before 4.1
  4. Do a search&replace over the whole database replacing all occurences of B with the relating entry in A

I am just not sure if this is the most smart and efficient way to handle this situation. It also feels a bit too brute-force. So before implementing it I just wanted to check with the infinite wisdom of the WPSE crowd ;)

[edit] Having read ck-macleods answer (thanks!) I think a fix should solve this once and for all so you do not need to constantly keep this issue in the back of your head. [/edit]

[edit2] I just found a related ticket on Trac. Adding for reference. [/edit2]

2
  • where you have error issue of #13852 did you mean #18532? :)
    – Aravona
    Commented Sep 14, 2015 at 9:41
  • 2
    Oops, yes, fixed. :)
    – kraftner
    Commented Sep 14, 2015 at 9:59

3 Answers 3

4

This is another approach than the other answer that works when importing content with the importer and fixes the URLs once and for all. Again: This is not battle-tested but is the solution I settled on and it did work.

I prefer this as it solves the issue once and for all and if it works, it works. As you are not leaving broken stuff in the DB and fix it on display you do not need to worry about stuff breaking later on.

/*
Plugin Name:  Bugfix Ticket 31581 for WP_Importer
Plugin URI:   https://wordpress.stackexchange.com/a/206992/47733
Description:  Fixes image references after post WP 4.1 image scaling change in post content when using the WP_Importer  (see https://core.trac.wordpress.org/ticket/31581)
Version:      0.0.1
*/

class Bugfix31581WPImporter {

    protected $remaps;

    /**
     * Initialize class, mainly setting up hooks
     */
    public function init(){

        if (WP_IMPORTING === true){

            $this->remaps = array();

            //This hook is chosen because it is pretty close to where the actual attachment import is happening.
            //TODO: May be reconsidered.
            add_filter('wp_update_attachment_metadata', array($this, 'collectRemaps'), 10 , 2);

            add_action('import_end', array($this, 'remap'));
            add_action('import_end', array($this, 'importEnded'));
        }

    }

    /**
     * Cleans up hooks after the import has ended.
     */
    public function importEnded(){
        remove_filter('wp_update_attachment_metadata', array($this, 'collectRemaps'), 10);

        remove_action('import_end', array($this, 'remap'), 10);
        remove_action('import_end', array($this, 'importEnded'), 10);
    }

    /**
     * When an attachment is added compare the resulting sizes with the sizes from the legacy algorithm and setup remap.
     *
     * @param $data
     * @param $post_id
     *
     * @return mixed
     */
    public function collectRemaps($data, $post_id ){

        $intermediate_sizes = $this->getIntermediateSizes();

        if(empty($data) || !array_key_exists('sizes', $data)){
            return $data;
        }

        foreach($data['sizes'] as $key => $size){

            $size_new_algorithm = array($size['width'], $size['height']);

            $dest_w = $intermediate_sizes[$key]['width'];
            $dest_h = $intermediate_sizes[$key]['height'];
            $crop = $intermediate_sizes[$key]['crop'];

            add_filter('wp_constrain_dimensions', array($this, 'legacy_wp_constrain_dimensions'), 10, 5);

            $size_old_algorithm = image_resize_dimensions($data['width'], $data['height'], $dest_w, $dest_h, $crop);

            //Bail out in the rare case of `image_resize_dimensions` returning false
            if($size_old_algorithm===false){
                continue;
            }

            $size_old_algorithm = array($size_old_algorithm[4], $size_old_algorithm[5]);

            remove_filter('wp_constrain_dimensions', array($this, 'legacy_wp_constrain_dimensions'), 10);

            // Compare the current size with the calculation of the old algorithm...
            $diff = array_diff($size_new_algorithm, $size_old_algorithm);

            // ...to detect any mismatches
            if(!empty($diff)){

                $oldFilename = $this->getOldFilename($size['file'], $size_old_algorithm);

                // If getting the old filename didn't work for some reason (probably other filename-structure) bail out.
                if($oldFilename===false){
                    continue;
                }

                if(!array_key_exists($post_id, $this->remaps)){
                    $this->remaps[$post_id] = array();
                }

                $this->remaps[$post_id][$size['file']] = array(
                    'file' => $oldFilename,
                    'size' => $key
                );
            }

        }

        return $data;
    }

    /**
     * Get resize settings for all image sizes
     *
     * Taken from wp_generate_attachment_metadata() in includes/image.php
     *
     * @return array
     */
    public function getIntermediateSizes(){

        global $_wp_additional_image_sizes;

        $sizes = array();
        foreach ( get_intermediate_image_sizes() as $s ) {
            $sizes[$s] = array( 'width' => '', 'height' => '', 'crop' => false );
            if ( isset( $_wp_additional_image_sizes[$s]['width'] ) )
                $sizes[$s]['width'] = intval( $_wp_additional_image_sizes[$s]['width'] ); // For theme-added sizes
            else
                $sizes[$s]['width'] = get_option( "{$s}_size_w" ); // For default sizes set in options
            if ( isset( $_wp_additional_image_sizes[$s]['height'] ) )
                $sizes[$s]['height'] = intval( $_wp_additional_image_sizes[$s]['height'] ); // For theme-added sizes
            else
                $sizes[$s]['height'] = get_option( "{$s}_size_h" ); // For default sizes set in options
            if ( isset( $_wp_additional_image_sizes[$s]['crop'] ) )
                $sizes[$s]['crop'] = $_wp_additional_image_sizes[$s]['crop']; // For theme-added sizes
            else
                $sizes[$s]['crop'] = get_option( "{$s}_crop" ); // For default sizes set in options
        }

        return $sizes;
    }

    /**
     * Turn the new filename into the old filename reducing the height by one
     *
     * @param $newFilename
     * @param $size
     *
     * @return mixed
     */
    public function getOldFilename($newFilename, $size){

        $dimensions = array();

        $filetypes = $this->getAllowedImageExtentions();

        // TODO: This pattern can be different. See `image_make_intermediate_size` in image editor implementation.
        $matchFiles = '/([0-9]{1,5})x([0-9]{1,5}).(' . $filetypes . ')$/';

        // Extract the dimensions
        preg_match($matchFiles,$newFilename,$dimensions);

        // If the file URL doesn't allow guessing the dimensions bail out.
        if(empty($dimensions)){
            return $newFilename;
        }

        $newStub = $dimensions[1] . 'x' . $dimensions[2] . '.' . $dimensions[3];

        $oldStub = $size[0] . 'x' . $size[1] . '.' . $dimensions[3];

        $oldFilename = str_replace($newStub,$oldStub,$newFilename);

        return $oldFilename;
    }

    /**
     * Extract all file extensions that match an image/* mime type
     *
     * @return string
     */
    protected function getAllowedImageExtentions(){
        $allowed_filetypes = get_allowed_mime_types();

        $allowed_images = array();

        foreach($allowed_filetypes as $extensions => $mimetype){
            if( substr($mimetype,0,6) == 'image/' ){
                $allowed_images[] = $extensions;
            }
        }

        return implode('|',$allowed_images);
    }


    /**
     * This is the heart of this class. Based on the collected remaps from earlier it does a S&R on the DB.
     */
    public function remap(){

        global $wpdb;

        foreach($this->remaps as $attachment_id => $replaces){

            foreach($replaces as $new_url => $old_data){

                $to_url = wp_get_attachment_image_src($attachment_id,$old_data['size']);
                $to_url = $to_url[0];

                $from_url = str_replace($new_url, $old_data['file'], $to_url);

                // remap urls in post_content
                $wpdb->query( $wpdb->prepare("UPDATE {$wpdb->posts} SET post_content = REPLACE(post_content, %s, %s)", $from_url, $to_url) );

                //TODO: This is disabled as enclosures can't be images, right?
                // remap enclosure urls
                //$result = $wpdb->query( $wpdb->prepare("UPDATE {$wpdb->postmeta} SET meta_value = REPLACE(meta_value, %s, %s) WHERE meta_key='enclosure'", $from_url, $to_url) );

            }

        }

    }

    /**
     * This is a copy of the legacy pre 4.1 wp_constrain_dimensions()
     *
     * @param $dimensions
     * @param $current_width
     * @param $current_height
     * @param $max_width
     * @param $max_height
     *
     * @return array
     */
    public function legacy_wp_constrain_dimensions($dimensions, $current_width, $current_height, $max_width, $max_height){
        if ( !$max_width and !$max_height )
            return array( $current_width, $current_height );

        $width_ratio = $height_ratio = 1.0;
        $did_width = $did_height = false;

        if ( $max_width > 0 && $current_width > 0 && $current_width > $max_width ) {
            $width_ratio = $max_width / $current_width;
            $did_width = true;
        }

        if ( $max_height > 0 && $current_height > 0 && $current_height > $max_height ) {
            $height_ratio = $max_height / $current_height;
            $did_height = true;
        }

        // Calculate the larger/smaller ratios
        $smaller_ratio = min( $width_ratio, $height_ratio );
        $larger_ratio  = max( $width_ratio, $height_ratio );

        if ( intval( $current_width * $larger_ratio ) > $max_width || intval( $current_height * $larger_ratio ) > $max_height )
            // The larger ratio is too big. It would result in an overflow.
            $ratio = $smaller_ratio;
        else
            // The larger ratio fits, and is likely to be a more "snug" fit.
            $ratio = $larger_ratio;

        // Very small dimensions may result in 0, 1 should be the minimum.
        $w = max ( 1, intval( $current_width  * $ratio ) );
        $h = max ( 1, intval( $current_height * $ratio ) );

        // Sometimes, due to rounding, we'll end up with a result like this: 465x700 in a 177x177 box is 117x176... a pixel short
        // We also have issues with recursive calls resulting in an ever-changing result. Constraining to the result of a constraint should yield the original result.
        // Thus we look for dimensions that are one pixel shy of the max value and bump them up
        if ( $did_width && $w == $max_width - 1 )
            $w = $max_width; // Round it up
        if ( $did_height && $h == $max_height - 1 )
            $h = $max_height; // Round it up

        return array( $w, $h );
    }

}

add_filter('import_start',array(new Bugfix31581WPImporter(),'init'));
1
  • Nice work an +1.
    – gmazzap
    Commented Oct 29, 2015 at 10:56
1

Solving the problem globally and perfectly for ALL image files (and links) in a large site - given the possibility, for instance, that individuals may have occasionally renamed image files manually imitating the WP style - and other odd variations - might be difficult. Database search and replace operations are also going involve complications (and risks!).

Could you handle the vast majority of errors - broken images and broken image links, I presume - and achieve the desired end result or reasonable facsimile, by the following method?

  1. Identify the date before which all resized images where resized by the old "intval" method rather than the new "round" method. (A different kind of cut-off could also be created, but date seems easiest.)

  2. For all posts published <= the cut-off-date, run preg_replace on the_content() at load/render time, capturing all image files with the problem pattern or patterns and replacing them with the desired pattern. The database would remain unaltered, but the output would be error-free in the most instances. I'm not sure whether the solution would need to apply both to "singular" page post content and to archive pages and other processes as well.

If a solution of this type would be helpful, then the next question would be whether the problem patterns and replacements could be adequately defined. It looks from your list of proposed solutions that possibly a few typical patterns could in fact be isolated (perhaps taken from prior media settings producing thumbnails and some other images).

I have already written a simpler function that I use (and am in the process of turning into a plug-in), that globally replaces all image files in designated directories, up to a certain date, with a default image or image-link, as per the above-described method. It was a for a site where, in an excess of copyright caution, the operators simply deleted all of their images, unaware that, in addition to producing ugly results on old pages, they were also turning out thousands of errors, two each for each image.

If you can narrow down the problem pattern more specifically, and the instances where the output would need to be altered, then I could see about plugging it into my format - which isn't very complicated, and which for a better RegExer than I might even be easy. On the other hand, I wouldn't want to waste your or my time if this approach would not answer the problem for you.

3
  • Thanks for your take on this! Just some thoughts: I think having wrong data in the DB and just monkey-patching it on display isn't a very clean and sustainable solution. It might break any time and harms performance on each view. It also might have unpredictable side effects e.g. for other plugins that parse or alter the content in some other way. Depending on how it is done images are still broken in the backend. In that case I think just resetting the scaling via wp_constrain_dimensions as mentioned in the question while doing the import and refrain from rebuilding thumbs would be cleaner.
    – kraftner
    Commented Sep 17, 2015 at 8:49
  • You're quite welcome. Thing is, the data in the DB isn't wrong data, it's just not the data you want anymore under the new regime. On the performance hit, I think it would likely be minimal, esp since it only applies, in theory, to posts before date X. More generally, there may not be a one-size-fits-all best solution: I think the good enough solution may vary with the character of the site, past image-handling applications and habits, size of the DB, practical and time constraints, and so on.
    – CK MacLeod
    Commented Sep 18, 2015 at 17:01
  • You are probably right that there won't be a one-size-fits-all solution to it. I am currently exploring various ways of handling this among which are a on-render approach similar to yours and an on-import approach which I'd prefer as it solves this once and for all. We'll see where this leads. :)
    – kraftner
    Commented Sep 19, 2015 at 8:49
1

Okay this is a basic approach on replacing broken images on the fly. Be aware that this is more a proof of concept than a battle-tested solution. It just hooks on the the_content filter which might (probably has) some unwanted side-effects in some situations. Handle with care. :)

Although it says so in the code too I also want to credit @Rarst for this answer used in my code.

/*
Plugin Name:  Bugfix 31581 Live
Plugin URI:   https://wordpress.stackexchange.com/a/206986/47733
Description:  Fixes image references in post content after post 4.1 image scaling change (see https://core.trac.wordpress.org/ticket/31581)
Version:      0.0.1
*/

class Bugfix31581Live {

    protected $matchURLs;
    protected $matchFiles;

    protected $remaps;

    public function init(){

        $filetypes = $this->get_allowed_image_extentions();

        $baseurl = wp_upload_dir();
        $baseurl = preg_quote($baseurl['baseurl'], '/');

        $this->matchURLs = '/' . $baseurl . '\/.??([a-zA-Z0-9_-]*?\.(?:' . $filetypes . '))/';

        //TODO: This pattern can be different. See `image_make_intermediate_size` in image editor implementation
        $this->matchFiles = '/([0-9]{1,4})x([0-9]{1,4}).(' . $filetypes . ')$/';

        add_filter('the_content', array($this, 'update_urls') );
    }

    public function update_urls($content){

        $urls = array();

        preg_match_all($this->matchURLs,$content,$urls);

        // Bail out early if we don't have any match.
        if($urls === false || empty($urls[0])){
            return $content;
        }

        // Loop through all matches
        foreach($urls[0] as $url){

            // Try to resolve this URL to an attachment ID
            $id = $this->get_attachment_id($url);

            // If not  let's see if this might be a URL that has been broken by our beloved Changeset 30660
            if( $id === false ){

                $dimensions = array();

                // Extract the dimensions
                preg_match($this->matchFiles,$url,$dimensions);

                // If the file URL doesn't allow guessing the dimensions bail out.
                if(empty($dimensions)){
                    continue;
                }

                // Old filename
                $old = $dimensions[1] . 'x' . $dimensions[2] . '.' . $dimensions[3];

                // New filename (not sure if this exists yet)
                $new = $dimensions[1] . 'x' . ($dimensions[2]+1) . '.' . $dimensions[3];

                // Build the new URL (not sure if this exists yet)
                $new_url = str_replace($old,$new,$url);

                // Try to get the attachment with the new url
                $id = $this->get_attachment_id($new_url);

                // Bad luck. This also doesn't exist.
                if( $id === false ) {
                    continue;
                }

                // Just to be sure everything is in sync we get the URL built from id and size.
                $db_url = wp_get_attachment_image_src($id,array($dimensions[1], $dimensions[2]+1));

                // Check if the URL we created and the one wp_get_attachment_image_src builds are the same.
                if($new_url === $db_url[0]){

                    // Awesome let's replace the broken URL.
                    $content = str_replace($url,$new_url,$content);
                }

            }

        }

        return $content;
    }

    /**
     * Get the Attachment ID for a given image URL.
     *
     * @link   https://wordpress.stackexchange.com/a/7094
     *
     * @param  string $url
     *
     * @return boolean|integer
     */
    protected function get_attachment_id( $url ) {

        $dir = wp_upload_dir();

        // baseurl never has a trailing slash
        if ( false === strpos( $url, $dir['baseurl'] . '/' ) ) {
            // URL points to a place outside of upload directory
            return false;
        }

        $file  = basename( $url );
        $query = array(
            'post_type'  => 'attachment',
            'fields'     => 'ids',
            'meta_query' => array(
                array(
                    'value'   => $file,
                    'compare' => 'LIKE',
                ),
            )
        );

        $query['meta_query'][0]['key'] = '_wp_attached_file';

        // query attachments
        $ids = get_posts( $query );

        if ( ! empty( $ids ) ) {

            foreach ( $ids as $id ) {

                $tmp = wp_get_attachment_image_src( $id, 'full' );

                // first entry of returned array is the URL
                if ( $url === array_shift( $tmp ) )
                    return $id;
            }
        }

        $query['meta_query'][0]['key'] = '_wp_attachment_metadata';

        // query attachments again
        $ids = get_posts( $query );

        if ( empty( $ids) )
            return false;

        foreach ( $ids as $id ) {

            $meta = wp_get_attachment_metadata( $id );

            foreach ( $meta['sizes'] as $size => $values ) {

                $tmp = wp_get_attachment_image_src( $id, $size );

                if ( $values['file'] === $file && $url === array_shift( $tmp ) )
                    return $id;
            }
        }

        return false;
    }

    protected function get_allowed_image_extentions(){
        $allowed_filetypes = get_allowed_mime_types();

        $allowed_images = array();

        foreach($allowed_filetypes as $extensions => $mimetype){
            if( substr($mimetype,0,6) == 'image/' ){
                $allowed_images[] = $extensions;
            }
        }

        return implode('|',$allowed_images);
    }

}

add_filter('init',array(new Bugfix31581Live(),'init'));

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.