This module defines jw_player_inline_js as TRUE in two instances but FALSE on the hook_init call. This causes the default to display as TRUE on the settings page but yet when it goes to runt he default falls back to FALSE, leading to potential confusion as to why the library isn't being included. This patch establishes a defined value to help mitigate this issue in the future.

CommentFileSizeAuthor
jw_player-inlineconsistency.patch1.92 KBbtopro
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

weseze’s picture

weseze CreditAttribution: weseze commented

Why not simple change the default behaviour to TRUE in the init hook?

btopro’s picture

btopro CreditAttribution: btopro commented

generally ambiguous bolean values used in multiple places should be defined as a global value to avoid inconsistencies like this from popping back up.

rickvug’s picture

rickvug CreditAttribution: rickvug commented

I agree with btopro about changing this to a global.

The reason for not going with a simple hook_init implementation is to try to reduce page weight to improve front end performance. Many sites only use video on a handful of pages and don't want to load the player JS unless is is actually required. However caching can cause problems as the player JS is added via a process function. When the cached value is displayed this process function will not run, breaking the player. Inserting a JW Player video via an input filter is a good example of when a caching issue may crop up. For this reason there is a variable to switch to loading the player on all pages via hook_init. That is the thinking behind the present design. IMO whether or not this is the best possible design, or if it is worth the effort, can be tackled in another issue.

kingfisher64’s picture

kingfisher64 CreditAttribution: kingfisher64 commented

Patch didn't apply cleanly against 26th Jan 2013 dev version.

Hunk #1 succeeded at 16 with fuzz 1.
patching file jw_player.module
Hunk #2 succeeded at 447 (offset 11 lines).
Hunk #3 FAILED at 457.
1 out of 3 hunks FAILED -- saving rejects to file jw_player.module.rej
btopro’s picture

btopro CreditAttribution: btopro commented

pretty easy patch, may want to do it by hand and reroll if you have an issue with it

ron_s’s picture

ron_s CreditAttribution: ron_s commented
Issue summary: View changes
Status: Needs review » Closed (duplicate)
Related issues: +#2713725: Refactor theming, +#2719995: Add not empty checks and Repeat option, +#2719977: Add js file to build player

In 7.x-2.x, we've removed all inline JS and are using #attached to ensure players render correctly even when caching is enabled.

Please test the following patches along with 7.x-2.x-dev. Thanks.

* Refactor theming: https://www.drupal.org/node/2713725
* Add not empty checks and Repeat option: https://www.drupal.org/node/2719995
* Add js file to build player: https://www.drupal.org/node/2719977