Problem/Motivation

The module saves generated thumbnails in an incorrect directory.

Instead of being stored in the public files directory, thumbnails are saved in:

/web/:/video_thumbnails

Expected location:

/web/sites/default/files/video_thumbnails

Steps to reproduce

  1. Enable the module.
  2. Add a video embed field using the HTML5 provider.
  3. Enable display thumbnail.
  4. Check the generated thumbnail file location.

Actual result

Thumbnails are saved in an incorrect path (/web/:/video_thumbnails).

Expected result

Thumbnails should be saved in the public files directory (/web/sites/default/files/video_thumbnails) using Drupal's file system API.

Root cause

The issue is caused by the $thumbsDirectory property in Html5.php not being properly initialized.

It should be initialized using the parent implementation via the getThumbsDirectory() method.

Proposed solution

Initialize $thumbsDirectory using the parent getThumbsDirectory() method to ensure the correct Drupal file system path is used.

Issue fork video_embed_html5-3586425

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

witcher_ created an issue. See original summary.

witcher_ opened merge request !13

witcher_’s picture

witcher_ commented
Assigned: witcher_ » Unassigned
Status: Active » Needs review
angel_devoeted’s picture

angel_devoeted commented

Tested MR !13 in a Drupal 11 environment (Drupal Pod) and I can confirm it fixes the incorrect path issue.
Before the fix, $thumbsDirectory was uninitialized, but now it correctly points to public://video_thumbnails and thumbnails are generated as expected.
The CI failure seems to be a false positive due to the environment's composer constraints and not the MR itself.
One minor concern: would it be safer to initialize this property in the constructor instead of the create method to ensure it's always set regardless of how the plugin is instantiated?

witcher_’s picture

witcher_ commented

@angel_devoeted, thanks for pointing this out. The initialization has been moved to the __construct() method.

A follow-up issue has been created to move all remaining variable initialization to the constructor.