jplayer: no theme override function, incorrect use of $base_path, and poor legacy browser support [#1845032]

i've found some problems with jplayer support, which i'm going to lump together in one issue because they're each quite small.

without theme override functions, it's not possible to add a skin from http://www.jplayer.org/latest/skin/ to jplayer, which isn't very useful.

the variable $base_path is added to $player_base_path before drupal_add_js() is called, which breaks on all sites that do language switching by URL prefix. drupal_add_js() doesn't need this; it should just be added later on to the swfPath parameter.

it's necessary to set the parameter solution to "flash, html" in order to support older browsers. i believe this should be set by default; people can change that by overriding the theme function if desired.

also, there were some tabs which need to be replaced with spaces: http://drupal.org/coding-standards#indenting

Comment	File	Size	Author
#1	audiofield-jplayer-support-1845032-1.patch	4.35 KB	mvc

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

mvc

they

English

Montréal, CA

CreditAttribution: mvc commented 19 November 2012 at 19:21

Status:

Active

» Needs review

File	Size
audiofield-jplayer-support-1845032-1.patch	4.35 KB

Comment #2

benjifisher

he/him or they/them

English

Boston area

CreditAttribution: benjifisher commented 21 December 2012 at 15:36

Status:

Needs review

» Reviewed & tested by the community

@mvc:

Nice job. I will mark #1729346: Wrong jPlayer file path triggers error at Locale module when tries to fetch it as a duplicate.

Comment #3

jenlampton

she / her

CreditAttribution: jenlampton commented 25 April 2013 at 05:56

+1 for RTBC, thanks so much! :)

Comment #4

costinius CreditAttribution: costinius commented 2 September 2013 at 23:47

Hi, I've installed the latest 7.x-1.0-beta8 module and it was working with jplayer, but without a skin. So, after applying the patch it stopped working completely (yep, tried without a skin, and with a skin added). Am I doing anything wrong? Thanks a lot!

Comment #5

benjifisher

he/him or they/them

English

Boston area

CreditAttribution: benjifisher commented 9 September 2013 at 17:30

@costinius:

It is working for @mvc and for two of us who have tested and reviewed, so my first guess is yes, you are doing something wrong. But first guesses are not always right, and there are a lot of things that could contribute to the problem: javascript, server configuration, other modules, which browser you are using ...

Try starting with a fresh installation of Drupal and the latest version of this module from git. (See the "Version control" tab on the project page for instructions. In this case, I think that the latest version from git is the same as 7.x-1.0-beta8, but you should save us the trouble of checking that.) Is the problem reproduceable? Can you try with a few different browsers? Try checking for javascript errors.