sonata-project / SonataMediaBundle

Symfony SonataMediaBundle
https://docs.sonata-project.org/projects/SonataMediaBundle
MIT License
451 stars 495 forks source link

Array to string conversion - SonataMediaBundle:Provider:thumbnail.html.twig #1065

Closed navalex closed 8 years ago

navalex commented 8 years ago

Hi everybody,

I'm trying to add a YouTube video into my gallery with SonataAdmin 3.x-dev and SonataMedia 3.x-dev. When I create the gallery and add the video, all work. But when I try to show the gallery, i get the error:

An exception has been thrown during the rendering of a template ("Notice: Array to string conversion") in SonataMediaBundle:Provider:thumbnail.html.twig at line 12.

But I don't found any same problem on the web.

greg0ire commented 8 years ago

Try to determine if the problem comes from name or value by splitting the line into several lines.

Also, please provide a stack trace.

navalex commented 8 years ago

Ok, I dump the variable options and for a youtube video, it return an array as value for player_parameters:

array(3) {
  ["html5"]=>
  bool(false)
  ["player_url_parameters"]=>
  string(153) "rel=0&autoplay=0&loop=0&enablejsapi=0&disablekb=0&egm=0&border=0&fs=1&start=0&hd=1&showsearch=0&showinfo=0&iv_load_policy=1&cc_load_policy=1&wmode=window"
  ["player_parameters"]=>
  array(6) {
    ["border"]=>
    int(0)
    ["allowFullScreen"]=>
    bool(true)
    ["allowScriptAccess"]=>
    string(6) "always"
    ["wmode"]=>
    string(6) "window"
    ["width"]=>
    int(100)
    ["height"]=>
    int(56)
  }
}

So I try to add a condition iterable for the value:

<img {% for name, value in options %}{{name}}="{% if value is not iterable %}{{value}}{% endif %}" {% endfor %} />

But it not render an image for youtube video.

pachocho84 commented 8 years ago

I'm having the same issue when trying to use sonata media inside CKEditor.

The media list should be filtered by image provider (as shown in the url) but it doesn't, so when the template finds a youtube media gives error.

Any solution? Thanks

hug963 commented 8 years ago

Hi @greg0ire , just had the same problem while updating a website.

to recreate: use the {% thumbnail %} twig helper on a video media (youtube or vimeo)

This was created there: https://github.com/sonata-project/SonataMediaBundle/commit/22b20c5af49026416d5292b39b20527b89638416#diff-c42f7939aab63b9a26e22ab981a45140R172

This change didn't make sense as it brings the 'thumbnail' helper inline with the 'media' helper. These two have different functions, and the 'thumbnail' for a video should be an image not the video's source and options.

This change broke the 'thumbnail' helper for all videos.

greg0ire commented 8 years ago

@elvetemedve , can you help?

hug963 commented 8 years ago

Hey @greg0ire, Would you mind having a look at this? It is the only thing standing between two websites and an update to sonata 3.0... :)

greg0ire commented 8 years ago

This change didn't make sense as it brings the 'thumbnail' helper inline with the 'media' helper.

What exactly do you mean by that ? ping @elvetemedve , can you please advise?

elvetemedve commented 8 years ago

The referenced commit I made was needed to let me pass custom options to the template which renders the image. I think my modification is general enough to be compatible with video thumbnail generation. I guess the bug must be in the template of the video thumbnail or the $provider object.

I don't have time to look at this now. Since it's FOSS software, feel free to contribute. ;)

greg0ire commented 8 years ago

Thanks for answering @elvetemedve !

@hug963 : I don't use the media bundle, so I can hardly help unless you give very much context. Here is what I think I understood / assumed from what you people give me :

  1. @elvetemedve made a modification so that it is up to the "media providers" to populate options.
  2. There is a media provider for youtube and vimeo.
  3. There are also media providers for photos / images.
  4. The same template is used for every provider.
  5. The provider for youtube or vimeo wrongly provides options for a video tag, when it should provide options for an img tag.

