Closed (fixed)
Project:
SWFObject API
Version:
6.x-1.0-alpha1
Component:
Code
Priority:
Critical
Category:
Feature request
Assigned:
Reporter:
Created:
8 Jul 2008 at 13:13 UTC
Updated:
12 Nov 2008 at 07:33 UTC
Jump to comment: Most recent file
Comments
Comment #1
duellj commentedIf you haven't created it already, here's a patch for swfobject_api port to 6.x. Let me know if this works for you (worked fine in my testing).
--Jon Duell
Comment #2
japanitrat commentedhead seems completely b0rken.
Comment #3
duellj commentedWhat do you mean? Does the patch not apply for the HEAD version? Or Is HEAD itself broken?
Comment #4
japanitrat commentedHEAD itsefl is broken and the mentioned patch won't fix it.
problem is the settings-page / variable_get. (already reported)
also it seems that it mixes up the default parameters. for instance in some lines it refers to the express-option, although it should refer to the version.
I've rewritten both, the settings-page as well as the theme-method (has a lot of new parameters now), so old code won't work with my one, if you go with the syntax in the readme. but it works better now. at least for my purposes ;)
Comment #5
arthurf commentedHi-
Can you redo your patch using DRUPAL-5 if your 6x branch is using the 1.5 release of swfobject? The version you patched against is somewhat old.
thanks!
a.
Comment #6
duellj commented@arthurf
Here's my patch against the DRUPAL-5 branch.
Thanks,
Jon
Comment #7
arthurf commentedOk, I created the DRUPAL-6--1 branch, so you can check that out and see if it's working for you. I don't have a 6 install setup at the moment, but I'll try to test shortly. Thanks very much for this patch, it's great to have people helping out!
Comment #8
designerbrent commented@arthurf: I Can't seem to find the DRUPAL-6--1 branch. Do you know where i can find it?
Comment #9
arthurf commented@designerbrent You can grab it from the CVS repo here: http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/swfobject_a... or you can just check it out with a CVS client
Comment #10
designerbrent commented@arthurf: thanks. Do you plan on creating a release?
Comment #11
greenskin commentedHere's a downloadible port to Drupal 6.
Comment #12
robloachJust a quick note that this module rocks....
Comment #13
Mark Theunissen commentedJust a note to anyone using greenSkin's .zip in #11, there's a runaway "print" statement in the function theme_swfobject_api which prints the text "Array" next to the flash! The line is "print $params;" on 112. Just remove it and everything should work.
Thanks for the module everyone! :)
Comment #14
robloachIt would be nice to have a patch from HEAD/DRUPAL-5--2 instead of just a zip ;-) .
Comment #15
designerbrent commentedI think the zip is the same as the patch but i"m not sure.
Comment #16
Mark Theunissen commentedI've found another bug - the 6.x zip in #11 doesn't work in Internet explorer. The problem is that IE can't parse Javascript when the last element in the list has a comma after it like this:
var params = {
width: '760',
height: '210',
no_flash: 'Sorry, you need to install flash to see this content.',
version: '5',
type: 'movie',
bg_color: '#FFFFFF',
};
Note bg_color: '#FFFFFF',
You can fix this around line 146 of swfobject_api.module. Just don't output a comma on the last parameter.
Comment #17
designerbrent commentedThat is correct, Mark. We found that recently and I would make a patch, however, the version that I have has several other hacks in it to suit my needs. When I get a chance, I need to fix that. Maybe I'll just and just get the relevant code out that we did for this update.
Comment #18
Mark Theunissen commentedOk, sure thing. I have checked the contents of the .zip file against the DRUPAL-6--1 branch and they are indeed different.
I wouldn't mind helping out reconciling the two, as we use this module and would love to get a proper 6 release. But seeing as you've done a whole lot of work already, maybe you could share your current code or make a patch for 6-1 ?
Comment #19
arthurf commentedCan somebody give this back to me as a patch and I'd be happy to work on applying this to the D6 branch.
Comment #20
Mark Theunissen commentedHappy to do it - as long as designerbrent is not already half way there? ;)
Comment #21
designerbrent commentedGo ahead Mark. I haven't had time to work on it yet.
Comment #22
Mark Theunissen commentedOk, I have finished creating the patches, which combine the changes found in the .zip file in #11 with the branch 6-1--1.
Here are a list of the other changes I've made:
Note that the /po directory must be renamed to /translations for it to be fully Drupal 6 compatible.
I have tested in Firefox Mac, IE6, IE7 and it all looks good!
Cheers
Mark
Comment #23
chasz commentedhow much of a bandwidth hit is this??
swfobject is like 30kb right?
+1 subscribing
Comment #24
bonobo commentedPossible to get this tagged as a 6.x-2 dev release?
With emfield admin options pointing to this module, and with the recent emfield alpha3 release, creating a dev release for this module could have the added benefit of more testers.
Comment #25
Crell commentedRevised patch. This is #22 rolled into a single file, patched against the DRUPAL-6--1 branch.
I also went ahead and did a ton of cleanup to the theme function. It now uses Drupal.settings in a clean fashion, so we're not writing bits and pieces of inline JS from PHP (which is painful). That clears up a bunch of issues with IE compatibility because we let Drupal build the JS for us. It also means that we're now properly checking for multiple uses, so we can place a flash image multiple times on a single page without things breaking. The JS has all been moved to the footer as well for better performance.
I also restored the "administer swfobject" permission. "administer site configuration" is a horrible horrible permission that flies in the face of configurability, and it should be exterminated with extreme prejudice from Drupal. :-)
Checked in Firefox 3, Safari 3, and IE 6, works fine in all of the above.
I need this for at least 2 of my modules, so let's get it in and tagged ASAP. :-)
Comment #26
Mark Theunissen commentedThanks Crell, I wasn't sure about the permission because I couldn't decide whether it was intentionally removed (as it was absent from either the zip or the branch, can't remember).
The theme function definitely needed an overhaul. ;)
Cheers
Mark
Comment #27
arthurf commentedI'll get rolling on this later this week. I have a client deadline that I have to push through first.
Comment #28
trophaeum commentedsubscribing, looking at integrating this into video_filter
Comment #29
grey_ commentedSubscribing
Comment #30
arthurf commentedI merged these changes into DRUPAL-6--1-0-ALPHA1. Thanks to everybody who contributed to this- great to have so much support!
Comment #31
Crell commentedHooray! Thanks, Arthur!
Marking fixed since any future follow-up work should go in a different issue.
Comment #32
robloachRock on!
Comment #33
Anonymous (not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.