Verified Commit 668d8beb authored by godotislate's avatar godotislate
Browse files

fix: #3614911 JS aggregation returns 400 for libraries loaded in the header only as dependencies

By: mherchel
By: jurgenhaas
By: bernardm28
By: catch
By: godotislate
parent a748fd63
Loading
Loading
Loading
Loading
Loading
+4 −1
Original line number Diff line number Diff line
@@ -137,7 +137,10 @@ public function optimize(array $js_assets, array $libraries) {
          // either the include or exclude query arguments. Replace this with
          // the explicit list of libraries in the aggregate.
          if (isset($js_asset['libraries'])) {
            unset($query['include'], $query['exclude'], $query['delta']);
            unset($query['include'], $query['exclude'], $query['delta'], $query['scope']);
            foreach ($js_asset['items'] as &$asset) {
              unset($asset['scope']);
            }
            $query['libraries'] = UrlHelper::compressQueryParameter(implode(',', $js_asset['libraries']));
          }
          // Add a filename prefix to mitigate ad blockers which can block
+21 −4
Original line number Diff line number Diff line
@@ -52,18 +52,35 @@ protected function getGroups(AttachedAssetsInterface $attached_assets, Request $

    if ($request->query->has('libraries')) {
      [$js_assets_header, $js_assets_footer] = $this->assetResolver->getJsAssets($attached_assets, FALSE, $language, FALSE);
      // When the libraries query argument is set, the aggregate contains every
      // file from each of the listed libraries. However, which scope an
      // individual library appears in is determined by the HTML page request as
      // well as the library definition, depending on the dependencies between
      // libraries. This means we must ignore the scope when collecting the
      // libraries and get them from both the header and footer in order.
      // When building the array, also remove the scope from each asset so that
      // it is ignored when calculating the hash, as in
      // JsCollectionOptimizerLazy::optimize().
      $assets = [];
      unset($js_assets_header['drupalSettings'], $js_assets_footer['drupalSettings']);
      foreach ($js_assets_header as $key => $asset) {
        unset($asset['scope']);
        $assets[$key] = $asset;
      }
      foreach ($js_assets_footer as $key => $asset) {
        unset($asset['scope']);
        $assets[$key] = $asset;
      }
    else {
      [$js_assets_header, $js_assets_footer] = $this->assetResolver->getJsAssets($attached_assets, FALSE, $language);
    }
    else {
      $scope = $request->query->get('scope');
      if (!isset($scope)) {
        throw new BadRequestHttpException('The URL must have a scope query argument.');
      }
      [$js_assets_header, $js_assets_footer] = $this->assetResolver->getJsAssets($attached_assets, FALSE, $language);
      $assets = $scope === 'header' ? $js_assets_header : $js_assets_footer;
    // While the asset resolver will find settings, these are never aggregated,
    // so filter them out.
      unset($assets['drupalSettings']);
    }
    return $this->grouper->group($assets);
  }

+5 −0
Original line number Diff line number Diff line
name: 'Header assets test'
type: module
description: 'Tests aggregation of libraries loaded in the header via a dependency.'
package: Testing
version: VERSION
+6 −0
Original line number Diff line number Diff line
header:
  header: true
  js:
    js/header.js: {}
  dependencies:
    - core/drupal
+4 −0
Original line number Diff line number Diff line
/**
 * @file
 * A JavaScript file loaded in the header to test asset aggregation.
 */
Loading