Questions I have :

  1. What provider is in use when the bug happens (please provide a github link) ?
  2. What method of this provider is used to provide thumbnails options ? Can you link to it?
  3. Is there any documentation (in rst or in code directly) on this and if yes, can you link to it?
hug963 commented 8 years ago

What exactly do you mean by that ?

With this change the “media” and “thumbnail” helpers return exactly the same thing. https://github.com/sonata-project/SonataMediaBundle/blob/3.x/Twig/Extension/MediaExtension.php#L102 https://github.com/sonata-project/SonataMediaBundle/blob/3.x/Twig/Extension/MediaExtension.php#L149

The thumbnail template is the same for all providers and rightly so as it should always be an image.

This is the use of the “thumbnail” helper: return an image for all providers.

This is why the source was hardcoded. https://github.com/sonata-project/SonataMediaBundle/commit/22b20c5af49026416d5292b39b20527b89638416#diff-c42f7939aab63b9a26e22ab981a45140R172

generatePublicUrl()” is the only method that returns an image for the video providers.

The “media” provider should return the media (a video for a video Provider, and image for an image provider, a file for a file provider, etc).

The “thumbnail” provider should return an image that represents the media no matter what, this change broke that. And by passing video options to an image template created the bug we are discussing.

The “getHelperProperties” method for the video providers clearly return a video and has no way of passing an image back to the template.

@elvetemedve I think you might be using the "thumbnail" helper where you should be using the "media" helper.

greg0ire commented 8 years ago

The “media” provider should return the media (a video for a video Provider, and image for an image provider, a file for a file provider, etc).

The “thumbnail” provider should return an image that represents the media no matter what, this change broke that.

OK so to fix that we should IMO either:

hug963 commented 8 years ago

Do you mean adding a "getThumbnail" method on all providers?

I guess this is what would make the most sense.

it could just return "generatePublicUrl()" in the video providers and "getHelperProperties()" in the image providers, and it would fix this for now.

greg0ire commented 8 years ago

Do you mean adding a "getThumbnail" method on all providers?

On those where it makes sense (maybe it does not for all medias). That would be create a new interface (ThumbnailProviderInterface ?)

Also, it shouldn't return options, but modify them, to be consistent with the "media" provider, right? So addThumbnailOptions() ?

hug963 commented 8 years ago

I don't think we need a new provider, I think just adding a getThumbnail() method on the BaseProvider that returns "generatePublicUrl()"

and overriding it in the ImageProvider to return "getHelperProperties" would be enough?

hug963 commented 8 years ago

Wait, it all exists here already: https://github.com/sonata-project/SonataMediaBundle/tree/3.x/Thumbnail

and it is called here in the Image provider: https://github.com/sonata-project/SonataMediaBundle/blob/3.x/Provider/ImageProvider.php#L128-L137

hug963 commented 8 years ago

I am sorry but this change broke a few things:

  1. For the image provider it returns the whole media instead of the thumbnail and overrides the built-in method
  2. For all the other providers (Video, File) it broke the thumbnail entirely by returning videos or files instead of the thumbnail
  3. you could already do what this does by using the "media" twig helper

The "thumbnail" helper is not flexible for a reason: it doesn't need to be.

greg0ire commented 8 years ago

The "thumbnail" helper is not flexible for a reason: it doesn't need to be.

I think @elvetemedve would disagree with you there :

The referenced commit I made was needed to let me pass custom options to the template which renders the image.

I read the doc and I don't understand why on earth the same method is used to provide options for the thumbnail templates and the media template. It really makes no sense at all to me, my wtf-o-meter is going crazy. @sonata-project/contributors , maybe you can shed a light on this fishy situation? To me, a new method really needs to be introduced and called, maybe getThumbnailHelperProperties, for consistency's sake ? It would call generatePublicUrl(), and maybe also provide a proper alt attribute.