Only set a default name for the media item if mapped in the media source

This definitely seems like a usability concern; tagging appropriately.

A possible workaround here might be to implement hook_form_alter() so that it makes the Name field required, and empty by default, for non-images.

@MrPaulDriver that's a good point about documents having a required name field, which is not hidden in Form Display. It would be great if this was the default.

However, this is tagged as a 'Bug report' and I wasn't able to reproduce the bug. Moving the name field to the enabled fields in Media library form display worked exactly as I would have expected it to work: after uploading a new file named Test.pdf via the Media library widget, I was presented with a form containing a thumbnail preview, a language select and a required name field, pre-populated with 'Test.pdf'. I was very well able to adjust the name to my liking and save the form.

@Antoniya The reason I am describing this as a bug is because these two core modules; media and media library ought to behave in the same way when uploading a file. In this respect, the media modal upload widget does not work as expected.

I respectfully completely disagree with this statement. :) These two modules have very different purposes. Media is primarily utilitarian; it's a way to create and manage media entities, but depending on your needs as a user, not necessarily the "best" or "easiest" way. Media Library, on the other hand, is meant to be the UI that lets you interact with media entities quickly and easily, as a non-technical user.

So to me, it doesn't follow that they should necessarily behave the same way. IMHO, Media Library needs to optimize for the best possible user experience, whereas Media needs to optimize for power and flexibility.

That's why I consider this a usability issue, but not a bug (although I'm leaving this issue in that category). It sounds like the media library is working as designed; it's just that it might not be doing things as well as it could or should be for files that aren't images.

@MrPaulDriver yes, I certainly did.

This makes total sense now as a feature request. Indeed content editors can be just as lazy as us devs (hehe), so I know what you mean when you say 'well, yes they could, but they probably won't change the auto generated name'.

@Antoniya: For developers, laziness is a feature, not a bug. ;)

We discussed this issue at the #3183735: Drupal Usability Meeting 2020-12-04. We agreed that the proposed resolution (do not provide a default value for the Name field when it is shown on the form) is not a good idea for a few reasons:

1. It is more inconvenient to have nothing when you want the default value (file name) then to have a default when you want to override it.
2. This can be handled by policy rather than enforced in code.
3. On some sites, it might lead to an inconsistent mix of file names and manually entered names. This would affect searching and browsing.
4. Although there are differences between media types, there is also value in consistency. Special rules that apply only to some media types are generally not a good idea.

We have no objection to adding an option to provide a default value or not. There is a small cost (yet another option on yet another form), and we should consider whether enough sites would use this option to be worth that cost, but that is outside the scope of the usability review. We should also consider alternatives and work-arounds: adding another field or (see #5) implementing a hook in a contrib or custom module.

If you want to move forward with adding an option, the next step is to rewrite the issue summary:

• Less is more. Not because we are lazy, but we are all too busy to read long desciptions.
• The two points under "Proposed resolution" are more appropriate for the "Remaining tasks" section.

We have no objection to adding an option to provide a default value or not. There is a small cost (yet another option on yet another form), and we should consider whether enough sites would use this option to be worth that cost, but that is outside the scope of the usability review.

For what it's worth, I have no objection to adding something like this to the Media Library Extras module, which is intended to handle exactly these sorts of useful-but-not-quite-useful-enough-for-core scenarios.

@Antoniya: For developers, laziness is a feature, not a bug. ;)

trudat 🙈

So if the option 'Autogenerate media name based on file name' is added either in core or in the Media Library Extras module, it would be enabled by default for all media bundles, but site builders will be able to disable it for a specific bundle if they wish (assuming the option exists on bundle level)? @MrPaulDriver, I believe this should cover the use case you described?

I might be mistaken, but don't we already have a setting for this per media bundle? At /admin/structure/media/manage/image, I can un-map the name field. That means, I do not want the name field to be populated with the file name. Drupal should check for that before prefilling. I believe the name field is mapped by default, so this wouldn't change the default behavior.

I want my media to have SEO friendly titles. Prefilling the file name discourages that.

@jonraedeke you're right in that the name field can be unmapped when using media/add but the media library widget modal in ckeditor has the Name field default set to prepopulate with the file name.

#21 Applying the media type field mapping to the Media library default value is a good idea:

1. The behaviour is unchanged for new and existing sites.
2. This is how I assumed the media field mapping to work.

Attached patch only set a default name for the media item if mapped in the media type source configuration.

No tests yet, let's see what breaks first.

Thank you existing test coverage.

Attached patch fixes existing tests and implements correct logic for finding the name attribute in the media source field mapping.

Automated tests will keep failing until #3255749: Composer v2.2 prompts to authorize plugins is resolved.

Yeah using this setting makes sense to me, it's also what I expected it to do.

Here is a test for this, test-only isn't really needed for features but since it was the same as the interdiff might as well add it like a test-only.

