Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
If the Media: Youtube module is installed, the Youtube tab comes up for all users who can access the media browser, even if they don't have the "Add media from remote services" permission from Media Internet Sources (which controls whether they can see the Web tab).
Comments
Comment #1
Rob C CreditAttribution: Rob C commentedSomething like this?
Comment #2
RobW CreditAttribution: RobW commentedComment #3
RobW CreditAttribution: RobW commentedSweet. Committed: http://drupalcode.org/project/media_youtube.git/commit/2d3aef2.
Comment #5
David_Rothstein CreditAttribution: David_Rothstein commentedThe title of this issue suggests that the intention is to fully respect the Media Internet Sources permission, but that's not what the patch does. Instead, it's adding it on top of the existing media_access() check, which means that (unlike Media Internet itself) granting the "add media from remote sources" permission isn't enough to grant access to the YouTube tab too.
Also seems a little odd that the permission check is in the view() method, rather than access()...
I noticed this while configuring the module for a site - doesn't affect me too much because for now I have to give the users who will have access to this on that site the other permission anyway (for different reasons), but figured I'd mention it.
Comment #6
David_Rothstein CreditAttribution: David_Rothstein commentedQuick and completely untested patch :)
Comment #7
David_Rothstein CreditAttribution: David_Rothstein commentedI guess it's worth pointing out that this patch isn't very backwards-compatible though (might change who has access to the tab when deployed on a live site).
Comment #8
RobW CreditAttribution: RobW commentedThanks, and nice work. The current dev is getting to be pretty non-backwards compatible, so one more drop won't hurt. If you can re-roll with an empty update hook with a comment about checking your media access permissions that would help. Although the patch looks good I'll leave it up until we get one more person to test and RTBC to avoid another early commit mistake.
Comment #9
David_Rothstein CreditAttribution: David_Rothstein commentedAh, there is a problem actually; I was using media_internet_access() but that function doesn't seem to be available in Media 1.x. So this patch replaces it with an inline permission check. (The "administer files" permission doesn't seem to be available in Media 1.x either, but "add media from remote sources" is so that should hopefully be good enough.)
I also added the update function. Not sure if the way I did it is what you're looking for (a completely empty function with a code comment that will appear in the update listing, vs. something like a drupal_set_message()).
Anyway, the patch is still 100% untested, so yes, it could use at least one person trying it out :)
Comment #10
David_Rothstein CreditAttribution: David_Rothstein commentedTurned out I needed this for a site after all, but the site is running 7.x-2.0-rc1 so here is a patch that applies to that version.
The patch in #9 is still the one to review. However, at least this one will get a little manual testing. I'll report back if I run into any problems, but I don't expect any.
Comment #11
aaron CreditAttribution: aaron commentedI had to re-roll the patch, but it looks great from my end.
Comment #12
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedno longer applies (feb 6 dev)
patch -p1 < *1774266*
patching file includes/MediaYouTubeBrowser.inc
Hunk #1 FAILED at 14.
1 out of 1 hunk FAILED -- saving rejects to file includes/MediaYouTubeBrowser.inc.rej
patching file media_youtube.install
Hunk #1 succeeded at 156 with fuzz 2 (offset -10 lines).
Comment #13
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedComment #14
Devin Carlson CreditAttribution: Devin Carlson commentedA reroll of #11 now that Media Internet 1.x includes a
media_internet_access()
function.Comment #15
Devin Carlson CreditAttribution: Devin Carlson commentedTested #14 with Media 1.x + 2.x and committed to Media: YouTube 7.x-2.x.