Currently on File field setting forms, we have both 'Allowed media types' and 'Allowed file extensions'. These settings conflict with each other. We discussed changing this into a radios with three different options:

( ) Allowed file types
  [ ] Audio
  [ ] Image
  [ ] Video

( ) Allowed file mime types
  [ text field to enter a space-separated list of mime types ]

( ) Allowed file extensions
  [ text field to enter a space-separated list of file extensions ]

Selecting either an option in allowed file types or file mime options should automatically pre-fill in the list of allowed file extensions since we need to ensure that the file_extensions instance setting is always available (to allow people to switch back to alternate widgets).

We then need to ensure that we properly validate against these options in the media browser. Also, these options should validate so that *something* underneath the selected radio button is also selected. For example, the user cannot select 'Allow specific file types' and not click one of the file type checkboxes. Or select 'Allow specific file extensions' and not enter any extensions in the text field.

CommentFileSizeAuthor
#21 fix-inconsistencies-1289900-20.patch4.53 KBaaron
#13 better-descriptions-for-field-settings-1289900-13.patch3.53 KBShawn DeArmond
#12 Edit Page A test | Media-1.png161.69 KBAnonymous (not verified)
#12 Edit Page A test | Media-2.png97.46 KBAnonymous (not verified)
#12 Field info | Media.png93.13 KBAnonymous (not verified)
#6 better-descriptions-for-field-settings-1289900-6.patch3.26 KBaaron
#4 better-descriptions-for-field-settings-1289900-4.patch2.11 KBaaron
#3 better-descriptions-for-field-settings-1289900-3.patch0 bytesaaron
#2 allowed-types-1289900-2.patch2.95 KBAnonymous (not verified)
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Assigned: Unassigned »
Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Assigned: » Unassigned
Status: Active » Needs work
FileSize
allowed-types-1289900-2.patch2.95 KB

This patch here is a work in progress... but working on it forced an issue about the utility of mapping broad mime types to file extensions, so this work is somewhat abandoned.

aaron’s picture

aaron CreditAttribution: aaron commented
Status: Needs work » Needs review
FileSize
better-descriptions-for-field-settings-1289900-3.patch0 bytes

this one grabs the titles & descriptions for allowed extensions & allowed media types. applies to file field & image field as well.

aaron’s picture

aaron CreditAttribution: aaron commented
FileSize
better-descriptions-for-field-settings-1289900-4.patch2.11 KB