Not sure about "The behaviour is unchanged for new and existing sites.", I think we would need an update to set the mapping for all name fields, since that way, if somebody had cleared this setting, it would still behave the same way after updating (ie the name gets pre-filled)

#30 was applied to 9.4.7 and it is working as expected. When the field mapping is skipped, the default name does not prepopulate with the filename. When the field mapping is set to Name, the default name is prepopulated with the filename.

Rerolled patch Against 9.5.x

Added patch against #34 in drupal 10.1.x

Thanks @mansi your patch ,your patch #36 is successfully applied on drupal 9.5.x and 10.1.x and its working fine.

Adjusting credit because the patches in #34 and #36 are identical. (You should see an "Add test/retest" link underneath patch #34, which could have been used to run it against Drupal 10.)

Adjusting credit again because the same thing is true of #34, which is identical to #30.

I think this change makes sense, but the test is a bit messy (there appears to be some copypasta) and could be streamlined. But generally I don't think this is far off.

1. +++ b/core/modules/media_library/tests/src/FunctionalJavascript/WidgetUploadTest.php
   @@ -732,4 +732,73 @@ public function testWidgetUploadAdvancedUi() {
   +  /**
   +   * Tests that uploads in the Media library's widget works as expected.
   +   */
   

   This could be more descriptive. How about something like "Tests that the name field is automatically mapped as expected"?

2. +++ b/core/modules/media_library/tests/src/FunctionalJavascript/WidgetUploadTest.php
   @@ -732,4 +732,73 @@ public function testWidgetUploadAdvancedUi() {
   +  public function testWidgetUploadSetName() {
   

   This should have the void return type.

3. +++ b/core/modules/media_library/tests/src/FunctionalJavascript/WidgetUploadTest.php
   @@ -732,4 +732,73 @@ public function testWidgetUploadAdvancedUi() {
   +    $driver = $this->getSession()->getDriver();
   

   I'm not seeing where $driver is used in this method.

4. +++ b/core/modules/media_library/tests/src/FunctionalJavascript/WidgetUploadTest.php
   @@ -732,4 +732,73 @@ public function testWidgetUploadAdvancedUi() {
   +    foreach ($this->getTestFiles('image') as $image) {
   +      $extension = pathinfo($image->filename, PATHINFO_EXTENSION);
   +      if ($extension === 'png') {
   +        $png_image = $image;
   +      }
   +      elseif ($extension === 'jpg') {
   +        $jpg_image = $image;
   +      }
   +    }
   +
   +    if (!isset($png_image) || !isset($jpg_image)) {
   +      $this->fail('Expected test files not present.');
   +    }
   

   It doesn't look like $png_image is used elsewhere in this method, so I think this could be streamlined. We just need to have an image of any type.

5. +++ b/core/modules/media_library/tests/src/FunctionalJavascript/WidgetUploadTest.php
   @@ -732,4 +732,73 @@ public function testWidgetUploadAdvancedUi() {
   +    // Assert the upload form is not visible for the non-file based media type
   +    // type_one.
   

   This comment seems divorced from what's actually going on in the test.

6. +++ b/core/modules/media_library/tests/src/FunctionalJavascript/WidgetUploadTest.php
   @@ -732,4 +732,73 @@ public function testWidgetUploadAdvancedUi() {
   +    $jpg_uri = $file_system->copy($jpg_image->uri, 'public://');
   

   Is this necessary?

7. +++ b/core/modules/media_library/tests/src/FunctionalJavascript/WidgetUploadTest.php
   @@ -732,4 +732,73 @@ public function testWidgetUploadAdvancedUi() {
   +    // Test that the name field has been pre-filled.
   +    $this->assertStringContainsString($jpg_image->name, $page->findField('Name')->getValue());
   

   Can't we just use $assert_session->fieldValueEquals() here?

8. +++ b/core/modules/media_library/tests/src/FunctionalJavascript/WidgetUploadTest.php
   @@ -732,4 +732,73 @@ public function testWidgetUploadAdvancedUi() {
   +    // Remove the field map for name.
   +    $this->drupalLogin($admin);
   +    $this->drupalGet('admin/structure/media/manage/type_four');
   +    $edit = [
   +      'field_map[name]' => '_none',
   +    ];
   +    $this->submitForm($edit, 'Save');
   

   IMHO we could just do this by loading the media type and modifying it directly, rather than logging in as an admin and doing it in the UI. That would remove some complexity from this test.

9. +++ b/core/modules/media_library/tests/src/FunctionalJavascript/WidgetUploadTest.php
   @@ -732,4 +732,73 @@ public function testWidgetUploadAdvancedUi() {
   +    // Test that the name field has not been pre-filled.
   +    $this->assertEmpty($page->findField('Name')->getValue());
   

   We probably don't want to call $page->fieldField('Name')->getValue() here, because if the field isn't found, the chained call to getValue() will cause a fatal error. Couldn't we use $assert_session->fieldValueEquals('Name', '') instead?