Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
This evening, while making no change to my website I created a new blog post with a new embedded Soundcloud file, and found that Drupal only displayed the URL for the tune rather than the player. Older posts were still working fine, until I try to resave them.
The website is http://www.yeahmusic.co.uk/ - this is a really weird one, not sure if it is me or Soundcloud themselves.
I've updated this module to the dev version, as well as making sure all the other required modules are up to date, but no clue.
Any ideas anyone?
Comment | File | Size | Author |
---|---|---|---|
#50 | fix_soundcloud_stopped_embedding-1023718-v4_reroll.patch | 2.83 KB | nodecode |
#49 | fix_soundcloud_stopped_embedding-1023718-v4.patch | 2.35 KB | nodecode |
#47 | fix_soundcloud_stopped_embedding_v4.patch | 2.32 KB | nodecode |
#45 | soundcloud.inc v4.zip | 2.3 KB | nodecode |
#41 | media_soundcloud.zip | 8.87 KB | AntiNSA |
Comments
Comment #1
ewills CreditAttribution: ewills commentedHmm - digging into it, it looks like Soundcloud have changed their embed codes on their music pages.
I think line 113 in soundcloud.inc is the problem:
$h1tags = preg_match('/(
)/i', $response->data, $patterns);
This no longer comes up with any matches.
I'll have a go at some new pattern matching tomorrow to get it to recognise the new XML link which contains the swf link.
Comment #2
ewills CreditAttribution: ewills commentedOk - I've fixed it for when you just add a URL to the CCK field (rather than the whole embed code).
The module was generating the embed code itself in a bit of a hacky way. I've now enabled it to use the proper Soundcloud oEmbed method, meaning Soundcloud always generates the embed code automatically for you from the tune url.
I haven't tested this with embedding the whole embed code (it should work fine anyway as I haven't touched any of that side of things). I also haven't tested it for sets of music, but as Soundcloud is generating the code itself I can't see why it wouldn't work.
Edit:
Sets work just fine :)
Comment #3
joachim CreditAttribution: joachim commented> The module was generating the embed code itself in a bit of a hacky way.
Yup! Thanks for taking a look at this :)
Should we use drupal_http_request here?
And here, drupal_xml_parser_create()?
Powered by Dreditor.
Comment #4
ewills CreditAttribution: ewills commentedI've updated the patch to use the drupal_http_request, but not the drupal_xml_parser_create(), as doing this means I have to use xml_parse_into_struct(), which then creates a pretty nasty array that I'd have to loop through just to get the HTML bit. In SimpleXML it's just a case of accessing the appropriate child in the object.
Unless there's a specific reason for not using SimpleXML (or you can come up with a nicer solution using drupal_xml_parser_create()) I'd quite like to keep it as SimpleXML.
Comment #5
apoc1 CreditAttribution: apoc1 commentedHey, I'm having the same problem, but I can't patch the module, as I don't have enough permissions on the server (shared hosting)
Is there any other way to fix this (even manual?)
Comment #6
ewills CreditAttribution: ewills commentedI've attached the module with the patch applied - that should work for you.
Comment #7
joachim CreditAttribution: joachim commented> but I can't patch the module, as I don't have enough permissions on the server (shared hosting)
Patch the module on your own system, then upload the resulting module.
Comment #8
joachim CreditAttribution: joachim commentedCan someone test what happens if the user puts in a bad embed code? Should this part of the code check first for a correct response from the server before trying to parse the XML?
Powered by Dreditor.
Comment #9
ewills CreditAttribution: ewills commentedIf the url itself is invalid, the module doesn't let the user submit the content.
If the url is valid but doesn't go anywhere (404), the module just displays the URL in place of the soundcloud player in every instance it is called. What would you like it to do?
Comment #10
joachim CreditAttribution: joachim commentedI'd say if the soundcloud website doesn't return something valid, then this module should throw up an error to the user if it still can from this hook.
What do other emfield modules do in this case, eg YouTube?
Comment #11
ewills CreditAttribution: ewills commentedWith YouTube, if you put in an invalid URL (i.e. http://www.youtube.com/watch?v=Luaaaa71nET) the content is allowed to be saved, the video just doesn't show up.
I'll have a look at what I can do from within the hook tomorrow.
Comment #12
joachim CreditAttribution: joachim commentedI think as long as we do the same as the YouTube module, that's ok.
Comment #13
dreamdust CreditAttribution: dreamdust commentedPatches don't work for me.
Comment #14
joachim CreditAttribution: joachim commentedCan you give more detail please? Did they not apply, or did something go wrong later?
Comment #15
dreamdust CreditAttribution: dreamdust commentedDunno what details to give. Drupal 6.20, applied fine. Cleared cache. No errors. It's doing the same thing as before, it just stopped embedding and displays the URL. Is the patch working for you? I tried both patches.
Comment #16
ewills CreditAttribution: ewills commentedCould you give an example of a link you're trying to embed?
Comment #17
szantog CreditAttribution: szantog commented#13 you need to SAVE the node. I patched the the module, and saw, it didn't work. I started to debug it, then figured, the field->data doesn't update self when view a node. :)
This patch works me, but starting from this I was wondering, why is the complete swf object stored in $field['delta']['data']. Build the html, embed code is theme logic, isn't it?
Comment #18
szantog CreditAttribution: szantog commentedTemporary solution, i merged the patch in a theme function in my theme:
Comment #19
dreamdust CreditAttribution: dreamdust commentedI can confirm this is the problem. I had to re-save the node. Of course, I have 300+ nodes with soundcloud embeds that I have to save now.
Tell me, why does the module store the embed code? Is it really a big deal to hit soundcloud's servers everytime someone needs an embed code?
Comment #20
ewills CreditAttribution: ewills commentedYou can quickly save all of these nodes by going to the content pages - admin/content/node - and selecting all the nodes you need saving, then choosing "Reload embedded media data" in the update options and click "Update"
Comment #21
joachim CreditAttribution: joachim commented> Tell me, why does the module store the embed code? Is it really a big deal to hit soundcloud's servers everytime someone needs an embed code?
Is that not standard? Again -- please compare with Youtube module.
Comment #22
ewills CreditAttribution: ewills commentedHmm - yeah the YouTube module just stores the YouTube id in the media_youtube_node_data field - I'll take a look at adapting this module to do the same thing.
Comment #23
dreamdust CreditAttribution: dreamdust commentedWell there's no benefit to storing the embed code is there? I mean, if you're already hitting the server to load the flash widget then loading the embed code in addition to the widget isn't going to add much if any overhead.
Thanks! That's a big help.
Comment #24
joachim CreditAttribution: joachim commented> I mean, if you're already hitting the server to load the flash widget then loading the embed code in addition to the widget isn't going to add much if any overhead.
It's making two remote calls instead of one.
Best see what YouTube does and stick to that, since that was previously a part of emfield core, and so presumably does the Right Thing. Can someone look into that please?
Comment #25
ewills CreditAttribution: ewills commentedThe YouTube module does not store the embed code itself, instead it queries YouTube for it (so if the embed code changes you don't need to update all your modules).
I've fixed the Soundcloud patch so it follows the same behaviour now, where it pulls the embed code straight from Soundcloud every time a tune is displayed.
Comment #26
nodecode CreditAttribution: nodecode commentedsubscribing.
Comment #27
jseffel CreditAttribution: jseffel commentedTried the patch from #25:
patch -p0 < fix_soundcloud_player_v3.patch
patching file providers/soundcloud.inc
After that I tried to embed: http://soundcloud.com/timtainment/sets/genre-set-techno/
I got the "You have specified an invalid media URL or embed code." message.
Comment #28
joachim CreditAttribution: joachim commentedComment #29
AntiNSA CreditAttribution: AntiNSA commented+1
Comment #30
dLux CreditAttribution: dLux commentedwith the v3.patch i get:
Fatal error: Uncaught exception 'Exception' with message 'String could not be parsed as XML'
with no patch i get a couple:
warning: SimpleXMLElement::__construct() [simplexmlelement.--construct]
messages..
v1 & v2.patch aren't working either; +1 subscribing..
Comment #31
cyrilg CreditAttribution: cyrilg commentedwhenever I try to embed something either using an url or the embed code from Soundcloud, I get the following error after saving the node:
warning: Parameter 6 to emaudio_soundcloud_audio() expected to be a reference, value given in //sites/all/modules/emfield/emfield.module on line 650.
Anybody knows how to fix that ?
Comment #32
cyrilg CreditAttribution: cyrilg commentedTo fix it I had to modify the code of the file media_soundcloud/providers/soundcloud.inc.
On line 155 instead of
function emaudio_soundcloud_audio($embed, $width, $height, $field, $item, &$node, $autoplay)
it needs to be:
function emaudio_soundcloud_audio($embed, $width, $height, $field, $item, $node, $autoplay)
(Notice the "&" missing on the 6th parameter)
Comment #33
nodecode CreditAttribution: nodecode commentedso can anyone else confirm if fixing the typo as in #32 does the trick?
Comment #34
AntiNSA CreditAttribution: AntiNSA commentedI can
Comment #35
joachim CreditAttribution: joachim commentedJust a reminder from your maintainer of what needs to happen to fix this:
1. someone makes a patch and uploads it
2. at least 2 people try the patch and confirm it fixes the original problem
Comment #36
joachim CreditAttribution: joachim commentedI'm also confused as to how we've gone from patches earlier in this issue that changed a fair bit of code, to just removing a &...
Comment #37
shiroitatsu CreditAttribution: shiroitatsu commentedConfused as well: i used the patches & corrected the typo, but still not working.
Comment #38
nodecode CreditAttribution: nodecode commentedI used #6 and it seemed to work just fine for me, however, according to the thoughtful conversation between ewills and joachim, this is not future-proof if soundcloud decides to change their player embed code in the future. So ewills attempted to modify providers/soundcloud.inc to query the embed code directly from soundcloud's servers in #25 with the v3 patch (Note: for anyone trying to apply the v3 patch, it's against the 6.x-1.1 branch not the 6.x-1.x-dev branch or any of the previous patches in this thread).
Results from testing the v3 patch...
I've had no problems with any soundcloud urls EXCEPT when I used the ulr from #27 (http://soundcloud.com/timtainment/sets/genre-set-techno) i got a white screen of death when viewing the node. I went to the actual page on soundcloud and found that there is no embed code for this particular audio clip when clicking [share]. I believe this may have something to do with the problem I and others are seeing (more details below)...
After seeing the white screen, if I go back to edit the node and enter any other soundcloud url, it works, however, i get this set of error messages ONLY on the first view of the updated (working) node:
I reverted back to the patch in #6 and got similar results except that the error was on line 110
@joachim/ewills: Can I be of any further assistance in testing this more thoroughly?
Comment #39
nodecode CreditAttribution: nodecode commentedstill testing... i've found no errors with any soundcloud urls other than that one from #27.
Is there some way someone can take a look at what is being returned by:
$xml = drupal_http_request($swf);
...on line 87 of the v3 patched soundcloud.inc to see what is the value of $xml when using the soundcloud url from #27?
Perhaps then we can figure out why SimpleXmlElement is throwing those errors.
Comment #40
AntiNSA CreditAttribution: AntiNSA commentedI can confirm # 32 works. when using views I dont use the display embedded code field, I use the default. Everything works great. I think it should be marked as fixed.
Comment #41
AntiNSA CreditAttribution: AntiNSA commentedhere is a working version with the typo fixed. It works great for me.
Comment #42
nodecode CreditAttribution: nodecode commented@AntiNSA: I think you and kenzax may be discussing a different issue. I'm not entirely sure what it is that you're trying to resolve.
The version you uploaded also adds a "+" to line 85. I don't know where you're going with this but, for me, it's not a solution to the problems described in #30 or #38.
Comment #43
AntiNSA CreditAttribution: AntiNSA commentedthere may be an extra + . I will check it. But it is working great to embedd soundcloud with audio embedd... I need it to do so with fbsm. With fbsmp it is treating the link like a normal link.
Comment #44
nodecode CreditAttribution: nodecode commentedBacking up for a moment here... I've been testing ewills' versions of the module starting from #6 and i believe this is where the problems started.
@ joachim, in response to #8 where you asked:
I've tested thoroughly and I say YES the code SHOULD check first for a correct server response (code 200) before executing the SimpleXmlElement function because this is causing some of the errors I and other users are seeing. Not only can this version of the module not handle soundcloud server errors, it appears embed code can no longer be entered (only a simple valid url will work).
Try it yourself:
It all points back to lines 109-110 and the SimpleXmlElement:
In between those two lines should be a check for a proper response from the server. Perhaps like:
I have more test results to share if any of you are interested in fixing this together.
Comment #45
nodecode CreditAttribution: nodecode commentedI made two changes to ewills' version in #25 and now all tests succeed for me in each situation I outlined in my last post.
Changes made:
<object></object>
tags (this is how soundcloud embed code is formatted). Perhaps there's a better way to do this? But it works.@AntiNSA: this version does not attempt to fix the issue you're seeing, though your fix should be easy to implement if it's still a problem.
@joachim: your insights would be most helpful
I have no idea how to create a patch, so I've uploaded soundcloud.inc. Just drop in the "media_soundcloud/providers" directory to test.
Comment #46
joachim CreditAttribution: joachim commentedEverything you need to know about patches: http://drupal.org/patch
Please have a go at making one -- it makes the job of reviewing changes so much easier.
Comment #47
nodecode CreditAttribution: nodecode commentedSo I tried creating the patch with Git and its frustratingly complicated (i wasted 4 hours of my life). I gave up and created this patch with ssh diff command directly on my server. Hopefully this works, if not let me know. Please keep in mind it's my first attempt at creating a patch... ever!
Comment #48
joachim CreditAttribution: joachim commented4 hours!? Was it problems installing things on Windows? There's a workflow specific to this module here too: [#866584].
At any rate, congratulations on your first Drupal patch! :D
It looks pretty good, though it'll need a reroll with spaces instead of tabs, and some coding style issues (use Coder module to check for those).
Comment #49
nodecode CreditAttribution: nodecode commentedOkay i have a new patch rolled with Git. Will this work?
Comment #50
nodecode CreditAttribution: nodecode commentedSorry, I forgot to clean up the code... this patch should be fit for testing. Feedback is appreciated.
Comment #51
joachim CreditAttribution: joachim commentedThanks!
Finally had some time to work on this... :)
> the YouTube module just stores the YouTube id in the media_youtube_node_data field - I'll take a look at adapting this module to do the same thing.
I had a look at the YouTube module, and from what I can tell the reason it doesn't need to store embed code is that it expects to find local flash players which it can just give the YouTube URL to. We don't have that here, so I'm restoring the cached embed code.
Rerolled with a few other fixes for code style.
#1023718 by ewills, nodecode, joachim: Fixed Soundcloud embedding.
Comment #53
jseffel CreditAttribution: jseffel commentedI'm still having problems. Embedding soundcloud media via embed code works fine, but using the URL does not. The URL validates but the player is missing.
Reopen?
Comment #54
joachim CreditAttribution: joachim commentedFile a new bug please?