Hi there,
I would like to report a bug of this module.
I used this module with media 2.x, media gallery 2.x and plupload integration.
With this modules activated was impossible to upload video file with extention mp4, mov... ecc....
Multimedia upload is deprecated.
So we need to use file entities.
But this module have this file extention allowed

define('FILE_ENTITY_DEFAULT_ALLOWED_EXTENSIONS', 'jpg jpeg gif png txt doc docx xls xlsx pdf ppt pptx pps ppsx odt ods odp');

declared in the beginning of the file_entity.module.
So I changed this line like:

define('FILE_DEFAULT_ALLOWED_EXTENSIONS', 'jpg jpeg gif png txt doc docx xls xlsx pdf ppt pptx pps ppsx odt ods odp mp3 mov mp4 m4a m4v mpeg avi ogg oga ogv wmv ico');

and my problem was solved.
Hope it can help someone!
And sorry for my bad english!

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ParisLiakos’s picture

Title: media entity deprecated, but file entity do not know » Make allowed extensions configurable
Category: bug » feature
Status: Fixed » Active

We should make this configurable..maybe turn it to a variable?

mrfelton’s picture

Status: Active » Needs review
FileSize
1.2 KB

Simple patch to switch the setting over to a variable.

mrfelton’s picture

Oops, last patch blew away the array.

mrfelton’s picture

Ok, lets try again. Should have been a space separated string.

ghyupdc’s picture

Oops, last patch blew away the array.

ParisLiakos’s picture

Status: Needs review » Reviewed & tested by the community

thanks..it makes sense

ParisLiakos’s picture

Status: Reviewed & tested by the community » Fixed
mrfelton’s picture

As a side note, media.module references FILE_DEFAULT_ALLOWED_EXTENSIONS and also needs to be updated to use the new variable.

ParisLiakos’s picture

Status: Fixed » Closed (fixed)

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

iva2k’s picture

Status: Closed (fixed) » Active

Adding a variable makes it one step closer to being configurable, however, without any UI to actually change the value of this variable the fix is incomplete.

iva2k’s picture

Status: Active » Needs review
FileSize
1.63 KB

Here's a patch that adds variable_ui hooks for a "free UI".

ParisLiakos’s picture

+++ b/file_entity.variable.incundefined
@@ -0,0 +1,31 @@
+//    'path' => 'admin/config/?',

nah, just remove it..there is a ui in variable module, we dont need to provide another one

iva2k’s picture

I agree that making UI for every silly variable is a waste of time. I think variable/variable_ui should be part of the core.

Do you want me to roll another patch to remove this comment? Or just omit it from commit?

ParisLiakos’s picture

i will just ommit it;) but that said..cant remember if file_entity owns any other variable, so we register it...if we have more, lets put them in there since we are implementing the hook

ParisLiakos’s picture

Ok, lets register those too:

  • file_entity_admin_files_limit
  • file_entity_cron_last

they are both integers

Dave Reid’s picture

What is the plan when we start porting the module to Drupal 8 and inclusion in Drupal core? This hook would not be appropriate to remain since it's not included in core. Would this implementation move to variable module? This should add a @todo for a plan. Or better yet, likely it should form-alter the 'File system' settings page to add this variable, which would be a better fit for core.

ParisLiakos’s picture

in d8 there is CMI, so i am not sure what happens to variable module..for core inclusion, this should be removed, like any other integration with non-core modules that are in file_entity

slashrsm’s picture

Priority: Normal » Major

Another thing that should be considered here is the fact that Media provides very similar variable and even UI for it's configuration. That is VERY confusing, since one easily assumes that this variable effects file_entity also, modifies it and nothing happens. I believe that this could be VERY frustrating.

I'd consider this a major UX flaw...

shenzhuxi’s picture

I think 'file_entity_default_allowed_extensions' should be set based on the file types declared by hook_file_default_types.

ParisLiakos’s picture

or allow everything..

slashrsm’s picture

Allowing all types is generally not a very good idea.

aaron’s picture

Status: Needs review » Needs work

I agree that allowing all types would not be good. Per #17 and the issues previous to that, I am setting this issue to "needs work". For #19, I think that that should be an issue for the Media module.

iva2k’s picture

@aaron
"needs work" - can you clarify what needs to change in #12? Is it only #13 note or something else?

aaron’s picture

I would add the variables noted in #16 and move it to the file settings form of core mentioned in #17. Yes, I would also take care of #13.

iva2k’s picture

Status: Needs work » Needs review
FileSize
2.33 KB

I re-rolled patch #12 to include #13, #16. The only change to file_entity module is an addition of new file file_entity.variable.inc (and a pointer in .info file)

As to a request of a plan in #17, when time comes, file_entity.variable.inc file can be moved to variable/includes, next to other include files addressing core variables.

Personally I don't like CMI and think that variable module hits the nail on the head with 99% of module configuration needs. I'm considering to strip settings UI in all my modules and make variable module take care of all that. Declarative programming rocks!

Status: Needs review » Needs work

The last submitted patch, file_entity_variables-1836266-26.patch, failed testing.

iva2k’s picture

.info file has changed since #12... Updated patch from #26.

slashrsm’s picture

Status: Needs work » Needs review

Looks nice. I would add an entry in README.txt to inform people that they can use variable.module to edit configuration that is not exposed in UI otherwise. Would also add this to online docs after this gets committed.

ParisLiakos’s picture

Status: Needs review » Needs work

i dont think we should add anything to the .info file btw
needs work for a README entry as well

mailfox’s picture

iva2k’s picture

@mailfox
Can you post a screenshot of the variables page for the settings?

czigor’s picture

Status: Needs work » Needs review
FileSize
1.74 KB

Info file entry is useless, I have removed it. Otherwise the patch is practically the same as #28.

There's no README in this module.

aaron’s picture

Status: Needs review » Reviewed & tested by the community

This looks good to me.

Devin Carlson’s picture

Assigned: Unassigned » Devin Carlson
Status: Reviewed & tested by the community » Needs review
FileSize
944 bytes

Even as a big proponent of the Variable module, requiring a user to figure out that they have to download and install a submodule (Variable Admin) of a primarily API module (Variable) that has a significantly smaller user base than Media just to configure a basic setting (what file types users can upload) seems like a bit much to me.

Attached is a simple patch to add the default allowed file extensions to the top of the File system page per Dave Reid's suggestion in #17.

I think that this is ready to go and will commit in the next few days unless anyone has any objects/concerns/improvements. :)

Devin Carlson’s picture

Status: Needs review » Fixed

Committed #35 to 7.x-2.x.

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

Issue summary: View changes

I've changed the deprecated entity