From 6c8f58a3abe1b5537f678ea7950350f635a5a1a4 Mon Sep 17 00:00:00 2001
From: Nathaniel Catchpole <catch@35733.no-reply.drupal.org>
Date: Mon, 2 Nov 2015 19:36:41 +0000
Subject: [PATCH] Issue #2597359 by Wim Leers: Require responses with
attachments to contain the final attachment values
---
.../Ajax/AjaxResponseAttachmentsProcessor.php | 10 ++++++
core/lib/Drupal/Core/Asset/AssetResolver.php | 3 ++
.../Core/Asset/AssetResolverInterface.php | 2 ++
.../AttachmentsResponseProcessorInterface.php | 3 +-
.../HtmlResponseAttachmentsProcessor.php | 36 ++++++++++++-------
.../src/Tests/Common/AttachedAssetsTest.php | 2 ++
6 files changed, 42 insertions(+), 14 deletions(-)
diff --git a/core/lib/Drupal/Core/Ajax/AjaxResponseAttachmentsProcessor.php b/core/lib/Drupal/Core/Ajax/AjaxResponseAttachmentsProcessor.php
index 7d36b51c718e..69a8f1378887 100644
--- a/core/lib/Drupal/Core/Ajax/AjaxResponseAttachmentsProcessor.php
+++ b/core/lib/Drupal/Core/Ajax/AjaxResponseAttachmentsProcessor.php
@@ -148,6 +148,16 @@ protected function buildAttachmentsCommands(AjaxResponse $response, Request $req
$css_assets = $this->assetResolver->getCssAssets($assets, $optimize_css);
list($js_assets_header, $js_assets_footer) = $this->assetResolver->getJsAssets($assets, $optimize_js);
+ // First, AttachedAssets::setLibraries() ensures duplicate libraries are
+ // removed: it converts it to a set of libraries if necessary. Second,
+ // AssetResolver::getJsSettings() ensures $assets contains the final set of
+ // JavaScript settings. AttachmentsResponseProcessorInterface also mandates
+ // that the response it processes contains the final attachment values, so
+ // update both the 'library' and 'drupalSettings' attachments accordingly.
+ $attachments['library'] = $assets->getLibraries();
+ $attachments['drupalSettings'] = $assets->getSettings();
+ $response->setAttachments($attachments);
+
// Render the HTML to load these files, and add AJAX commands to insert this
// HTML in the page. Settings are handled separately, afterwards.
$settings = [];
diff --git a/core/lib/Drupal/Core/Asset/AssetResolver.php b/core/lib/Drupal/Core/Asset/AssetResolver.php
index 9927049bea6e..a533c8ad61df 100644
--- a/core/lib/Drupal/Core/Asset/AssetResolver.php
+++ b/core/lib/Drupal/Core/Asset/AssetResolver.php
@@ -334,6 +334,9 @@ public function getJsAssets(AttachedAssetsInterface $assets, $optimize) {
// Allow modules and themes to alter the JavaScript settings.
$this->moduleHandler->alter('js_settings', $settings, $assets);
$this->themeManager->alter('js_settings', $settings, $assets);
+ // Update the $assets object accordingly, so that it reflects the final
+ // settings.
+ $assets->setSettings($settings);
$settings_as_inline_javascript = [
'type' => 'setting',
'group' => JS_SETTING,
diff --git a/core/lib/Drupal/Core/Asset/AssetResolverInterface.php b/core/lib/Drupal/Core/Asset/AssetResolverInterface.php
index c912d7663c49..e0845c62ee87 100644
--- a/core/lib/Drupal/Core/Asset/AssetResolverInterface.php
+++ b/core/lib/Drupal/Core/Asset/AssetResolverInterface.php
@@ -69,6 +69,8 @@ public function getCssAssets(AttachedAssetsInterface $assets, $optimize);
*
* @param \Drupal\Core\Asset\AttachedAssetsInterface $assets
* The assets attached to the current response.
+ * Note that this object is modified to reflect the final JavaScript
+ * settings assets.
* @param bool $optimize
* Whether to apply the JavaScript asset collection optimizer, to return
* optimized JavaScript asset collections rather than an unoptimized ones.
diff --git a/core/lib/Drupal/Core/Render/AttachmentsResponseProcessorInterface.php b/core/lib/Drupal/Core/Render/AttachmentsResponseProcessorInterface.php
index 6067a8625711..74614eec077a 100644
--- a/core/lib/Drupal/Core/Render/AttachmentsResponseProcessorInterface.php
+++ b/core/lib/Drupal/Core/Render/AttachmentsResponseProcessorInterface.php
@@ -46,7 +46,8 @@ interface AttachmentsResponseProcessorInterface {
* The response to process.
*
* @return \Drupal\Core\Render\AttachmentsInterface
- * The processed response.
+ * The processed response, with the attachments updated to reflect their
+ * final values.
*
* @throws \InvalidArgumentException
* Thrown when the $response parameter is not the type of response object
diff --git a/core/lib/Drupal/Core/Render/HtmlResponseAttachmentsProcessor.php b/core/lib/Drupal/Core/Render/HtmlResponseAttachmentsProcessor.php
index e0160d1b46a9..d4070629c626 100644
--- a/core/lib/Drupal/Core/Render/HtmlResponseAttachmentsProcessor.php
+++ b/core/lib/Drupal/Core/Render/HtmlResponseAttachmentsProcessor.php
@@ -9,6 +9,7 @@
use Drupal\Core\Asset\AssetCollectionRendererInterface;
use Drupal\Core\Asset\AssetResolverInterface;
use Drupal\Core\Asset\AttachedAssets;
+use Drupal\Core\Asset\AttachedAssetsInterface;
use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\Form\EnforcedResponseException;
use Drupal\Core\Extension\ModuleHandlerInterface;
@@ -155,7 +156,19 @@ public function processAttachments(AttachmentsInterface $response) {
$attachment_placeholders = $attached['html_response_attachment_placeholders'];
unset($attached['html_response_attachment_placeholders']);
- $variables = $this->processAssetLibraries($attached, $attachment_placeholders);
+ $assets = AttachedAssets::createFromRenderArray(['#attached' => $attached]);
+ // Take Ajax page state into account, to allow for something like
+ // Turbolinks to be implemented without altering core.
+ // @see https://github.com/rails/turbolinks/
+ $ajax_page_state = $this->requestStack->getCurrentRequest()->get('ajax_page_state');
+ $assets->setAlreadyLoadedLibraries(isset($ajax_page_state) ? explode(',', $ajax_page_state['libraries']) : []);
+ $variables = $this->processAssetLibraries($assets, $attachment_placeholders);
+ // $variables now contains the markup to load the asset libraries. Update
+ // $attached with the final list of libraries and JavaScript settings, so
+ // that $response can be updated with those. Then the response object will
+ // list the final, processed attachments.
+ $attached['library'] = $assets->getLibraries();
+ $attached['drupalSettings'] = $assets->getSettings();
// Since we can only replace content in the HTML head section if there's a
// placeholder for it, we can safely avoid processing the render array if
@@ -168,6 +181,7 @@ public function processAttachments(AttachmentsInterface $response) {
$attached,
$this->processFeed($attached['feed'])
);
+ unset($attached['feed']);
}
// 'html_head_link' is a special case of 'html_head' which can be present
// as a head element, but also as a Link: HTTP header depending on
@@ -182,6 +196,7 @@ public function processAttachments(AttachmentsInterface $response) {
$attached,
$this->processHtmlHeadLink($attached['html_head_link'])
);
+ unset($attached['html_head_link']);
}
// Now we can process 'html_head', which contains both 'feed' and
@@ -200,6 +215,10 @@ public function processAttachments(AttachmentsInterface $response) {
$this->setHeaders($response, $attached['http_header']);
}
+ // AttachmentsResponseProcessorInterface mandates that the response it
+ // processes contains the final attachment values.
+ $response->setAttachments($attached);
+
return $response;
}
@@ -255,8 +274,8 @@ protected function renderPlaceholders(HtmlResponse $response) {
/**
* Processes asset libraries into render arrays.
*
- * @param array $attached
- * The attachments to process.
+ * @param \Drupal\Core\Asset\AttachedAssetsInterface $assets
+ * The attached assets collection for the current response.
* @param array $placeholders
* The placeholders that exist in the response.
*
@@ -266,16 +285,7 @@ protected function renderPlaceholders(HtmlResponse $response) {
* - scripts
* - scripts_bottom
*/
- protected function processAssetLibraries(array $attached, array $placeholders) {
- $all_attached = ['#attached' => $attached];
- $assets = AttachedAssets::createFromRenderArray($all_attached);
-
- // Take Ajax page state into account, to allow for something like Turbolinks
- // to be implemented without altering core.
- // @see https://github.com/rails/turbolinks/
- $ajax_page_state = $this->requestStack->getCurrentRequest()->get('ajax_page_state');
- $assets->setAlreadyLoadedLibraries(isset($ajax_page_state) ? explode(',', $ajax_page_state['libraries']) : []);
-
+ protected function processAssetLibraries(AttachedAssetsInterface $assets, array $placeholders) {
$variables = [];
// Print styles - if present.
diff --git a/core/modules/system/src/Tests/Common/AttachedAssetsTest.php b/core/modules/system/src/Tests/Common/AttachedAssetsTest.php
index 7440d995df6a..cc04606bcc9f 100644
--- a/core/modules/system/src/Tests/Common/AttachedAssetsTest.php
+++ b/core/modules/system/src/Tests/Common/AttachedAssetsTest.php
@@ -108,8 +108,10 @@ function testAddJsSettings() {
$build['#attached']['library'][] = 'core/drupalSettings';
$assets = AttachedAssets::createFromRenderArray($build);
+ $this->assertEqual([], $assets->getSettings(), 'JavaScript settings on $assets are empty.');
$javascript = $this->assetResolver->getJsAssets($assets, FALSE)[1];
$this->assertTrue(array_key_exists('currentPath', $javascript['drupalSettings']['data']['path']), 'The current path JavaScript setting is set correctly.');
+ $this->assertTrue(array_key_exists('currentPath', $assets->getSettings()['path']), 'JavaScript settings on $assets are resolved after retrieving JavaScript assets, and are equal to the returned JavaScript settings.');
$assets->setSettings(['drupal' => 'rocks', 'dries' => 280342800]);
$javascript = $this->assetResolver->getJsAssets($assets, FALSE)[1];
--
GitLab