fisharebest / webtrees

Online genealogy
https://webtrees.net
GNU General Public License v3.0
462 stars 299 forks source link

User generated thumbnails broke page layout on Webtrees theme #1189

Closed UksusoFF closed 7 years ago

UksusoFF commented 7 years ago

Thumbnail or #indi_mainimage must contain max-width and max-height.

makitso commented 7 years ago

Can you provide a link that shows the problem?

UksusoFF commented 7 years ago

Please see attachment. I can send login/pass to email if needed.

sample_1 sample_2 sample_3

UksusoFF commented 7 years ago

Small additions: It's custom theme which extend WebtreesTheme. I change to only Webtrees and same here.

It's custom thumbnail uploaded with media file. Maybe they also must be cropped with getPreference('THUMBNAIL_WIDTH')

vytux-com commented 7 years ago

It looks like you uploaded the same size image to thumbnail folder. If you let webtrees create the thumbnail images they will be the correct size or you must create thumbnails yourself but you must make sure the size is correct in that case.

UksusoFF commented 7 years ago

you must make sure the size is correct in that case.

Ouch, why?

I see this code in Media@getImageAttributes():

if (($which == 'thumb') && ($imgsize['0'] > $THUMBNAIL_WIDTH)) {
    // don’t let large images break the dislay
    $imgsize['imgWH'] = ' width="' . $THUMBNAIL_WIDTH . '" ';
}

So that sometimes thumbnail render with getPreference('THUMBNAIL_WIDTH') and sometimes not?

UksusoFF commented 7 years ago
   public function hookHeaderExtraContent() {
        return parent::hookHeaderExtraContent() .
         '<style type="text/css">
            [src*="thumb=1"] {
               width: '.$this->tree->getPreference('THUMBNAIL_WIDTH').'px;
               height: auto;
            }
         </style>';
    }

I can fix this for custom theme by this dirty css but maybe Media class must consider tree thumbnail settings in all render functions?

vytux-com commented 7 years ago

Yes you could always use CSS to fix user mistakes, however if you have a 23 megapixel image as a thumbnail the the browser would have to download and process and then resize the image to a small thumbnail. If we were to do that might as well not have any thumbnails to start with.

The way the site works currently you will see the big images and will be able.to fix the error by replacing the thumbnail with a correctly resized one

UksusoFF commented 7 years ago

Why Webtrees not crop thumbnail when it's uploaded?

vytux-com commented 7 years ago

We automatically create thumbnails if the are not uploaded ... If the user does upload one we presume that they are uploading what is required by them.

Most users will not upload the thumbnails unless the site is not able to generate or they would like a special thumbnail for that image.

fisharebest commented 7 years ago

FYI, I am planning to remove the "user-generated-thumbnail" feature.

We need thumbnails in many different sizes. For example, the individual page, the media page, facts/events, person boxes, etc.

Now that webtrees has dropped support for older PHP versions, we can use nice image libraries, such as http://glide.thephpleague.com/

vytux-com commented 7 years ago

Cool, the only time I used user generated thumbnails was for PDF files...

UksusoFF commented 7 years ago

We automatically create thumbnails if the are not uploaded ... If the user does upload one we presume that they are uploading what is required by them. Most users will not upload the thumbnails unless the site is not able to generate or they would like a special thumbnail for that image.

Yes, i understand. But expectation that they will be automatically cropped for needed sizes. Especially that behavior not described at hint:

Choose the thumbnail image that you want to upload. Although thumbnails can be generated automatically for images, you may wish to generate your own thumbnail, especially for other media types. For example, you can provide a still image from a video, or a photograph of the individual who made an audio recording.

Maybe need add some text, like: Image thumbnail must be %THUMBNAIL_WIDTH% pixels in width.

But it's really doesn't matter if:

FYI, I am planning to remove the "user-generated-thumbnail" feature.

rorrison commented 7 years ago

FYI, I am planning to remove the "user-generated-thumbnail" feature.

(user delurking to comment)

I'd find that disappointing - I often use my own thumbnails, when I want the thumbnail to focus on a particular part of the image. For example, the primary image for my dad, http://orrison.com/genealogy/individual.php?pid=I1&ged=COMPLETE.GED - and quite a few more of the images in the album on his page. If there was a way after uploading to select a square region of the image to use as a thumbnail, that would be great.

fisharebest commented 7 years ago

@rorrison

If you create a new (cropped) image from the main image, then webtrees will be able to create different sized versions of the cropped image.

I already do this in my own tree - e.g. create individual "portrait" images from a group photo.

Also, the cropped images will be part of the GEDCOM and would be recognised by other software. The current thumbnail system is entirely non-portable.

UksusoFF commented 7 years ago

I already do this in my own tree - e.g. create individual "portrait" images from a group photo.

How it's look? New version support multiple thumbnails for one photo per person?

It will be nice select thumbnail like this: http://deepliquid.com/projects/Jcrop/demos.php?demo=thumbnail

fisharebest commented 7 years ago

FYI, I am planning to remove the "user-generated-thumbnail" feature.

This change has now been made.

webtrees will generate thumbnails with different sizes and aspect-ratios for each context.

It will also generate high-resolution images for devices with high resolution displays.

FYI, I am planning to add support for media objects that contain more than one media file.

This will allow a GEDCOM compatible way to store images equivalents for audio/video/pdf/etc.