Support from Acquia helps fund testing for Drupal Acquia logo

Comments

becw’s picture

Issue tags: +Media Sprint 2011

Adding an issue tag...

becw’s picture

Assigned: becw » Unassigned
Status: Active » Needs review
FileSize
1.76 KB

Here's a patch.

Dave Reid’s picture

Component: Code » Media Browser
idflood’s picture

Status: Needs review » Needs work

The code looks good but it doesn't work well with the restriction by "URI Scheme". I suspect it may be caused by a conflict with the file_extensions params. The "file extension" is required in the "file" field settings and the view alter patch check for !empty($params['file_extensions']).

Another problem is that when you change any of the exposed filter the view_query_filter doesn't seem to be applied.

aaron’s picture

unfortunately, this probably ultimately depends on the monster patch over at the file entity module: http://drupal.org/node/1292382

aaron’s picture

Another problem is that when you change any of the exposed filter the view_query_filter doesn't seem to be applied.

That's because media_get_browser_params() gets set with the URL query parameters, which are different when submitting the type from views/ajax.

aaron’s picture

[10:42pm] aaronwinborn: is there a way to store a value to be saved with the view instance, based for example on $_GET from the referring page, for later use with a hook_views_alter_query() when later calling views/ajax?
[10:42pm] merlinofchaos: aaronwinborn: Wow, uh.
[10:42pm] aaronwinborn: this is for #1293908-6: Respect file type restrictions in media browser library view
[10:43pm] merlinofchaos: aaronwinborn: Man. I dunno. Maybe you could add some .js to put extra data in the .ajax call by modifying Drupal.ajax[base]
[10:43pm] merlinofchaos: That's all I can think of.
[10:43pm] aaronwinborn: that would probably work, thanks merlinofchaos
[10:45pm] davereid: aaronwinborn: probably a better solution would be to somehow alter the exposed filter for file type in the view to only allow the options in the field
[10:45pm] • davereid pulls another idea out of his 'crazy bad idea hat'
[10:47pm] aaronwinborn: davereid: yes, that's what my (currently unsubmitted) patch does. only it forgets by the time we change an exposed filter, because we've lost the original context
[10:47pm] davereid: ugh

aaron’s picture

Status: Needs work » Needs review
FileSize
5.3 KB

This takes care of those concerns, and also locks the type select to the allowed types. Tested against WYSIWYG and the default image field of story.

John Pitcairn’s picture

Status: Needs review » Needs work

Patch at #8 does not work for file field (plain file or remote youtube video) - view library displays no results.

Applied against 7.x-2.0-unstable3+1-dev

Dave Reid’s picture

Assigned: Unassigned » Dave Reid
aaron’s picture

john, did you flush your cache?

tsvenson’s picture

Oki done some testing with this and here are my results.

Patch applies cleanly, Hunk 1 has on offset of 46 though...

Cache needs to be cleared after its applied as well.

It does the trick for fields. It both restrict upload of new files to specified extensions and restrict the choices in the Views library the same.

For the WYSIWYG though it gets half there. It does restrict upload, but still lists everything.

However, if you try to upload a file with a not listed extension, it does complain, but then the Views library shows nothing at all. Just cancel the browser and reopen it again and the files are back.

Seems like something is not going absolutely correct with the plugin.

Dave Reid’s picture

adam-delaney’s picture

subscribing

klonos’s picture

Hey Adam! ...there's a big green button at the top-right corner of every issue's summary that says "follow". You can use that instead ...Stop subscribing, start following ;)

#1307170: Make "Follow" button more findable

saltednut’s picture

Status: Needs work » Needs review
FileSize
4.88 KB

Quick reroll that should apply to 7.x-2.x - work to continue with a manual review.

saltednut’s picture

Manual tests show this working on file fields with using the media widget.

When I open the file browser using WYSIWYG it is open to all file types.

Is there a way to configure the media browser to limit file types on the body field (ie: files allowed to be inserted into body via WYSIWYG)?

raytiley’s picture

Status: Needs review » Needs work

I applied the patch to a clean install of Drupal. When I go to add media there is a javascript exception which causes the media browser to display incorrectly.

The exception is being thrown in: media.browser-views-ajax.js on line 11.

I threw a console.log and found that Drupal.settings.media.browser is an empty array. Don't really know enough about the code to figure out whats going on.

-ray

gmclelland’s picture

Is there a way to configure the media browser to limit file types on the body field (ie: files allowed to be inserted into body via WYSIWYG)?

@brantwynn - I believe that issue is discussed in #1016376: Expose the media types allowed for WYSIWYG variable in the UI

Dave Reid’s picture

Assigned: Dave Reid » Unassigned
Dave Reid’s picture

Status: Needs work » Fixed

I actually figured out a much easier way to do this. I added a tag of 'media_browser' to the default views and use hook_query_media_browser_alter() that will automatically apply filtering to any query using that tag. We should split out the conditional removal of views filters to a separate issue if we want it still.

This has been committed and fixed along with #1293660: Remove old media browser library plugin with http://drupalcode.org/project/media.git/commit/9bce5cc. Thank you everyone for your work on this issue! We finally got rid of a bunch of old code!

Status: Fixed » Closed (fixed)

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

jay187’s picture

Sorry but I can't get this working under my current install. I'm using media-7.x-2.x-dev and I have to FileFields with Media Widgets on the profile page. Both are allowing different file types but unfortunately if I open the MediaBrowser I can select any file type. So there's no restriction.

Cyberschorsch’s picture

Status: Closed (fixed) » Needs work

This is still an issue.

Devin Carlson’s picture

Taken from #1824304: Doxygen for media_query_media_browser_alter is incorrect, the attached patch fixes the outstanding library restriction issues and adds tests for the various restrictions (scheme, file type, extension).

Status: Needs review » Needs work
Devin Carlson’s picture

Status: Needs work » Fixed

Tested #25 with an existing site and confirmed that media browser restrictions were enforced properly: scheme, file type, extension (not applicable to remote files).

Committed #25 to Media 7.x-2.x.

almc’s picture

Got the patch able to apply only partially, so triggered re-test:
25: correct-media-query-media-browser-alter-1293908-25.patch queued for re-testing.

The last submitted patch, 25: correct-media-query-media-browser-alter-1293908-25.patch, failed testing.

Status: Fixed » Closed (fixed)

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