- Issue created by @leeksoup
- ๐บ๐ธUnited States cilefen
For bug reproduction purposes, could you please define โprivate videoโ in some technical detail?
Sorry ... "private video" means a video that is stored in the private file path for the site.
To achieve this, I added a Private Video media type under /admin/structure/media, setting
Name = "Private Video"
machine name "private_video"
Media source = "Video file"Publishing options -- published by default
In Manage Display, all modes except "Media Library" display the video file. I can add screenshots if helpful.
Thanks.
- ๐ฎ๐ณIndia sijumpk
This is not just happening with
Private files
, even if we usePublic files
asUpload destination
, same thing happens. But if we disable theEnable Display field
option for file field, embedding will work file for both private and public files. That's how the option is set for the default Video media type (/admin/structure/media/manage/video/fields/media.video.field_media_video_file
). Media library is usingDrupal\media_library\Form\FileUploadForm
for form generation, which is not handling the 'display' option like inDrupal\file\Plugin\Field\FieldWidget\FileWidget
. Because of thatfield_media_video_file_1_display
will get set as NULL instead 1. - Merge request !6320Update AddFormBase.php; Fixing media file display getting set as null. โ (Open) created by sijumpk
- Status changed to Needs review
8 months ago 12:09pm 25 January 2024 - ๐ฎ๐ณIndia sijumpk
Updated the code in such a way that if the display property is null, it will be changed to TRUE. Now media uploading to CKEditor5 will get displayed without any problem.
- Status changed to Needs work
7 months ago 6:55pm 28 January 2024 - ๐บ๐ธUnited States smustgrave
This will need test coverage.
Current fix caused existing test failures too.
- ๐ฎ๐ณIndia sijumpk
sijumpk โ changed the visibility of the branch 10.2.x to hidden.
- ๐ฎ๐ณIndia sijumpk
sijumpk โ changed the visibility of the branch 3415771-private-videos-upload to hidden.
- ๐ฎ๐ณIndia sijumpk
sijumpk โ changed the visibility of the branch 11.x to hidden.
- ๐ฎ๐ณIndia sijumpk
sijumpk โ changed the visibility of the branch 10.2.x to active.
- Status changed to Needs review
7 months ago 4:42am 5 February 2024 - ๐ฎ๐ณIndia sijumpk
Altered the code to fix the existing problem and test case also added.
- ๐ฎ๐ณIndia TanujJain-TJ
Tested and verified this MR and it fixes the issue, this happens with both private and public video because of enabling this
Enable Display field
option in Video field setting as stated by @sijumpk. attaching screenshots for reference. RTBC++ - Status changed to Needs work
7 months ago 11:52pm 6 February 2024 - ๐บ๐ธUnited States smustgrave
MR is pointing to 10.2 not 11.x
Also not sure we need a javascript test here. There's an ImageUploadTest file under functional so imagine VideoUploadTests could go there also.
- ๐ฎ๐ณIndia sijumpk
sijumpk โ changed the visibility of the branch 10.2.x to hidden.
- Status changed to Needs review
7 months ago 5:08am 14 February 2024 - ๐ฎ๐ณIndia sijumpk
@smustgrave, your are right. We can do the testing using just Functional test only, like in case of ImageUploadTest. So the FunctionalJavascript test is now replaced with Functional test. Also changed the MR to 11.x version. Please review it.
- ๐บ๐ธUnited States smustgrave
Test move seems good
Will leave in review for additional eyes.
Thanks for all the work on this, @sijumpk! Is this patch only for version 11.x?
- Status changed to RTBC
7 months ago 3:48pm 16 February 2024 - Status changed to Needs work
6 months ago 1:07pm 4 March 2024 - ๐ฌ๐งUnited Kingdom catch
#5 makes me think that Drupal\media_library\Form\FileUploadForm should be doing similar handling to Drupal\file\Plugin\Field\FieldWidget\FileWidget so that the field gets set to 1 instead of 0.
If there's a reason this can't be done, I think we need more explanation in the issue summary and also the inline code comment at least, but feels like the wrong fix to me.
I think @catch is correct in #28. It's also unclear to me why the "include file in display" checkbox on the media item's page does not match the DB value.