The flash players don't appear if clean URL support is disabled.

Comments

zirafa’s picture

StatusFileSize
new5.68 KB

I think this patch should fix this problem.

zirafa’s picture

However, 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.

zirafa’s picture

Just for further clarification:

This will also work:

song url: http://example.com/?q=audio/16/play
mp3.swf?song_url=http://example.com/?q=audio/16/play

Because the second question mark is not interpreted as part of flash's query string.

This will not work:

song url: http://example.com/?q=audio/16/play&extra
mp3.swf?song_url=http://example.com/?q=audio/16/play&extra

You can see how the flash query string will think &extra is another GET variable, instead of part of the song url.

drewish’s picture

i'll look into that but you should use base_path()

zirafa’s picture

base_path() only returns a relative url...you would prefer that over an absolute $base_url?

zirafa’s picture

I would argue for full paths because that would make it easy to generate "cut-n-paste" html for the players

drewish’s picture

Whoops, 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.

zirafa’s picture

This 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.

drewish’s picture

it looks like the last commit broke the flash player with the i8n module... http://drupal.org/node/114568

drewish’s picture

humm, 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.

zirafa’s picture

That 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.

zirafa’s picture

On 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.

zirafa’s picture

StatusFileSize
new6.75 KB

Ok 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.

zirafa’s picture

The 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

zirafa’s picture

Anybody review #13 above? it should be pretty straightforward. I can re-roll if necessary (probably should).

zirafa’s picture

StatusFileSize
new6.76 KB

Ok I've re-rolled the above patch against 5.x-0.2. It uses relative URLs for the player paths.

zirafa’s picture

StatusFileSize
new7.09 KB

And 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.

drewish’s picture

Status: Active » Needs work

zirafa, 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.

zirafa’s picture

StatusFileSize
new6.75 KB

Ok, I re-rolled against HEAD with the relative URLs.

zirafa’s picture

Status: Needs work » Needs review
drewish’s picture

Status: Needs review » Needs work

nope, you've still got the old versions of the files listed in the diffs.

you patched:

--- button.inc  23 Jan 2007 04:37:20 -0000      1.2
--- xspf_extended.inc   23 Jan 2007 04:36:49 -0000      1.2
--- xspf_slim.inc       23 Jan 2007 04:36:49 -0000      1.2
--- 1pixelout.inc       30 Dec 2006 01:20:48 -0000      1.1

take a look at:
http://cvs.drupal.org/viewcvs/drupal/contributions/modules/audio/players/

button.inc 	 1.3 	 2 weeks 	 drewish 	 - zirafa's patch for #114325 "Players don't work without clean URLs".
xspf_extended.inc 	 1.3 	 2 weeks 	 drewish 	 - zirafa's patch for #114325 "Players don't work without clean URLs".
xspf_slim.inc 	 1.3 	 2 weeks 	 drewish 	 - zirafa's patch for #114325 "Players don't work without clean URLs".
1pixelout.inc 	 1.2 	 2 weeks 	 drewish 	 - zirafa's patch for #114325 "Players don't work without clean URLs".
zirafa’s picture

StatusFileSize
new6.06 KB

ah. 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.

drewish’s picture

Status: Needs work » Fixed

great, i've committed that (with a few whitespace modifications) sorry i couldn't re-roll it myself but i've been slammed with school.

zirafa’s picture

no prob. i gotta work on my CVS chops anyway ;)

Anonymous’s picture

Status: Fixed » Closed (fixed)