Videos uploaded with CKEditor5 widget do not display

Created on 19 January 2024, 8 months ago
Updated 5 March 2024, 6 months ago

Problem/Motivation

I am trying to embed private videos and display them in nodes using CKEditor5 in Core. (Running version 10.1.6 currently.) But more recent uploads do not display. I tracked the issue down to "include file in display" box showing checked (i.e. yes, display) but internally being set to NULL in the DB. See forum post https://www.drupal.org/forum/support/post-installation/2024-01-17/embedd... โ†’

Steps to reproduce

  1. Add a new Blog Post type node.
  2. Insert a private video using the "Insert media" widget.
  3. Save the node.

On my site, the video does not display. The video is uploaded properly and the embed code looks fine but the video doesn't display. (See the forum post for more details.)

The video's media page at /media/86/edit shows the "include file in display" box is checked but the DB shows the "display" flag as NULL instead of 1 as the older uploads show.

mysql> select * from media__field_media_video_file_1;
+---------------+---------+-----------+-------------+----------+-------+------------------------------------+----------------------------------+--------------------------------------+
| bundle        | deleted | entity_id | revision_id | langcode | delta | field_media_video_file_1_target_id | field_media_video_file_1_display | field_media_video_file_1_description |
+---------------+---------+-----------+-------------+----------+-------+------------------------------------+----------------------------------+--------------------------------------+
| private_video |       0 |        66 |          66 | en       |     0 |                                143 |                                1 |                                      |
| private_video |       0 |        69 |          69 | en       |     0 |                                146 |                                1 |                                      |
| private_video |       0 |        72 |          72 | en       |     0 |                                149 |                                1 |                                      |
| private_video |       0 |        73 |          73 | en       |     0 |                                150 |                                1 |                                      |
| private_video |       0 |        76 |          76 | en       |     0 |                                153 |                                1 |                                      |
| private_video |       0 |        79 |          79 | en       |     0 |                                156 |                             NULL | NULL                                 |
| private_video |       0 |        80 |          80 | en       |     0 |                                157 |                             NULL | NULL                                 |
| private_video |       0 |        82 |          82 | en       |     0 |                                159 |                             NULL | NULL                                 |
+---------------+---------+-----------+-------------+----------+-------+------------------------------------+----------------------------------+--------------------------------------+
8 rows in set (0.01 sec)

So I tried the resolution below.

It's there now.

Proposed resolution

I found that if I go to the specific video's media page at /media/86/edit and first UNcheck the "include file in display" box and save it, then CHECK it again and save it, then the video appears.

This is happening with all my new uploads though. My config is set to display and the "include file in display" checkbox is checked as it should be, but it is misleading b/c it isn't the actual state for my newly uploaded videos. (i.e. they don't display)

Resolution would be to fix the problem of the misleading checkbox. It should really be set to display (1) when the box is checked.

Merge request link

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

๐Ÿ› Bug report
Status

Needs work

Version

11.0 ๐Ÿ”ฅ

Component
Mediaย  โ†’

Last updated 1 minute ago

Created by

Live updates comments and jobs are added and updated live.
  • Needs subsystem maintainer review

    It is used to alert the maintainer(s) of a particular core subsystem that an issue significantly impacts their subsystem, and their signoff is needed (see the governance policy draft for more information). Also, if you use this tag, make sure the issue component is set to the correct subsystem. If an issue significantly impacts more than one subsystem, use needs framework manager review instead.

Sign in to follow issues

Merge Requests

Comments & Activities

  • 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 use Public files as Upload destination, same thing happens. But if we disable the Enable 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 using Drupal\media_library\Form\FileUploadForm for form generation, which is not handling the 'display' option like in Drupal\file\Plugin\Field\FieldWidget\FileWidget. Because of that field_media_video_file_1_display will get set as NULL instead 1.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sijumpk
  • Status changed to Needs review 8 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณ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.

  • Pipeline finished with Failed
    8 months ago
    #82322
  • Status changed to Needs work 7 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    This will need test coverage.

    Current fix caused existing test failures too.

  • Merge request !6360Update AddFormBase.php โ†’ (Closed) created by sijumpk
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sijumpk

    sijumpk โ†’ changed the visibility of the branch 10.2.x to hidden.

  • Pipeline finished with Canceled
    7 months ago
    Total: 1151s
    #83943
  • Pipeline finished with Success
    7 months ago
    Total: 555s
    #83946
  • Merge request !6361Update AddFormBase.php โ†’ (Closed) created by sijumpk
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sijumpk

    sijumpk โ†’ changed the visibility of the branch 3415771-private-videos-upload to hidden.

  • Pipeline finished with Failed
    7 months ago
    Total: 1308s
    #83948
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sijumpk

    sijumpk โ†’ changed the visibility of the branch 11.x to hidden.

  • Pipeline finished with Success
    7 months ago
    Total: 720s
    #84584
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sijumpk

    sijumpk โ†’ changed the visibility of the branch 10.2.x to active.

  • Pipeline finished with Failed
    7 months ago
    Total: 214s
    #87106
  • Pipeline finished with Success
    7 months ago
    Total: 698s
    #87110
  • Status changed to Needs review 7 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณ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
  • ๐Ÿ‡บ๐Ÿ‡ธ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.

  • Pipeline finished with Failed
    7 months ago
    Total: 197s
    #93826
  • Pipeline finished with Failed
    7 months ago
    Total: 526s
    #93839
  • Pipeline finished with Success
    7 months ago
    Total: 678s
    #93862
  • Merge request !6577Resolve #3415771 "Video media upload issue" โ†’ (Open) created by sijumpk
  • Pipeline finished with Failed
    7 months ago
    Total: 523s
    #93886
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sijumpk

    sijumpk โ†’ changed the visibility of the branch 10.2.x to hidden.

  • Pipeline finished with Failed
    7 months ago
    Total: 648s
    #93898
  • Pipeline finished with Failed
    7 months ago
    Total: 498s
    #93918
  • Pipeline finished with Success
    7 months ago
    Total: 2960s
    #93931
  • Status changed to Needs review 7 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณ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?

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sijumpk

    Current MR is only for 11.x.

  • Status changed to RTBC 7 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Believe this is good still.

  • Status changed to Needs work 6 months ago
  • ๐Ÿ‡ฌ๐Ÿ‡ง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.

Production build 0.71.5 2024