Placeholders not working after update to 3.5.3

[deschler]

After upgrading to version 3.5.3 placeholders don't work for me anymore. FileBrowseFields with empty values just return an empty string rather than the placeholder fb_version.

Relevant filebrowser settings:

FILEBROWSER_DIRECTORY = ''
FILEBROWSER_VERSIONS_BASEDIR = '.v'
FILEBROWSER_VERSIONS = {
    'admin_thumbnail': {
        'verbose_name': ugettext('Admin thumbnail'),
        'width': 60, 'height': 60, 'opts': 'crop upscale'
    },
    'content_image_1c': {
        'verbose_name': ugettext('Content image'),
        'width': 400, 'height': 225, 'opts': 'crop upscale'
    },
}
FILEBROWSER_PLACEHOLDER = 'backgrounds/placeholder.png'
FILEBROWSER_SHOW_PLACEHOLDER = True

[sehmaschine]

yes, that´s strange and I'm able to reproduce the bug … but your patch looks a bit strange as well … it shouldn´t be necessary to create another file object.

[sehmaschine]

I can´t remember why we moved the placeholder functionality out from fb_versions … it seems a better place to check compared with the method version_generate in base.py.

my proposal (line 89 of fb_versions.py):

site = context.get('filebrowser_site', get_default_site())
if FORCE_PLACEHOLDER or (SHOW_PLACEHOLDER and not site.storage.isfile(source)):
    source = PLACEHOLDER
fileobject = FileObject(source, site=site)

[elwinbuisman]

I'm missing a placeholder when using the version_generate method. For example:

def get_thumbnail(self):
    return self.image.version_generate(self.image_size)

Turns out the placeholder is only available when using the template tag from fb_versions.py, according to the proposed fix (in the previous comment. Why is that?

[sehmaschine]

@elwinbuisman not sure why you wanna actually generate an image from a placeholder ... what's the usecase for that? IMO, the placeholder should be used with the frontend while generating images still use the original version.

[elwinbuisman]

@sehmaschine Well, in this case I'm passing the image-urls (with the rest of the document) to elastissearch. The django template will be rendered with those results. (So I can't use a template tag)

Of course its possible to solve it something like this:

def get_thumbnail(self):
    if self.image:
        return self.image.version_generate(self.image_size).url
    else:
        return settings.FILEBROWSER_PLACEHOLDER

My point is: It seems wrong to me, because the template_tag (API) provides different results then the internal API. And the behavior is unexpected because the difference is not documented anywhere. (thus rendering the placeholder setting useless in my case)

[sehmaschine]

I see – that usecase makes sense indeed. That said, I'm not sure if we should prevent versions from being generated just because a placeholder is defined. It might not be documented well enough, but the placeholder has always been a frontend feature from my point of view.

I'm reopening this ticket for further discussions.

[elwinbuisman]

Thanks. I do see the point of your argument of the placeholder being a front-end feature.

Maybe the best solution would be to create a (helper) function to accomplish the same result as the template tag. (or a combination to keep it dry). In that case the separation would be clear.

[sehmaschine]

I have another question concerning your code example: It seems that you use the placeholder if no image is defined. Are we talking about a production setup here? Because the idea of a placeholder is for development only – e.g., if you don't have access to the media files or if you only have partial access. I just wanna understand exactly what you're trying to achive.

[elwinbuisman]

That's correct, I'm using the placeholder in case an image is not defined in production set-up. It is used as a fall back in case an image is not present (for whichever reason).

I thought the use case for the placeholder was just that. Why? Well, because filebrowser allows a user delete files, even when they are used (in, for example, articles), which breaks integrity of the data. (Logically) I assumed the placeholder is a fall back to solve this problem.

However the placeholder (and force_placeholder) setting is (also) perfect for the usecase you provided.