WordPress.org

Make WordPress Core

Opened 18 months ago

Last modified 8 months ago

#45982 new defect (bug)

PDF thumbnails have a default black background

Reported by: nebrekab Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 5.0.3
Component: Media Keywords:
Focuses: Cc:

Description

I wouldn't necessarily call this a bug, but when a PDF is uploaded where the page has no solid background colour its thumbnail image background is rendered black. As most paper used is white ( I would hazard a guess... :) ), it feels like the default background colour of thumbnails should be set to white. I have created a fix within /wp-includes/class-wp-image-editor-imagick.php

<?php
try {
                        $this->image = new Imagick();
                        $file_extension = strtolower( pathinfo( $this->file, PATHINFO_EXTENSION ) );
                        $filename = $this->file;

                        if ( 'pdf' == $file_extension ) {
                                $filename = $this->pdf_setup();
                        }

                        // Reading image after Imagick instantiation because `setResolution`
                        // only applies correctly before the image is read.
                        $this->image->readImage( $filename );

                        if ( ! $this->image->valid() )
                                return new WP_Error( 'invalid_image', __('File is not an image.'), $this->file);

                        // Select the first frame to handle animated images properly
                        if ( is_callable( array( $this->image, 'setIteratorIndex' ) ) )
                                $this->image->setIteratorIndex(0);

                        $this->mime_type = $this->get_mime_type( $this->image->getImageFormat() );
                        
                        // !bb modification to default to white backgrounds

                        if ( 'pdf' == $file_extension ) {
                                
                                if ($this->image->getImageAlphaChannel()) {

                                    // Remove alpha channel (use 11 if constant does not work... see https://stackoverflow.com/questions/10805122/imagemagick-setimagealphachannel-not-working-php)
                                    $this->image->setImageAlphaChannel(Imagick::ALPHACHANNEL_REMOVE);
                                
                                    // set image background color to white
                                    $this->image->setImageBackgroundColor('#ffffff');
                                }
                        }// !bb end modification to default to white backgrounds
                        
                        
                }

Attachments (1)

image-library-load-filters.diff (2.0 KB) - added by joostdekeijzer 8 months ago.
Hi, I have the same issue. The workaround from #39216 does not work for me, I need to setImageAlphaChannel. I'm using PHP7.2 with ImageMagick 6.9.10 and ghostscript 9.5. I've attached a patch that adds filters to both the Imagick class and the GD class both passing the $this->image object as a reference. Then, in a plugin, you can make the fix that works for you. Would that be a solution?

Download all attachments as: .zip

Change History (9)

#1 follow-up: @subrataemfluence
18 months ago

Welcome to trac and thanks for the ticket!

My personal opinion is using hard coded specific background color is not a good idea. Different website can have different backgrounds. If a black background hits eyes, then a white background on another color can do the same.

However, if we can somehow make this background matched with that of site's body color dynamically, that probably makes more sense!

#2 in reply to: ↑ 1 @nebrekab
18 months ago

Thanks for the swift reply!

Yes, it sure doesn't to be hardcoded - the default could be a constant or set via a function setting? - but generally I see it as more to do with the colour of paper being more likely to be white than any other colour, certainly not black, and that it's not so much to do with the colour of the website's design. Somewhere in the code (Imagick perhaps?) they have made the decision to default to black, and I just feel as though that's not the most logical choice... :)

If it could be set in some way shape or form in WordPress - that would be amazing. How do we go about getting this in place? :)

Replying to subrataemfluence:

Welcome to trac and thanks for the ticket!

My personal opinion is using hard coded specific background color is not a good idea. Different website can have different backgrounds. If a black background hits eyes, then a white background on another color can do the same.

However, if we can somehow make this background matched with that of site's body color dynamically, that probably makes more sense!

Version 0, edited 18 months ago by nebrekab (next)

#3 @swissspidy
17 months ago

#46318 was marked as a duplicate.

#4 follow-up: @rdiazg
17 months ago

Hello, I reported the same issue and in my test I fixed it without define any colour by default instead of this I merge all layers with constant LAYERMETHOD_FLATTEN, could be this a good solution?

<?php

$ this -> image -> mergeImageLayers ( Imagick :: LAYERMETHOD_FLATTEN );

#5 in reply to: ↑ 4 @mikenolan
16 months ago

Replying to rdiazg:

Hello, I reported the same issue and in my test I fixed it without define any colour by default instead of this I merge all layers with constant LAYERMETHOD_FLATTEN, could be this a good solution?

<?php

$ this -> image -> mergeImageLayers ( Imagick :: LAYERMETHOD_FLATTEN );

We have a legacy thumbnail generation script for uploaded PDFs and this is the approach we use on the command line (with a -flatten parameter on mogrify). It's been working well for several years but it would be nice to be able to switch to native PDF thumbnails rather than fill the media library with additional images. Probably makes sense to put it in the pdf_setup() function at the bottom of the class.

This ticket was mentioned in Slack in #core by radigar. View the logs.


13 months ago

#7 @madejackson
10 months ago

For anyone who has this issue in the future, the proposed solutions in this thread didn't work for me.

Instead, I used this workaround, which works perfectly: #39216

@joostdekeijzer
8 months ago

Hi, I have the same issue. The workaround from #39216 does not work for me, I need to setImageAlphaChannel. I'm using PHP7.2 with ImageMagick 6.9.10 and ghostscript 9.5. I've attached a patch that adds filters to both the Imagick class and the GD class both passing the $this->image object as a reference. Then, in a plugin, you can make the fix that works for you. Would that be a solution?

#8 @joostdekeijzer
8 months ago

With my patch (see comment above) and below code my pdf's are without black backgrounds:

<?php
    add_action( 'imagick_load_after_read', function( $image, $filename, $file_extension ) {
      if ( 'pdf' === $file_extension ) {
        $image->setImageAlphaChannel( Imagick::ALPHACHANNEL_REMOVE );
      }
    }, 10, 3 );

Note: See TracTickets for help on using tickets.