Notice: Undefined property: stdClass::$player in scald_image_scald_prerender [#2041153]

(line 191 of .../modules/scald/modules/providers/scald_image/scald_image.module).

Scald image player (html5) fails to render image if $atom->rendered->player is not set;

The “visible” effect is that the dnd widget does not show if it would contain a flicker image, qtip is enabled, and sdl_preview is configured to use the html5 player (so the above notice is only visible in the watchdog).

(The notice doesn't fire at scald-d7.ows.fr, as it uses the “default” player).

I did not file this issue in scald_flicker issue queue, as I could not find any documentation stating that the atom provider must set this property; BTW, scald_gallery recently completely dropped his prerender function.

Furthermore, there is a comment in scald.module :

* Basically, a player prepares the $atom->rendered->player in
* hook_scald_prerender() where $mode == 'player'. That value is then used to
* render the atom by the context provider module.

scald_image module fails twice, as it does important processing on $atom->rendered->player when mode = "atom", which would be overwritten by any player conforming the above statement.

I'm marking this issue as major, as tasks must be clearly (re)defined between providers, transcoders, and players...

BTW, scald_image player process, in scald 1.0 release, was made from $atom->rendered->file_transcoded_url, which makes more sense...

Comment	File	Size	Author
#7	scald-2041153-image-prerender-7.patch	597 bytes	jcisio
#7
#4	scald-2041153-image-prerender-4.patch	2.42 KB	jcisio
#4

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

gifad CreditAttribution: gifad commented 12 July 2013 at 23:24

A temporary workaround, for flicker images, may be to insert in scald_image.module :

    if (!isset($atom->rendered->player)) // flicker, for instance
      scald_image_scald_prerender($atom, $context, $options, "atom");

before line 189

This is a workaround, not a fix !

Comment #2

jcisio CreditAttribution: jcisio commented 14 July 2013 at 22:59

Believe or not, scald_image is a provider with many exceptions (thumbnail is used as the atom/base_id, hack for Picture integration, HTML5 player on the top of the "rendering system" which itself is another "player") and is not good enough to take as an example.

Usually the transcoder prepares URIs, then the player use it to prepare $atom->rendered->player. However, player likes HTML5 figure is just a "wrapper" (and it should be able to play virtually anything if the atom provider/transcoder prepare the markup). I don't have time to look at the code right now, but I guess it is neither Scald nor Scald Image, just a problem with the HTML5 player or Scald Flickr.

Comment #3

jcisio CreditAttribution: jcisio commented 14 July 2013 at 23:09

Well, I've however just looked the code. There are multiple problems (but not all of them cause this PHP notice) with:
- Scald Flickr does not return the full remote image URL (the downloaded image maybe "full" image, but it should not be).
- Scald Image transcoder does not know to transcode remote image (it does not set $atom->rendered->file_transcoded_url).
- The last problem, which cause the notice, is that the HTML5 player does not use $atom->rendered->file_transcoded_url (or at least $atom->file_source) as a fallback when $atom->rendered->player is not available.

Comment #4

jcisio CreditAttribution: jcisio commented 15 July 2013 at 21:05

Status:

Active

» Needs review

File	Size
scald-2041153-image-prerender-4.patch	2.42 KB

This patch works for all image providers. However I haven't tested at large: hook invocation reordering, even logically, may cause unexpected error.

Comment #5

jcisio CreditAttribution: jcisio commented 15 July 2013 at 21:20

Status:

Needs review

» Fixed

All tests passed locally, thus I committed patch #4.

Comment #6

gifad CreditAttribution: gifad commented 16 July 2013 at 13:38

This part of the fix

+ module_invoke($atom->provider, 'scald_prerender', $atom, $context, $options, 'atom');
+ module_invoke($types[$atom->type]->provider, 'scald_prerender', $atom, $context, $options, 'type');

(in that order) has the resulting effect to cancel any setting of $atom->rendered->player made any image provider, including scald_image.module !!!
Hope this is a typo...

Comment #7

jcisio CreditAttribution: jcisio commented 16 July 2013 at 13:47

Status:

Fixed

» Needs review

File	Size
scald-2041153-image-prerender-7.patch	597 bytes

Indeed. It breaks Picture integration, which was not caught by the test. Let's if this one passes.

The order is logic, because at the type provider (of every image atom provider), every atom has a file_transcoded_url then the image type provider renders it with image tag.

Comment #8

jcisio CreditAttribution: jcisio commented 16 July 2013 at 14:03

Status:

Needs review

» Fixed

It passed. Nice. Committed #7.

Comment #9

gifad CreditAttribution: gifad commented 16 July 2013 at 22:25

It passed. Nice.

Nicer, it works...
BTW, I discovered the last bug when trying to solve some of your problems :

There are multiple problems (but not all of them cause this PHP notice) with:
- Scald Flickr does not return the full remote image URL (the downloaded image maybe "full" image, but it should not be).
- Scald Image transcoder does not know to transcode remote image (it does not set $atom->rendered->file_transcoded_url).

1 - I managed to save the remote image URL in atom->data, and made a scald_flicked_prerender to insert it in the img tag;
2 - "transcoding" is just done by widh attribute for now;

I spent a couple of hours wandering why the URL was pointing to my home page instead of Filcker farm!