Closed (fixed)
Project:
Audio
Version:
5.x-1.x-dev
Component:
players
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
30 Jan 2007 at 19:23 UTC
Updated:
28 Feb 2007 at 19:20 UTC
Jump to comment: Most recent file
Comments
Comment #1
zirafa commentedI think this patch should fix this problem.
Comment #2
zirafa commentedHowever, if the destination url loaded into the flash player has an extra get variable, the flash player may have trouble reading the url because it interprets the playlist_url or song_url get variables as flash variables to pass into flash.
Works:
song_url=http://example.com/audio/15/play
mp3.swf?song_url=http://example.com/audio/15/play
Doesn't work:
song_url=http://example.com/audio/15/play&getfile
mp3.swf?song_url=http://example.com/audio/15/play&getfile
the &getfile is passed to mp3.swf and not considered part of song_url. This will result in incorrect behavior.
Workarounds are available:
http://www.adobe.com/cfusion/knowledgebase/index.cfm?id=tn_14234
Basically you just have to be careful when passing flash variables, so that the data is parsed correctly. The simplest solution is to try and avoid sending flash variables with ampersands in them.
Comment #3
zirafa commentedJust for further clarification:
This will also work:
Because the second question mark is not interpreted as part of flash's query string.
This will not work:
You can see how the flash query string will think &extra is another GET variable, instead of part of the song url.
Comment #4
drewish commentedi'll look into that but you should use base_path()
Comment #5
zirafa commentedbase_path() only returns a relative url...you would prefer that over an absolute $base_url?
Comment #6
zirafa commentedI would argue for full paths because that would make it easy to generate "cut-n-paste" html for the players
Comment #7
drewish commentedWhoops, my bad, that's what I get for shooting off replies between classes. With a second to reflect on it that patch looks good. I'm going to commit it but leave this open so I'll have a reminder to look at how to clean up the parameter passing to the flash players.
Comment #8
zirafa commentedThis link provides more info about flash query urls:
http://livedocs.macromedia.com/flash/8/main/wwhelp/wwhimpl/common/html/w...
It claims that escaping the ampersand will solve the problem, but from my tests it doesn't seem to be working. I think we are outputting everything correctly so I think we should dismiss this as a Flash problem and not worry about it.
Comment #9
drewish commentedit looks like the last commit broke the flash player with the i8n module... http://drupal.org/node/114568
Comment #10
drewish commentedhumm, that link to the flash docs didn't work for me. it just gave me a logo with no text. i think we really ought to look at passing the parameters as flashvars rather than as parts of the URL.
Comment #11
zirafa commentedThat link to the flash documentation takes a long time to load.
Hmm, since the flash players are real files on the server we may have to define a variable with the path to the players directory (similar to file_directory_path()). I'm guessing that trying to use drupal_get_path is resulting in the path getting aliased by the i18n module which shouldn't ever happen.
Comment #12
zirafa commentedOn second thought, drupal_get_path('module', 'audio') should work fine. It might be $base_url, or it might be a bug in i18n module which is still in dev.
Comment #13
zirafa commentedOk here's a new patch that passes variables using Flashvars, and it also adds an embed tag. I also changed $base_url to base_path() for the time being to see if it fixes the i18n problem. We can change it later if necessary.
Comment #14
zirafa commentedThe flash url thing is solved - the basic answer is that you should be *extra* certain the ampersands are encoded as %26 in a flash variable. See #38 over here: http://drupal.org/node/93968
Sincerely,
Zirafa Holmes, Scotland yard
Comment #15
zirafa commentedAnybody review #13 above? it should be pretty straightforward. I can re-roll if necessary (probably should).
Comment #16
zirafa commentedOk I've re-rolled the above patch against 5.x-0.2. It uses relative URLs for the player paths.
Comment #17
zirafa commentedAnd here is a version with absolute URL paths for the players. Drewish I'll leave it up to you to decide if you want the paths to be absolute or relative.
Comment #18
drewish commentedzirafa, neither of those patches applies cleanly. can you re-roll them? or perhaps just the relative URLs one? i suppose at this point i'd like to leave them as relative paths.
Comment #19
zirafa commentedOk, I re-rolled against HEAD with the relative URLs.
Comment #20
zirafa commentedComment #21
drewish commentednope, you've still got the old versions of the files listed in the diffs.
you patched:
take a look at:
http://cvs.drupal.org/viewcvs/drupal/contributions/modules/audio/players/
Comment #22
zirafa commentedah. this is what happens when you have too many checkouts and can't figure out what versions they are.
i *think* this should be patched against HEAD.
Comment #23
drewish commentedgreat, i've committed that (with a few whitespace modifications) sorry i couldn't re-roll it myself but i've been slammed with school.
Comment #24
zirafa commentedno prob. i gotta work on my CVS chops anyway ;)
Comment #25
(not verified) commented