Hey folks,
I've been working on a patch that adds a checkbox to the field that allows for entering custom embed code, i.e. an iframe, to allow for non-support video host embedding. It's mostly functional, but there are some lingering issues.
1) (Major) Does not work when VEF is marked as required. I tried determining this in hook_field_widget_form(), but if the user changes the checkbox and then saves, it still thinks the previously required field is still required. This needs to be determined after submitting.
2) (Minor) When a node is first published, the embedded video doesn't show up. Every subsequent visit, or after a refresh, will properly show the embedded video. I haven't looked much in to this.
3) (?) The field is not validated in any way. Since it's just plain HTML, users could potentially put anything in here. Not sure how big of an issue this is.
Attached is a patch that has my work thus far.
Comment | File | Size | Author |
---|---|---|---|
#11 | Embedded Video_Ustream too Low.jpg | 76.36 KB | adrianedge |
#2 | video_embed_field_2 - Copy.txt | 24.76 KB | adrianedge |
embed-code-WIP.patch | 8.43 KB | wbobeirne | |
Comments
Comment #1
adrianedge CreditAttribution: adrianedge commentedI have been trying to add this code to my video_embed_field.module file to use it even in its raw state. But I am not sure how to add and how to subtract and what lines up with what. Can you give me any clarification.
Best,
Adrian
Comment #2
adrianedge CreditAttribution: adrianedge commentedThis is a copy of my video embed field module file, where I was trying to apply your changes. I didn't get too far, but I'd like to implement this feature even with the bugs you describe so badly that I will keep trying.
I am guessing that the lines with the plus sign indicate code to be added, and the ones with the minus sign code to be taken away?? Am I correct?
There are some other symbols in front of some of the function calls, are these supposed to be there. I am viewing your code by clicking on it and viewing it in my browser (firefox).
Lastly, please help me update my file. I will be happy to compensate for the hours if you want.
Best,
Adrian
Comment #3
adrianedge CreditAttribution: adrianedge commentedThis is a copy of my video embed field module file, where I was trying to apply your changes. I didn't get too far, but I'd like to implement this feature even with the bugs you describe so badly that I will keep trying.
I am guessing that the lines with the plus sign indicate code to be added, and the ones with the minus sign code to be taken away?? Am I correct?
There are some other symbols in front of some of the function calls, are these supposed to be there. I am viewing your code by clicking on it and viewing it in my browser (firefox).
Lastly, please help me update my file. I will be happy to compensate for the hours if you want.
Best,
Adrian
Comment #4
jec006 CreditAttribution: jec006 commentedHi adrian,
The best way to apply his patch and get this working before its committed is to use git
If you go into the video_embed_field directory on a linux/unix machine/server and run:
it should apply the patch for you (basically add the lines with + and remove the lines with minus).
If you don't have git on your server you can try something like
but thats a bit trickier.
It is definitely my intention to get this into the module as soon as it is working consistently, so hopefully it won't be too long of a delay.
Hope that helps
Comment #5
jdelaune CreditAttribution: jdelaune commentedNot sure the reasoning behind this patch.
Why not juts add a plain text field if you just want to use custom embed codes? You loose basically all the benefits of VEF by pasting in raw embed code. Including style settings, images etc.
I think the handler model is powerful and I would question why you don't write custom handlers to support the other sources you want to support. I write custom handers for the custom places our sites get videos from and you obtain all the benefits of VEF mentioned above.
Comment #6
jec006 CreditAttribution: jec006 commentedI think that is a good point ... I guess to me the single benefit would be for view mode configuration - having a single field simplifies things. I think you're right though, having the ability to make it custom does ruin your ability to use the good stuff vef provides ... and losing the consistency makes using thumbnails for listings and such things not reasonable.
Alright, good call. We'll hold off on this for now. I think jdelaune is right, if you want this functionality, the best answer is to force embed of all things using a text field (youtube and vimeo will give you the embed code also).
thank you as always for the advice and points jdelaune :)
Comment #7
adrianedge CreditAttribution: adrianedge commentedThank you ! I am using a shared unix hosting package at Network Solutions, which doesn't allow for me to use GIT. So I will try download my site to a Xamp localhost environment on a windows machine, and use patch -p1 < embed-code-WIP.patch in Cygwin.
My second question is will the patched video embed module allow me to embed Ustream, and configure the space as it does for YouTube?
Secondly what do I need to know well to start coding and contributing to Drupal developments. I'm assuming a healthy knowledge of PHP is a good start ? What other systems?
Best,
Adrian
Comment #8
adrianedge CreditAttribution: adrianedge commentedI read just the exchange before my last post.
Are you explaining that if I was to patch this module and then try to embed Ustream code in the Embedded Video field that I would loose the formatting and styling that a YouTube embed has once configured?
If so how do I work around this to be able to embed Ustream and maintain the look and functionality of embedding YouTube?
I've already tried to get this done by changing the arrangement in manage fields and using a full HTML text field to embed Ustream, but I loose all the formatting (including the voting, and description space) that's nice about this module.
Lastly how would one go about updating the handlers to include Ustream? And if not this Module, is there another that does this?
Best,
Adrian
Comment #9
jec006 CreditAttribution: jec006 commentedHey Adrian,
No, thats not the case, you just can't depend on the stuff being there across content items if custom embeds are allowed. I think in general this isn't really the route we want to take in vef (i know this is a reversal, but I thought about it more, and jdelaune is right). This definitely doesn't stop you from using the patch. The custom version doesn't' support thumbnails or styles though, so keep that in mind.
In re Ustream, ideally, you would create a custom handler for Ustream using the hooks provided by video embed field - you could then provide styles and a thumbnail yourself. Basically, you provide a couple functions that tell video embed how to find the video and the thumbnail and what options are available.
You can use the handlers in the module for youtube and vimeo as an example and there is documentation in the .api.php files in the module folder.
In terms of doing drupal development, I would say php is the main thing, and then just begin familiarizing yourself with the way that drupal works. Its a huge system with lots of its own standards, styles and ways of doing things.
Comment #10
adrianedge CreditAttribution: adrianedge commentedI tried the patch after installing Cygwin. This is what I got.
/cygdrive/c/xampp/htdocs/LiveIslands/sites/all/modu les/video_embed_field
$ patch -p1 < embed-code-WIP.patch
can't find file to patch at input line 5
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/video_embed_field.field.inc b/video_embed_field.field.inc
|index 22418e7..5a6daaa 100644
|--- a/video_embed_field.field.inc
|+++ b/video_embed_field.field.inc
--------------------------
File to patch: video_embed_field.module
patching file video_embed_field.module
Hunk #1 succeeded at 76 (offset 20 lines).
Hunk #2 FAILED at 94.
Hunk #3 succeeded at 111 (offset 21 lines).
Hunk #4 succeeded at 153 (offset 21 lines).
Hunk #5 FAILED at 162.
Hunk #6 succeeded at 195 with fuzz 1 (offset -17 lines).
Hunk #7 FAILED at 346.
3 out of 7 hunks FAILED -- saving rejects to file video_embed_field.module.rej
patching file video_embed_field.install
Hunk #1 succeeded at 24 (offset -3 lines).
Hunk #2 FAILED at 191.
1 out of 2 hunks FAILED -- saving rejects to file video_embed_field.install.rej
That course of action seemed to have failed so I have since abandoned that approach and have been trying to pick my way through creating custom handlers.
But I am very new to this and having very slow unsure limited or no success. It's been a huge learning curve today, through Cygwin, Patching and now updating the custom handlers. Can I PayPal you some compensation to get help with enabling this for Ustream or at least an example?
Best,
Adrian
Comment #11
adrianedge CreditAttribution: adrianedge commentedThank you for everything jec006. My last questions were a bit hasty. I've persevered through and realized that I was doing a number of things wrong, including trying to patch the wrong file, after taking a better look at the files in the packaged module.
Instead of patching however... I've edited the handles, and got a working handle for Ustream.
One problem.. Drupal forces me to uninstall video embed first to reinstall/update the video embed module ( with replaced updated files which I copied into the video embed module install package).
During the uninstall I have to destroy all the field attachments for the Video Embed module that come configured in my template, to get video embed to uninstall. My only option after doing this has been to reconfigure the video embed content type, but I have yet to get the theme reconfigured properly, with the same spacing. As a result my video screens are in the middle of the page.
Question... How do I find the video_embed_field.handlers.inc and other *.inc files, to replace them with my updated versions, instead of uninstalling the module, and destroying the fields. Can you please point me the *.inc files in the installed drupal module, or to where the code is embedded?
I've attached a screen shot of my spacing problem in case you have any other ideas how to fix it with another approach.
Thanks again,
Adrian
Comment #12
jec006 CreditAttribution: jec006 commentedHey Adrian,
I'm sorry for the delay, it's been a really crazy week.
The best way to go about adding a new handler to video_embed_field is to create a new module and implement hook_video_embed_handler_info.
This would look basically like this:
Once you have told video_embed_field about each of your new handlers, you need to implement the callback functions that were defined for your
provider. They look like this:
Obviously, these are just skeleton functions and should only be taken as a template for building your module, but hopefully that gives you a slightly better idea of what you need to do. I think you probably have most of this as you have a player going, but I figured I'd write it all up so I can put it on the readme or whatever.
As for the player being below where it should be, that seems strange, is it possible that you have an unclosed tag in what your returning as the markup for the handler?
Hope that helps
Comment #13
jec006 CreditAttribution: jec006 commentedI've also added this documentation to video_embed_field.api.php
Comment #14
jec006 CreditAttribution: jec006 commented