Skip to content

Loading…

Fail to embed most soundcloud links due to soundcloud switching to https #676

Closed
patricksnape opened this Issue · 2 comments

2 participants

@patricksnape

Soundcloud have recently switched to using HTTPS for all their links. Therefore, in subreddits that regularly post Soundcloud links, the majority of the posted links do not receive an inline media expando.

Eg. http://www.reddit.com/r/mashups/comments/180uaf/old_but_one_of_my_favorite_mashups_from_one_of_my/

The proposed fix for this, I believe, is in r2/r2/lib/scraper.py:

video_id_rx = re.compile('^http://soundcloud.com/[a-zA-Z0-9_-]+/([a-zA-Z0-9_-]+)')

Should become

video_id_rx = re.compile('^http[s]?://soundcloud.com/[a-zA-Z0-9_-]+/([a-zA-Z0-9_-]+)')

Would it be helpful for me to submit a pull request for this change? Would this fix the issue?

@patricksnape

It should also be noted that the current regex does not allow for albums to be posted, and neither does the current embed HTML. It may be better to change the embed HTML to:

media_template = """<div style="font-size: 11px;">
                          <object height="81" width="100%">
                            <param name="movie"
                                  value="http://player.soundcloud.com/player.swf?url=$video_url">
                            </param>
                            <param name="allowscriptaccess" value="always"></param>
                            <embed allowscriptaccess="always" height="81"
                                   src="http://player.soundcloud.com/player.swf?url=$video_url"
                                   type="application/x-shockwave-flash"
                                   width="100%">
                            </embed>
                          </object>"""

and use the whole Soundcloud URL instead of just the video_id. This use of the player, as detailed at the Soundcloud JSAPI wiki suggests using URL and not ID, presumably because it handles albums as well as single tracks. We could then drop the video_id_rx completely.

@spladug
reddit member

Fixed in 252437d. Embed.ly's service API is used to get a fresh list of sites that it supports on every startup of the queue processor. This should prevent future occurrences of similar issues as well.

@spladug spladug closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.