The module does not appear to store the URL given for an iframe, so that when iframe atoms are used they do not have any usable content.

Steps to reproduce:
1. Install scald_iframe module on site.
2. Browse to /atom/add/iframe
3. Enter the URL "http://www.example.com"
4. Click Continue, and then finish
5. Observe PHP Notice: Notice: Undefined property: stdClass::$iframe_url in scald_iframe_scald_prerender() and that an atom is now being displayed, but it is empty. Observe that the html generated for it is:

    <!-- scald=5:full --><div class="iframe-wrapper" style="padding-bottom: 38.829787234043%;">
  <iframe seamless="seamless" width="940" height="365" src="" frameborder="0"></iframe>
</div>
<!-- END scald=5 -->

We note that in the code an attempt is made to set the atom's field 'scald_url' to the entered URL value, but that at no point was that field defined for scald_iframe atoms.

Is this module depending on another module to define that field, perhaps?

CommentFileSizeAuthor
#8 save-urls-2607618-8.patch3.89 KBgifad
#7 save-urls-2607618-7.patch3.59 KBgifad
#4 save-urls-2607618-4.patch416 bytesgifad
#3 save-urls-2607618-2.patch877 bytesscrufflefunk
#2 save-urls-2607618-1.patch877 bytesscrufflefunk

Comments

martin_q created an issue. See original summary.

scrufflefunk’s picture

scrufflefunk commented
Title: Source URL not being saved? » Possible workaround using $atom->data
Status: Active » Needs review
StatusFileSize
new save-urls-2607618-1.patch877 bytes

This patch uses the data field instead of the scald_url field, since in our case the latter does not exist.

scrufflefunk’s picture

scrufflefunk commented
StatusFileSize
new save-urls-2607618-2.patch877 bytes

This patch uses the data field instead of the scald_url field, since in our case the latter did not exist.

gifad’s picture

gifad commented
StatusFileSize
new save-urls-2607618-4.patch416 bytes

The url is already stored in the title property, there is no need to duplicate it...

martin_q’s picture

martin_q
Primary language English
Location Leipzig, Germany | Manchester, UK
commented

@gifad

The url is already stored in the title property, there is no need to duplicate it...

Storing a URL in a property called "title" sounds like bad code to me.

gifad’s picture

gifad commented

@martin_q : Could you please submit a patch, to show us what good code sounds like ?

gifad’s picture

gifad commented
StatusFileSize
new save-urls-2607618-7.patch3.59 KB

Ok, so here is a new patch, going on @scrufflefunk workaround using data property, with atom title and url decoupled (instead of being duplicated).
Next step would be to create a real field, but I did not want to touch my db schema..
Finally, this patch is compatible with running sites using either #2 or #4 patches

gifad’s picture

gifad commented
Title: Possible workaround using $atom->data » Source URL not being saved ?
StatusFileSize
new save-urls-2607618-8.patch3.89 KB

Back to original issue...

This patch just brings back the original 'scald_url' field, whose definition is now (I guess) somewhere in the ARTE installation profile (see http://info.arte.tv/profiles/foundation/README.md).

#7 or #8 : your choice...

renrhaf’s picture

renrhaf
Primary language French
Location 📍 Strasbourg 🐦🦜
commented

You are right, the module was initially developed by Arte for their needs.
This field 'scald_url' come from some features there.

  • Renrhaf committed 33ea882 on 7.x-1.x authored by gifad 
    Issue #2607618 by gifad, scrufflefunk, martin_q, Renrhaf: Source URL not...
renrhaf’s picture

renrhaf
Primary language French
Location 📍 Strasbourg 🐦🦜
commented
Assigned: Unassigned » renrhaf
Status: Needs review » Fixed

Thank you all of you for your time, and especially gifad for his patch.
I commited #8 with small modifications to 1.x branch.
Tested on a new install and the field successfully show and works, on ARTE side this causes no issue.

si.mon’s picture

si.mon commented
Status: Fixed » Closed (fixed)