In the lightning_media module, there is this code:

function lightning_media_library_info_alter(array &$libraries, $extension) {
  if ($extension == 'lightning_media') {
    $root = '/' . \Drupal::moduleHandler()->getModule('lightning')->getPath();
...

That getModule('lightning') call specifically depends on this module being within the lightning distribution. When we are creating our own sub-profiles, we currently need to include lightning make file within our own profile make file to get all the modules and libraries. But when doing that the name of the profile is our custom name and not "lightning" so this getPath fails, causing media to fail in strange places.

It seems that this code is just being done to locate the libraries within the profile. We handle that in our build by adding symbolic links from /libraries/lightning to profiles/lightning/libraries. But isn't this also maybe something the Libraries API module still helps with?

In any case, the code is a bit kludgy and should remove the specific profile dependency and maybe ideally find a better way to handle the libraries.

CommentFileSizeAuthor
#6 lightning_media-change-profile-path-handling-2692419-6.patch2.64 KBmirie
#5 lightning_media-change-profile-path-handling-2692419-5.patch2.52 KBmirie

Comments

mpotter created an issue. See original summary.

saltednut’s picture

saltednut
she/her
Primary language English
Location Chicago
commented

@mpotter can you share any of the code for sub-profiles? We are trying to come up with good solutions for how to use Lightning's features in DF and I was not aware this was working so far as to even get to this error.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

That hook_library_info_alter() was always a hack and I'm totally open to replacing it with something non-kludgey. If Libraries API will do the same thing cleanly, I'm all for it.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

I poked around a bit with regard to the Libraries API, and from what I can tell it's not really ready for prime time.

One possibility is to roll our own (hopefully temporary) system of search paths. So the libraries could be defined thusly:

backbone.collectionView:
  version: 1.0.0
  js:
    'libraries/backbone.collectionView/dist/backbone.collectionView.min.js':
      minified: true
  dependencies:
    - core/backbone
  search_paths:
    - @profile:lightning
    - /libraries
    - /sites/all/libraries

This would cause it to look for the library in the following locations:

  • <path to the lightning profile>/backbone.collectionView
  • <drupal root>/libraries/backbone.collectionView
  • <drupal root>/sites/all/libraries/backbone.collectionView

Another option is to attempt to load the libraries from cdnjs, where they are all hosted, and then prefer the external library to the packaged one. I dunno...I welcome any suggestions here.

mirie’s picture

mirie commented
StatusFileSize
new lightning_media-change-profile-path-handling-2692419-5.patch2.52 KB

Attached is a patch that uses ExtensionDiscovery instead of ModuleHandler. ModuleHandler only returns data for installed modules, which is totally fine for installed profiles, but fails for profiles that are using Lightning as a base profile.

mirie’s picture

mirie commented
StatusFileSize
new lightning_media-change-profile-path-handling-2692419-6.patch2.64 KB

I noticed that there are warnings for the for loop so I added some existence checks in this updated patch:

This is from simplytest.me Lightning instance using RC4:

Notice: Undefined index: css in lightning_media_library_info_alter() (line 81 of /home/d0s8v/www/profiles/lightning/modules/lightning_features/lightning_media/lightning_media.module).

Warning: Invalid argument supplied for foreach() in lightning_media_library_info_alter() (line 81 of /home/d0s8v/www/profiles/lightning/modules/lightning_features/lightning_media/lightning_media.module).

balsama’s picture

balsama
he/him
Primary language English
Location boston
commented
Status: Active » Needs review

  • balsama committed 6938acc on 8.x-1.x authored by mirie 
    Issue #2692419 by mirie: lightning_media depends on the lightning...
balsama’s picture

balsama
he/him
Primary language English
Location boston
commented
Status: Needs review » Fixed

Committed to 8.x-1.x. Thanks!

Status: Fixed » Closed (fixed)

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