I've been trying to get the colorbox module working with the Video module for my client. This patch allows you to click on a video thumbnail and open the video in a colorbox overlay. Once you have applied the patch, an extra option appears (content type->manage display) in the "Video thumbnail" format. Simply "Link video or video thumbanil to" > "Colorbox".

I have only had the time to make it work with Flowplayer - if anyone wants to make it work for other players, have a look at the new theme_video_colorbox() function.

Comments

tommy kaneko’s picture

Category: task » feature
Status: Active » Needs review
StatusFileSize
new7.76 KB
new4.94 KB

This is my first patch, so please point out any mistakes. the patch should be applied to the latest dev branch

You also need to add the js files in the zip file attached to video/js directory.

switzern’s picture

This is great! Thanks for putting this patch up. Just a couple things I had to tweak to get it working on my end.

I was getting an unidentified index on line 296 of video.theme.inc and an unidentified class on line 298 of that same file.

Here's how I tweaked those lines to get rid of the errors:

  $class = array('video-box', 'video-' . $variables['field']['field_name'] . '{width:\'' . $video->player_width . 'px\',height:\'' . $video->player_height . 'px\',player:\'' . $player . '\'}');

  $classes = implode(' ', $class);
  
  return l($image, $video->files->{$video->player}->url, array('attributes' => array('class' => $classes), 'html' => TRUE));

I'm also having trouble manually uploading a thumbnail to a second video on the same node. I'm getting the error mentioned in this thread, but the patch doesn't seem to fix it. Any idea what might be happening there?

Again, thanks for making this patch available.

tommy kaneko’s picture

StatusFileSize
new6.39 KB

Thanks for testing the patch. I had the same problem too. I have found the root of the problem to be in the function video_thumb_process(). On line 218:

<?php
    if (isset($file['thumbanail']) && !empty($file['thumbanail'])) {
      $large_thumb = file_load($file['thumbanail']);
    }
?>

The variable $file['thumbanail'] was an array, and file_load() did not like that. This may be a problem with the wider module, but I put in a check for arrays, which has solved my particular issue. I would welcome a developer with better knowledge of the video module to review this patch. In any case, I changed the above code to:

<?php
if (isset($file['thumbanail']) && !empty($file['thumbanail'])) {
      if (is_array($file['thumbanail'])) {
        $fid = '';
        foreach ($file['thumbanail'] as $key => $value) {
          if ($key == 'fid') {
            $fid = $value;
            break;
          }
        }
        $large_thumb = file_load($fid);
      }
      else {
        $large_thumb = file_load($file['thumbanail']);
      }
    }
?>

Attached is the patch that includes all the changes in this issue, including your fix. Let me know if it did the trick.

Remember to download the js files in #1 to get this working!

tommy kaneko’s picture

StatusFileSize
new7.67 KB

I noticed that there was a problem on nodes where video the video field was empty, theme_video_colorbox thinks that the thumbnail (the default one) should be made into a colorbox link. It threw errors which were annoying. Attached is a patch to replace the one above which adds a quick check to get rid of it.

switzern’s picture

Right on, thanks for addressing those issues.

I applied the new patch (on a fresh install and the version of the video module I'd been working with) and I'm getting a couple different errors now.

Everything works fine when I add a single video and manually upload a thumbnail, but when I try to manually add a thumbnail to a second video on the same node I get the following errors:

Notice: Undefined index: thumbanail in file_ajax_upload() (line 256 of modules/file/file.module).

Notice: Undefined index: 1 in file_ajax_upload() (line 265 of modules/file/file.module).

Notice: Undefined index: #suffix in file_ajax_upload() (line 274 of modules/file/file.module).

I spent a little time looking into them and wasn't able to figure too much out. I should be able to come back to this and spend some more time with it in the next couple days, so I'll let you know if I'm able to figure anything out.

tommy kaneko’s picture

I have had those issues too. I have managed to get rid of it for my particular setup by changing the code as discussed in #3.

I am pretty sure it would be a problem with function video_thumb_process() in video.module. It is specifically a problem with how file_load() is called. It seems the function video_thumb_process() does not always pass an 'fid' string to the file_load() function.

I think it may be case of a simple check. But then again, it is probably much more complicated than that. I'm new to Drupal 7, so this effort of mine is a bit of a swipe in the dark.

Will have a go in the morning and see if I can't come up with a patch. Good to be working with you!

rolandk’s picture

Just wanted to check if this patch has made its way into the dev version yet ?

hypertext200’s picture

Status: Needs review » Closed (won't fix)
hypertext200’s picture

Version: 7.x-1.x-dev » 7.x-2.x-dev
Status: Closed (won't fix) » Active

Working on the colorbox integration.

hypertext200’s picture

Assigned: tommy kaneko » Unassigned
hypertext200’s picture

Title: Colorbox and Flowplayer integration » Colorbox integration
hypertext200’s picture

Status: Active » Needs review
StatusFileSize
new4 KB

Attaching the patch.

Jorrit’s picture

Status: Needs review » Needs work

The patch doesn't conform the Drupal coding standards (tabs/spaces and more), and there is a typo: colrobox.

hypertext200’s picture

Status: Needs work » Needs review
StatusFileSize
new5.73 KB

Yep, and nice eye on typo, fixed it and fix another issue which will led colorbox to open multiple times. Not sure why formatting are weird hope this will be okay.

hypertext200’s picture

StatusFileSize
new5.72 KB

Fixed the formatting manually

hypertext200’s picture

Status: Needs review » Needs work

Still the formatting are not correct.

hypertext200’s picture

Status: Needs work » Needs review
StatusFileSize
new5.78 KB

Formatted patch.

caskraker’s picture

Tested the patch under #17 but got an error:

De page "/mysite/video/embed97?width=&height=10&iframe=true" can't be found.

There is no such path video/embed in my system, and most certainly it does not contain my video's.
Tried changing the path tot default/files/videos/original where the Video module stores it's video-uploads but i keep the 97?....... stuff in the page name and thus the same error.

Any clues?

Chipie’s picture

I have multiple values for the video field in node. I have applied the patch in #17. After clicking on a thumbnail, only the first video is shown in the colorbox. Does the patch support in #17 support multiple values for the video field in a node?

hypertext200’s picture

@caskraker, Please cache clear and see if the path is still not there, cause the /video/embed/ is from the Video core.
@Chipie, atm this patch does not provide the support for multiple files.

caskraker’s picture

#20: when i check my video dir (sites/all/modules/video) there is no /embed subdir. I clear caches all the time ;-)

I abandoned colorbox for displaying video in the meantime, but i remain interested in finding a solution for this. Is there any chance of integrating the patch in a future version?

Thanx,
Cas

sikvsx’s picture

Version: 7.x-2.x-dev » 7.x-2.9
Assigned: Unassigned » sikvsx
Category: feature » bug
Status: Needs review » Needs work

i have apply the patch successfully and i have also set the video thumbnail link to colorbox. when i click on thumbnail it opens a new web browser page and shows the video it that.

hypertext200’s picture

@caskraker the path is not the file path, it a Drupal internal path. @sikvsx thanks for testing.

wjackson’s picture

The patch in #17 works great! Thank you, heshan.lk!

@sikvsx, I experienced the same issue when upgrading beyond colorbox 2.0. I was able to resolve the problem by installing the latest version of colorbox and installing the most recent colorbox library.

Jorrit’s picture

Status: Needs work » Fixed

I committed patch #17. I forgot setting up git attribution properly, sorry Heshan :(.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.