oops, empty patch. here we go again!

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
Status: Needs review » Needs work 
+++ b/includes/media.fields.incundefined
@@ -187,13 +187,13 @@ function media_field_widget_settings_form($field, $instance) {
-    '#title' => t('Allowed media types'),
+    '#title' => t('Allowed remote media types'),
     '#options' => $options,
     '#default_value' => $settings['allowed_types'],
-    '#description' => t('Media types which are allowed for this field'),
+    '#description' => t('Select any remote media types which are allowed for this field. Checking these boxes have no effect unless there are remote sites also checked in the Allowed URI schemes section below.'),
     '#weight' => 1,

I'm thinking our description for this field (pre and post-patch) is redundant and really doesn't provide good description aside from the note about the Allowed URI schemes.

+++ b/includes/media.fields.incundefined
@@ -187,13 +187,13 @@ function media_field_widget_settings_form($field, $instance) {
-
+  ¶

Adding a line with trailing spaces here.

+++ b/media.moduleundefined
@@ -528,12 +528,16 @@ function media_form_field_ui_field_edit_form_alter(&$form, &$form_state) {
+    $form['instance']['settings']['file_extensions']['#title'] = t('Allowed local file extensions');
+    $form['instance']['settings']['file_extensions']['#description'] .= t(' This setting applies only to files uploaded to a local stream.');
   }
 
   // On image fields using the media widget we remove the alt/title fields
   if ($form['#field']['type'] == 'image' && $form['instance']['widget']['type']['#value'] == 'media_generic') {
     $form['instance']['settings']['alt_field']['#access'] = FALSE;
     $form['instance']['settings']['title_field']['#access'] = FALSE;
+    $form['instance']['settings']['file_extensions']['#title'] = t('Allowed local file extensions');
+    $form['instance']['settings']['file_extensions']['#description'] .= t(' This setting applies only to files uploaded to a local stream.');
   }

I'd probably just change the title to 'Allowed file extensions for uploaded files' and then we probably don't even need to add an additional description.

aaron’s picture

aaron CreditAttribution: aaron commented
Status: Needs work » Needs review
FileSize
better-descriptions-for-field-settings-1289900-6.patch3.26 KB

this also includes an allowed uploaded file exts. field for media fields.

aaron’s picture

aaron CreditAttribution: aaron commented
Assigned: Unassigned » aaron
Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented

On patch review this looks good! Could use someone manually testing it.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

I'm getting these warnings when i try to create a node with a media reference field:

Notice: Undefined index: file_directory in media_field_widget_form() (line 269 of /Users/bjd/workspace/drupal-7/sites/media.7/modules/media/includes/media.fields.inc).
Notice: Undefined index: file_extensions in media_field_widget_form() (line 270 of /Users/bjd/workspace/drupal-7/sites/media.7/modules/media/includes/media.fields.inc).
Notice: Undefined index: max_filesize in media_field_widget_form() (line 271 of /Users/bjd/workspace/drupal-7/sites/media.7/modules/media/includes/media.fields.inc).
aaron’s picture

aaron CreditAttribution: aaron commented
Status: Needs review » Needs work

me too... :(

aaron’s picture

aaron CreditAttribution: aaron commented
Status: Needs work » Needs review

actually, i get that error even w/o applying this patch.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Needs review » Needs work
FileSize
Field info | Media.png93.13 KB
Edit Page A test | Media-2.png97.46 KB
Edit Page A test | Media-1.png161.69 KB

There are some other bugs that are breaking the media reference field. I can select an item from the library for the field, but I can't upload a file with the same extension.

Only local images are allowed.

This field is configured to allow all the default types...

Only local images are allowed.

Shawn DeArmond’s picture

Shawn DeArmond CreditAttribution: Shawn DeArmond commented
FileSize
better-descriptions-for-field-settings-1289900-13.patch3.53 KB

Here's an updated patch that hides the allowed_types if there are no remote streams.

Haven't addressed #12 yet.

ParisLiakos’s picture

ParisLiakos CreditAttribution: ParisLiakos commented

Subscribe.Applied patch in #6 ,cleared cache twice,same error as #12

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented

OH! We need to add the default variables to media_field_info() under 'instance settings' and not in media_field_widget_info().

Shawn DeArmond’s picture

Shawn DeArmond CreditAttribution: Shawn DeArmond commented

#12 is happening on 7.x-2.x, even when I don't apply this patch. Should that be its own issue?

ParisLiakos’s picture

ParisLiakos CreditAttribution: ParisLiakos commented

It was: #1290428: Cannot upload media file
I am coming from there,it got closed as duplicated in favor of this

Shawn DeArmond’s picture

Shawn DeArmond CreditAttribution: Shawn DeArmond commented

@rootatwc, if it's a totally separate issue, which I think it is, then it should be its own issue, and we should review this one separately.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented

Ok yeah I am fixing the PHP notices now. I found it was a different issue.

ParisLiakos’s picture

ParisLiakos CreditAttribution: ParisLiakos commented

@#18 : done

aaron’s picture

aaron CreditAttribution: aaron commented
Status: Needs work » Needs review
FileSize
fix-inconsistencies-1289900-20.patch4.53 KB

this fixes #12 and #15. builds on #13.

ParisLiakos’s picture

ParisLiakos CreditAttribution: ParisLiakos commented
Status: Needs review » Reviewed & tested by the community

Works:)
so #1290428: Cannot upload media file IS duplicate of this:)

Shawn DeArmond’s picture

Shawn DeArmond CreditAttribution: Shawn DeArmond commented

Works for 2.x. Haven't tested 1.x.

aaron’s picture

aaron CreditAttribution: aaron commented
Status: Reviewed & tested by the community » Needs review

woops, doesn't address #12 -- it's the earlier #9 it fixes. sorry

aaron’s picture

aaron CreditAttribution: aaron commented
Status: Needs review » Reviewed & tested by the community
aaron’s picture

aaron CreditAttribution: aaron commented
Status: Reviewed & tested by the community » Fixed

committed! thanks!

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented

Followup: #1290500: Unify upload validation for both upload and the web tabs.

aaron’s picture

aaron CreditAttribution: aaron commented
Version: 7.x-2.x-dev » 7.x-1.x-dev
Status: Fixed » Needs review

need to do a quick check vs. v1

aaron’s picture

aaron CreditAttribution: aaron commented
Version: 7.x-1.x-dev » 7.x-2.x-dev
Status: Needs review » Fixed

works there as well. committed.

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

Anonymous’s picture

(not verified) CreditAttribution: commented
Issue summary: View changes

updating