Drupal 7

Media integration into Video fields

Requires:
http://drupal.org/project/media
http://drupal.org/project/video

Project Page:
http://drupal.org/sandbox/dlumberg/1364538

Git:
git clone --recursive --branch 7.x-1.x git.drupal.org:sandbox/dlumberg/1364538.git media_video

Comments

dlumberg’s picture

dlumberg commented
Status: Active » Needs review
miksan’s picture

miksan commented
Status: Needs review » Needs work

The code seems tidy :).
Here's a few remarks from an automated review: http://ventral.org/pareview/httpgitdrupalorgsandboxdlumberg1364538git

miksan’s picture

miksan commented

In media_video.module:
Line 68 & 74:
The use of call-time pass-by-reference has been deprecated since PHP 5.3.0 and the systems throws a warning:
"Deprecated function: Call-time pass-by-reference has been deprecated in drupal_load() (line 1128 of /home/mikkel/workspace/do-review.dev/htdocs/includes/bootstrap.inc)."
The fix is to remove the & from the parameters in the function calls.

dlumberg’s picture

dlumberg commented
Status: Needs work » Needs review

Thanks for reviewing miksan. Good catch, I had just copied the function definition. I removed the & references from

  // Get the Video Upload element.
  $elements = video_field_widget_form($form, $form_state, $field, $instance, $langcode, $items, $delta, $element);

and 

    // Get the Media Element.
    $media = media_field_widget_form($form, $form_state, $field, $instance, $langcode, $items, $delta, $element);

and committed the change to git.

miksan’s picture

miksan commented
Status: Needs review » Needs work

If I press the "select media" button and afterwards either upload or pick a video from the library nothing happens.
What is suppose to happen?

media_video.js:
Line 6:
This event never seems to be fired:

$('.video-widget .media-widget input.fid').change(function(e)

Line 25:
I think the trailing comma could create an error in IE (I have not tested it):

    }, // EO attach
dlumberg’s picture

dlumberg commented

What browser are you using? I tested it in Chrome, FF, Safari, IE9 and after uploading and pressing "upload" or selecting a video from the library and hitting "submit" it fires the JS call and reloads the form field. I haven't tested in older versions of IE.

miksan’s picture

miksan commented

Sorry still no success.
I have testet in IE9 in windows 7 and chrome/FF in linux mint.
I could not see media_video.js in source code. But after disabling overlay I saw it.
Still it looks like line 6 is still not getting triggered. I have not been diving into why it reacts like that.
Have you tried to restart with a clean Drupal install and activating only media_video (with its dependencies)?

dlumberg’s picture

dlumberg commented

Have you tried to restart with a clean Drupal install and activating only media_video (with its dependencies)?

I developed it in a Clean Install of drupal with nothing else and the default drupal 7 theme.
with Media 7.x-2.0-unstable5+3-dev
and Video 7.x-2.6

I will try a fresh install with overlay on and see what happens.

dlumberg’s picture

dlumberg commented

Starting a new install from scratch I had no issues and I couldn't reproduce your errors. I tested it in Windows 7 with IE9, Chrome and FF with the JS console open and didn't see any errors.

Here's my environment:


cd sites/all/modules
drush dl drupal
drush dl media-7.x-2.x-dev
drush dl file_entity
drush dl video
drush dl ctools
drush dl views
git clone --recursive --branch 7.x-1.x git.drupal.org:sandbox/dlumberg/1364538.git media_video
drush en media video media_video ctools file_entity views views_ui file_entity

Install.php using sqlite

Here's what I did to setup the content type "Video"


Add content type Video
Add field video to content type Video 
  >> field type: video
  >> widget type: Media file selector
  >> enabled browser plugins: library, my files (default media browser view)

Here's what I tested (the expected functionality of the module):


/node/add/video >> add a node with an uploaded video (to populate media library)
  >> node saved no issues

/node/add/video >> Select Media button >> select previously uploaded video
  >> selected previously uploaded video
  >> on submit in media browser the video field reloaded as expected

/node/add/video (using overlay) >> Select Media button >> select previously uploaded video
  >> selected previously uploaded video
  >> on submit in media browser the video field reloaded as expected
dlumberg’s picture

dlumberg commented
Status: Needs work » Needs review
svla299’s picture

svla299 commented

I have installed the module. When I press the "select media" button and select a file from the library I'm getting the message "The file used in the Video field may not be referenced." Is there a way to solve this problem?

paravibe’s picture

paravibe commented

In media_video.module file found 2 errors:
105 | ERROR | Wrong function doc comment end; expected "*/", found "**/"
119 | ERROR | Wrong function doc comment end; expected "*/", found "**/"

anwar_max’s picture

anwar_max
Primary language English
commented
Status: Needs review » Needs work

Hi dlumberg,

Manual Review:
1) Please add more information about module, configuration and installation process to README.txt.

Automated Review:

Please try to fix this issue Result of automated review

There is still a master branch, make sure to set the correct default branch: http://drupal.org/node/1659588 . Then remove the master branch, see also step 6 and 7 in http://drupal.org/node/1127732
Review of the 7.x-1.x branch:

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs work » Closed (won't fix)

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

If you reopen this please keep in mind that we are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (found some problems with the project) or "reviewed & tested by the community" (found no major flaws).

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Issue summary: View changes

Updated issue summary.