CVS edit link for lavagolemking
I am applying for a CVS account to publish and maintain a new Drupal module written by the Electronic Frontier Foundation. I have been working with Tim Jones from the Electronic Frontier Foundation on a module called MyTube . MyTube was written in 2008 as a way of serving Flash content or other remote embeds without putting users' privacy at risk with things like Flash cookies or remote referrers . Unfortunately, the project had not been worked on since early 2008, was not very configurable, and only worked for Drupal 5 (what EFF uses). As a Drupal 6 administrator at opensource.osu.edu, who was interested in running it on our website, I picked up the project, fixed a few bugs, made some improvements, made it compatible with Drupal 6, and converted it from using nodeapi into an input filter for flexibility purposes. I then backported my changes to Drupal 5 and passed the code onto EFF . The current version is located at http://opensource.osu.edu/mytube and also in our git repository at https://opensource.cse.ohio-state.edu/git/swaneybr/mytube.git .MyTube is an input filter (it used to use hook_nodeapi in original version) that replaces all embed, object, and iframe tags with locally-hosted thumbnails that, when clicked, run a JQuery script that swaps the thumbnail out with the original embed code. Effectively, until the user clicks play, no remote request is made to the remote site hosting the Flash (or whatever else) content, so the user cannot receive referrer headers or Flash cookies until opting in. Below each thumbnail is a (configurable) disclaimer warning users about the privacy risks in loading the content. The disclaimer includes a (configurable) link to a page about privacy concerns for Flash content, which by default goes to https://www.eff.org/deeplinks/2008/02/embedded-video-and-your-privacy and also has a link to the source of the video, with the link text being the top-level domain. This ensures visitors can know what they are clicking to load before they load it. In summary, MyTube can be thought of as a site-wide Flashblock . To alleviate the NoScript-esque inconvenience this may pose to some users, especially those who don't understand or care about the privacy implications of some video-hosting services, MyTube also automatically modifies embed code for a number of websites so it autoplays when loaded, and then locates and downloads the thumbnail for that embed, so the appearance and experience is roughly the same as if the embed code itself were pre-loaded with the page. See an example at http://opensource.osu.edu/node/299 . In this way, unlike with NoScript and the like, the user only needs to click ONCE to play the video for many video-sharing websites (I am continuing to add more as I find them). If the source is not supported, a default thumbnail is displayed instead, and all thumbnails (default or not) are overlayed with a play.png symbol. In such unsupported cases, users can also add a thumb= atttribute to the embed code to specify their own thumbnail. Wherever the thumbnail may be from, it will automatically be resized to the size of the embed, or (if so-configured) the embed resized to an administrator-set width and height. Currently, thumbnails and autoplay are supported for YouTube, Metacafe, and Vimeo, and autoplay alone is supported for a handful of other locations.
What I hope to accomplish by submitting this to drupal.org is 2 things: (1) I want MyTube to automatically find updates, especially if (god forbid) there were some security vulnerability in our code, and (2) more users will know about and feel comfortable deploying MyTube. If it's adopted on a large scale it will be highly beneficial to end users who are mostly unaware of the massive amounts of data user-profiling and marketing companies are collecting about them, but potentially also beneficial to some companies (such as security vendors) as a PR move to show they care about your privacy. While naïve visitors will not notice anything happening, the ones who know and care about online privacy will appreciate that visiting a page with a single YouTube embed will not be added to a central database somewhere. I have spoken with the technical staff at EFF about hosting it here, and Tim Jones has agreed to sign on as a co-maintainer for the project, while I plan on being the primary maintainer. MyTube is licensed under the GNU General Public License version 2 or 3 (currently a license file is included for both, but I can remove them if it's against your terms). If you would like to contact Tim, let me know; his primary e-mail address has changed since leaving EFF. If you wish to speak with someone at EFF about the project, their main contact is now Chris Contelini , but he has not contributed any code to MyTube.
I shamefully admit I am new to Drupal development, or web development in general, but to the best of my knowledge the code submitted complies with your security best practices and coding standards, and API calls are made in place of standard PHP libraries wherever possible. As far as I know, there is no other server-side protection that does what MyTube does, not just within Drupal, but anywhere. There are a couple of end-user tools in the form of add-ons for Firefox, including Flashblock and NoScript. While these are more secure for security-conscious users, and they work for all websites, MyTube will work for all** visitors of the website regardless of what security plugins they may or may not have.
For security issues, MyTube does not currently access the database in any way other than the variable_get function. The primary security concern is that it is an input filter, and should handle user input in a safe and careful manner. While the removal of unsafe or potentially unsafe content is normally handled by the HTML Filter, users must technically be allowed (either by disabling this filter or whitelisting the appropriate tags) to submit embed, object, param, or iframe tags. Therefore, I consider any instance where such tags can pass through and automatically load (in any fashion) a security vulnerability. There has been one such vulnerability in the original version, which I can describe in more detail on request, but calling the HTML Corrector before MyTube processes the code will mitigate the issue (MyTube therefore calls said filter before invoking its own methods). Relevant embed tags can be selectively allowed or disallowed, and MyTube will process only what the user is allowed to use. In the event of a security vulnerability, if a fix is not immediately available, it can be mitigated simply by removing embed/iframe/object/param tags from the HTML Filter whitelist, and MyTube will pose zero threat. Simply disabling MyTube without revoking this access will allow everything to pass through unfiltered, but disabling MyTube will not create any benefit (or problems for that matter) if the tags are also removed from the whitelist. The only other issue I am aware of, which is minor but I'm considering mitigating anyway, is that in rare isolated cases MyTube has failed to detect where a video source is located, and therefore not display a link in the disclaimer. I'm considering adding code to handle these embeds differently, warning users that the code appears malformed and may be malicious, but this would probably fall more in line with a new feature. MyTube was originally written by Tim with the intention that it would be run in a tightly-managed environment, where everyone posting content trusts one another (or a single-user blog), but I hope to make it safe enough so administrators can safely allow many relatively anonymous users to post embed content without it every automatically launching for anybody without their explicit per-case consent.
**Note: There is an IE Message setting, which displays a warning to users of Internet Explorer about possible incompatibility. I have in the past month noticed that Internet Explorer is actually loading the videos, so it may have been a security setting. The feature is still there in case of problems and an administrator wants to explain, and does nothing if left blank, but I am no longer able to reliably reproduce the error in IE. In past testing, IE has failed to load any kind of embed/object that wasn't pre-loaded with the original page. This including loading the SWF directly, or any kind of adding of the object through, for example, ELEMENT.innerHTML. Such a setback would make it impossible for MyTube to function, no matter what the implementation, but has not been seen for over a month. Furthermore, no such issue has been encountered with any other browser that supports javascript.
Comment | File | Size | Author |
---|---|---|---|
#40 | mytube-d6.tar_.gz | 29.91 KB | l@va |
#38 | mytube-d6.tar_.gz | 40.49 KB | l@va |
#31 | mytube-d6.tar_.gz | 29.99 KB | l@va |
#31 | Screenshot.png | 163.24 KB | l@va |
#30 | screen01.jpg | 649.69 KB | wadmiraal |
Comments
Comment #1
l@va CreditAttribution: l@va commentedComment #2
apadernoHello, and thank you for applying for a CVS account.
We just review a theme/module per applicant; this means also a single Drupal version of a module/theme. Let us know which one you want reviewed.
Comment #3
siliconmeadow CreditAttribution: siliconmeadow commentedThanks for the comprehensive write up. I've not considered this issue before. I'm happy to test the D6 version. Considering the impending release of D7, and the fact that the module has only been in use on the EFF site (and few others?) there may be little enthusiasm from the community to test the D5 version.
Comment #4
apadernoComment #5
wadmiraal CreditAttribution: wadmiraal commentedHya.
You might add a field to the settings form: "Enter size of play.png" (defaults to 30x30).
The version will be added by CVS.
dump()
function is not necessary :-). Try usingdsm()
(Devel module) for debuging.Kind regards,
Comment #6
l@va CreditAttribution: l@va commentedSorry my reply took so long; I was expecting to get e-mailed about any coming updates, just like about applying in the first place. If I can only have 1 version reviewed, then definitely go with D6. Most of my testing was done on D6; that's what we run at the Open Source Club, and I believe you're going to discontinue support for D5 in the near future.
Comment #7
l@va CreditAttribution: l@va commented1. Coder is now returning zero messages. Could you please provide me with some examples of where I failed to adhere to proper standards, if they remain?
2. It wasn't there to encourage hacking the code (thus not found in the documentation anywhere). Changing the play icon seemed a fairly unlikely scenario to me, but I could stick it in the preferences instead. You'd only want to change it if you were swapping out the included play.png file, and when you saw it off-center, that's where you'd change it. I spent a while just looking over the code figuring out what it did, so I wanted to make understanding that easy, but now that you mention it I could see both ways. I'll probably add a resizeable play button as a feature eventually just because you mentioned it.
3. Updated per your specifications.
4. All license files have been removed.
5. I'll just take dump() out. It was there in the original version, and I didn't know what it did, but figured I should leave it alone in case something in Drupal depended on it.
6. Could you show me an example of this? I've never seen anything like that happen before. It's not happening on our live site, where the exact same code you're looking at is running (minimal modules), it's not doing that in any of our testing sandboxes, and it's not doing that on eff.org, which is running a slightly older version of this (no metacafe.com). I also routinely look at the page source when debugging, so I'd probably notice something like that happening. This isn't normally how I address errors, and I don't mean offense to suggest it, but are you sure it's not something goofy with your environment?
Comment #9
l@va CreditAttribution: l@va commentedJust realized by looking at other people's requests that I should mark this as "needs review" myself. Also, is there a reason the name got changed to "http://www.wholesalenflstore.com/"? I didn't change it myself, and the page says it was last updated 3 hours, 40 minutes ago. [Changed back to what I think it was on submission.]
Comment #10
l@va CreditAttribution: l@va commentedwadmiraal: Having read through your coding standards and searched through my code at each paragraph of reading said standards, I believe I have updated my code (new version attached) to be strictly 100% compliant with your coding standards wherever possible. There is one isolated case where it would rename a variable if I concatenate it within the string, but I suspect that's not a major problem for you. My .info file is updated per your specifications, the license file is removed, and the dump() function is deleted. Aside from your last point, which I cannot seem to duplicate no matter what I do, I believe I have met all of your criteria. Can I please have another review?
If you are still getting the .module dumped into all your pages, could you tell me what other modules you have installed? At this point, aside from working on your optional suggestion (which will come in time) I don't know what else to do.
Comment #11
wadmiraal CreditAttribution: wadmiraal commentedGood job :-).
You should remove:
from the start of the
.module
file.Add:
at the very top of your
.module
file for CVS.The problem did not occur again. Your module is working as advertised (as far as I can see). I say this is RTBC :-).
Comment #12
apadernoThis is a partial review.
t()
.t()
is a literal string; any dynamic string (including the concatenation of strings, such as'first string' . 'second string'
and"first string $second_string"
) would not allow the argument to be put in the translation template.The reason is that the translation template file is created by a script that extracts the string passed to
t()
by scanning the module files, which are analyzed as plain text. The script is then not able to get the value of a PHP variable, or any dynamic value passed tot()
.t()
; avoid usingl()
witht()
as int('See the !settings-page.', array('!settings-page' => l(t('settings page'), 'admin/module/settings')))
.As reported in the documentation:
<b>
should not be present in XHTML output, which is the Drupal output.Avoid concatenating translated strings, but pass the full sentence to
t()
. HTML tags should be avoided inside the strings to translate, but the code should not become as the following one:It is JavaScript, not javascript.
As
!embed
< could be confused with at()
-placeholder, it should be better to use <code>!</code><code>embed</code>.CSS styles should be applied through HTML classes, or IDs.
JavaScript code should go in a separated file. It is possible to translate the strings that are output from JavaScript code through
Drupal.t()
.$_mytube_index
.hook_uninstall()
to remove the Drupal variables it defines.Comment #13
wadmiraal CreditAttribution: wadmiraal commented@kiamlaluno
Wow...sorry about that...I missed a lot it seems.
Comment #14
l@va CreditAttribution: l@va commented1. This is now fixed. I was not sure if by schema descriptions you meant descriptions below the settings, so those are still using
t()
for now. If you want the#description
parts of$form
inmytube_admin_settings()
to not uset()
either, let me know.2. Fixed to the best of my knowledge throughout the module.
3. Also fixed.
4. All instances of
<b>
tags were replaced with<strong>
. I could not reasonably completely remove all HTML tags and completely avoid concatenatingt()
strings, but I did my best to accomplish the latter.5. Fixed.
6. There was at least one case where, in testing, the class/id CSS was not applied for reasons unknown to me. Furthermore, the CSS is defined on either a per-video or per-setting basis. In this case, the inline CSS remains. Here is a snippet:
Also, for the section of escaped embed code you quoted, it would seem (in my opinion) redundant to define a separate ID for each color where I'm highlighting sections of the code and add those into a CSS file, just for a single instance where they appear, so I still have it in CSS. If you wish, I will put them into the CSS file and remove the inline CSS, but inline CSS seems most appropriate here.
7. What I was looking here for was not a translation, but configurability. Earlier on in testing, Internet Explorer was not loading any kind of object (Flash or not) that was not preloaded with the page. It didn't matter whether a .swf was loaded in a popup, whether I used
ELEMENT.innerHTML
, or whatever; unless it automatically loaded when the page loaded the objects would not load. This has not been the issue lately, and I cannot reproduce the issue anymore, but I had the setting there just in case. What would happen here is that if an administrator found their videos weren't loading in Internet Explorer, they could explain the problem to IE users in an alert dialog, instead of just inexplicably failing, and let the user know they can still load videos in another browser of their choice. Since I'm no longer able to reproduce the issue, and since I have no idea how to make this setting both configurable and appear in a static js file, I removed that feature.8. All global variable names are now prepended with "
_mytube
", or "_
" if they already started with "mytube
".9. All the constants I can think of are now in all caps. If any are not, then let me know because I missed it. I think functions and (non-constant) variables were already compliant.
10. Added a
.install
file with an implementation ofhook_uninstall()
. Do I also need to delete the sites/default/files/mytube folder where thumbnails are stored? I am not sure if unlink would work on a folder.Comment #15
l@va CreditAttribution: l@va commentedComment #16
apadernoBy schema descriptions I mean the ones used in the database schema, the one declared from the
hook_schema()
hook.Comment #17
l@va CreditAttribution: l@va commentedAh, thanks for clearing that up. MyTube doesn't implement
hook_schema()
because it never accesses the database (except invariable_get()
and nowvariable_del()
); it has no need to use the database as an input filter. Do I still need to add an implementation ofhook_schema()
for some reason?Comment #18
apadernoNo; a filter doesn't normally use databases.
Comment #19
l@va CreditAttribution: l@va commentedSo, am I missing anything?
Comment #20
l@va CreditAttribution: l@va commentedAnybody going to review this?
Comment #21
l@va CreditAttribution: l@va commentedStill waiting...
Comment #22
apadernoComment #23
sirkitree CreditAttribution: sirkitree commentedThis has gone back and forth enough to demonstrate that the author is a responsive and responsible contributor IMHO. Please grant a CVS account based on this fact - not deny him any longer based on the fact that the code is not perfect. He's certainly demonstrated that he is willing to improve and this should be done in his project's issue queue, not in this thread any longer. Further deliberation only serves to frustrate and confound new contributors. Marking RTBC.
Comment #24
siliconmeadow CreditAttribution: siliconmeadow commentedComment #25
apadernoNothing has been fixed, as the CVS application has not been approved, yet.
Comment #26
siliconmeadow CreditAttribution: siliconmeadow commentedSorry - that was a cheeky attempt for me to see if the application would be approved automatically if someone other than the applicant changed the status to fixed. I was feeling sorry for him being put through this purgatory and in agreement with sirkitree's comment.
Comment #27
siliconmeadow CreditAttribution: siliconmeadow commentedOthers have scoured through your code technically, and your willingness to address each and every point raised shows good attention to detail. This review will be primarily about the way the module is presented to the end user. It's much more subjective and open to interpretation, of course, but there are plenty of guidelines for documenting and "getting the tone right" which you can find here on d.o, although you might have to dig around a bit. The links mentioned will drop you into the book where you can use the navigation on the right-hand side to gain further understanding.
Filter module as a dependency - no need to specify as it's a required core module. Not a show stopper though.
Readme file - Have a look at the first recommended guideline here:
http://drupal.org/node/161085
which has a link here:
http://drupal.org/node/447604
I needed a bit of a refresher of what I was evaluating and your README.txt file was the first place I went to this morning. I was on a train and my internet access was a bit patchy. You mention three URLs for more info, but a module's README.txt should give the end-user enough detail to get started. It doesn't say anything about installing (even if it's the standard way), permissions, that it's a filter module, nor what the module's purpose is.
I don't know what more info I'm going to get by going to the links you mention in the README.txt file. Should I go to them in the order they appear in the URL? Put yourself in the user's shoes - they shouldn't have to make too many decisions at this stage. Let them know what they can expect it to do. Also, what it doesn't do - perhaps some people attempted to use it for the wrong purpose and after some time, discovered it wouldn't work for the purpose they had hoped. You could probably copy and paste some of what you'd posted in the beginning of your application to get started.
The help page appears to provide much of the detail you should have in the README.txt file. Perhaps some of this, too, could be in the README.txt file?
Proofreading the help as displayed
Last sentence of first paragraph of help should read:
Third paragraph - should probably be "customizable" instead of "customizability"?
The first paragraph after "Input format guidelines" seems to repeat the first sentence of the third paragraph, but much more awkwardly. Could it be deleted? Maybe the first paragraph of "Input format guidelines" could be:
The previous paragraph I suggested is just an idea, and upon reflection, it seems to repeat a lot of information from your previous paragraphs. Perhaps you and Tim Jones could collaborate on revising the what, why and how text in both the README.txt and the inline help for clarity and conciseness?
You might find it helpful to follow some of the stylistic guidelines the Docs team has outlined:
http://drupal.org/node/700542
In the same section (the "Contribute to documentation" of the "Getting involved guide") - is an example of the care and attention the community has put into accuracy of terminology. I notice in #12 that kiamlaluno picked up on your case insensitivity of the word JavaScript.
I hope this helps and doesn't cause a huge amount of work for you, especially since it seems you've jumped through every other hoop by now.
Comment #28
l@va CreditAttribution: l@va commentedFixed. I added it because it was specifically required; there is a security vulnerability it mitigates by calling HTML Corrector's filter. This is more an issue with Drupal 5, since HTML Corrector is not included by default.
Added heavy documentation to the README.txt file and added an INSTALL.txt file. Also, I added an FAQ section to README.txt and heavily emphasized installing the PHP5-CURL library in the INSTALL.txt.
Fixed.
Fixed.
Note: The portion below "Input format guidelines" is a preview of what the regular user will see when they click on the "More information about formatting options". The
hook_help
displays the output ofhook_filter_tips(0, 0, TRUE)
to be helpful, so the administrator can see what the user sees and what exactly the filter does. I cannot explicitly add a link to the filter tips from the help page, and this is why you have some redundancy (some of the details are explained to users).In that case, I simply didn't know. What you found were typos and grammatical errors in my writing, and I think they're all fixed in this version.
By the way, I do appreciate your above attempt to get this approved, as well as your input. This is getting old, and I'm getting tired of waiting around here without even being told why the application is not approved. I suspect there are at least a few who don't quite appreciate it as much, if you know what I mean...
I am not sure what else is needed here to get MyTube hosted on drupal.org.
Comment #29
siliconmeadow CreditAttribution: siliconmeadow commentedIndeed it's frustrating when others don't act as quickly as you'd wish and don't explain why. We all make mistakes and have communication breakdowns though, so please bear with us. I'm certain there is nothing personal going on. Drupal does appear to be growing exponentially and there are over 7k modules now. I suspect a good number are poorly maintained, if at all. And there are hundreds of duplicates. There are a number of balancing acts going on simultaneously and being orchestrated by a diverse population. kiamlaluno and the rest of us have a duty in the approval process to ensure CVS account holders have the skill to do produce quality code and documentation. Of course none of us can know everything, so we hope you know where to find the stuff you need on d.o, and show the initiative to find it.
Think of the application process as a learning exercise and hopefully a fun one at that. There is a wealth of information here to help you grasp the standards expected. Conversely, there is quite a bit of out-of-date information as well as documentation and code which does not reflect the standards the community aspires to. It appears as hypocrisy, but hopefully benign. I do encourage you, in any case, to immerse yourself in the documentation so you get a feel for what this place is about. You do admit that your new to Drupal development, afterall.
It was a glaring oversight that no one picked up on the lack of mandatory information in the README.txt file. I believe that omission on it's own has stopped applications.
The grammar issue is something not to gloss over. A significant number of Drupal users do not have English as their mother tongue.
Anyway, I guess it's time for me to stop pontificating.
"Technically-incline" and "customizability" have crept into the README.txt. Doh!
Comment #30
wadmiraal CreditAttribution: wadmiraal commentedHi lavagolemking,
Sorry I didn't get back to this review, was very busy. I won't go through the copywriting of the README.txt file.
So I downloaded the new version, activated it and... It outputs all the content from your .module file before the HTML (similar to #5). This is a fresh install of Drupal 6.20 we're talking about (Garland theme). Tried it on another install (Drupal 6.20 as well), and the same problem occurs. Now this is with different settings then #5, and not on the same computer (all this is tested locally). I tried deactivating every module except the core "Menu" module...and it still happens. And I've never encountered this with other modules, so it definitely has something to do with yours.
Can anyone else try to install this module and see what happens ?
I don't know why this only happens to me, but admit that it is weird.
Comment #31
l@va CreditAttribution: l@va commentedFixed. I guess I made my edits a little out of order.
I still cannot reproduce this issue on a clean installation, just downloaded from drupal.org and with the database just created. Screenshot attached. A couple things though:
I did find and fix an unrelated issue though. At some point in these changes, and I have no idea when but I know I ran into and fixed this before, MyTube stopped displaying thumbnails for sites with Clean URLs disabled. I fixed it to again strip the "?q=" from the IMG SRC attribute.
Comment #32
wadmiraal CreditAttribution: wadmiraal commentedFOUND IT !!
I was taking my shower this morning and suddenly it hit me: PHP opening tags ! You used
<?
opening tags, but you should use<?php
, as not all servers support short tags.Comment #33
siliconmeadow CreditAttribution: siliconmeadow commentedI've tried it on two sites so far and can't duplicate the problem.
Comment #34
siliconmeadow CreditAttribution: siliconmeadow commentedAh - good find!
Comment #35
siliconmeadow CreditAttribution: siliconmeadow commentedAdmin menu. You'll love it: http://drupal.org/project/admin_menu
Comment #36
siliconmeadow CreditAttribution: siliconmeadow commentedA couple of things I found in use:
warning: curl_setopt() [function.curl-setopt]: CURLOPT_FOLLOWLOCATION cannot be activated when safe_mode is enabled or an open_basedir is set in /path/to/mytube.module on line 754.
safe_mode is off, but open_basedir is set. A minor problem?
Comment #37
apadernoI think that the coding standards also report to not use the short tags.
Comment #38
l@va CreditAttribution: l@va commentedI thought that's what I was supposed to use, but I guess I misunderstood. The tags are now back to <?php.
Depends. Are you still getting thumbnails for YouTube/Vimeo/MetaCafé embeds? If so, nothing is wrong. CURL is only used for downloading thumbnails, so if it gets called and you don't get a thumbnail from it, then you have a major problem (usually you'll get the black, rounded default.gif). If you are getting the thumbnails, then it's just alerting you that something isn't ideal between your setup and the code being run.
Unfortunately, my expertise related to CURL is a little limited here, and I don't know what open_basedir is for. This portion of the code is more or less copied/adapted from what Tim Jones had, but with the exception of my next point, it sounds like things are working for you. I thought the particular line of code being mentioned was required before starting a CURL session, but if it causes a problem other than warnings let me know.
This one is a serious problem. There is a known and documented problem with YouTube playlists; neither Tim nor I had taken them into account when working on this, and YouTube embeds were always re-written in a very specific format when adding the custom YouTube parameters. It depends on the embed URL being of format
youtube.com/v/
Naturally, this broke YouTube playlists (which use/p/
), so I added a TODO tag. It seems I was unaware they were using a new embed code (/embed
), and furthermore this new embed didn't make the first parameter start with an & instead of ? (unlike the legacy embeds). The new iframes should now be accounted for, and thumbnails should work.This is very surprising to me. There is no code to handle this new format, of which I am just now becoming aware. MyTube will try to find the
youtube/v/
portion in the URL, and take the video ID from right after it to find the thumbnail. Since the/v/
string doesn't exist, it will fail, and fall back to the default.gif instead. Are you sure you weren't just talking about that black background with rounded corners? If so, that problem should be fixed now, and when the code is run again (clear Drupal's cache) the thumbnail should be downloaded and displayed.Comment #39
siliconmeadow CreditAttribution: siliconmeadow commentedIt's in the coding standards. You've missed out the the full <?php opening tag in the .install file:
http://www.php.net/manual/en/ini.core.php#ini.open-basedir
I've tested a few videos from YouTube and they seem to be working fine now.
Yes, sorry - it was the black background with rounded corners that appeared. And now, as you say, this version of the code displays the thumbnails directly from YouTube. And remind me to not piss off the person who has thumbs that large.
I also took the opportunity to try each of the other site that the README.txt file are supported, with the exception of myspace, as I don't have an account, nor could I get to mtvnservices.com to load.
I applaud your perseverance! I highly recommend you start immersing yourself in the Drupal documentation - particularly coding standards, CVS account holder docs so you're prepared for when your account is approved.
Comment #40
l@va CreditAttribution: l@va commentedAlright,
open_basedir
correctly now (which I might not), then it restricts which files can or cannot be opened by the PHP interpretor. This should not be a problem for MyTube unless PHP cannot open files in the location specified byfile_directory_temp()
orfile_directory_path()
. MyTube will download a file (or possibly multiple files, as in the case for Vimeo) to the temp folder, then when all is said and done and it has a thumbnail, it moves the file over tosites/default/files/mytube/
and returns a path to it. If it fails somewhere, it returns a path to the default.gif. This means that if you are denying CURL the ability to open folders in one of these locations, then you'll get nothing but default.gif. It sounds like you might have changed this setting, since the error indicates CURL was restricted to MyTube's own folder.Could you clarify this for me? Large images (width/height) should not affect anything because MyTube resizes them to the same width/height of the embed. The thumbnails are generated by the video sharing sites, and generally pretty small in terms of file size so you shouldn't normally run into problems even if you download a lot of them.
No need for an account on MySpace; just check Google Video with search terms
site:myspace.com
. For your reference - http://opensource.osu.edu:7280/node/402Comment #41
siliconmeadow CreditAttribution: siliconmeadow commentedExcellent - thanks :-)
Sounds like you do. Plesk users in particular will likely see
open_basedir
warnings, due to its effectiveness in restricting where ill-informed end users install php files for execution. You can imagine a web site novice downloading a freebie set of php scripts and being completely unaware of the malicious purpose of the code. If you get a lot of open_basedir items in your issue queue, let me know and I'll write up some instructions for what they (or their hosting company) need to do to make a more friendly vhost.conf file. The vhost.conf file is used to tweak non-standard apache conf features for vhosts in Plesk on a per-domain basis.It was a completely lame attempt at humour. Please ignore the comment, and sorry I diverted your attention with it.
Thanks.
I can't see any reason for your account to not be approved. I hope someone with the power to do so will approve it rather sharpish. Good job, and once again, thanks for your perseverance.
Comment #42
siliconmeadow CreditAttribution: siliconmeadow commentedHi Brian,
I hope you are still interested in having your module hosted on d.o. I speculated in the past, and still believe, the delay is due to the "Great git migration". You might want to read #1065558: How to handle migration from CVS Applications to Project Applications (new Git workflow) in the infrastructure queue.
Cheers,
Richard
Comment #43
l@va CreditAttribution: l@va commentedStill interested, and still watching.
Skimmed over linked document, and I'm not quite sure what this means for me, other than more waiting. Do I need to run this in some kind of CVS/GIT sandbox before it's published? Do I need to re-do the application as a GIT application? Do I need to re-apply for a GIT account, already having applied here?
Comment #44
l@va CreditAttribution: l@va commentedI have created a sandbox project for this module:
http://drupal.org/sandbox/lava/1074760
I have created a new issue at http://drupal.org/project/projectapplications based on the new instructions about applying for permission to create full projects.
Link to the issue: http://drupal.org/node/1074792
Comment #45
apadernoComment #46
apaderno