From 4b1d3103e996e619f752a338e73271cd0cad85b1 Mon Sep 17 00:00:00 2001
From: Nathaniel Catchpole <catch@35733.no-reply.drupal.org>
Date: Wed, 5 Feb 2014 13:22:50 +0000
Subject: [PATCH] Issue #2171269 by effulgentsia, Wim Leers, InternetDevels,
snig: Handle block rendering's #attributes correctly (fixes in-place editing
of custom blocks).
---
core/modules/block/block.module | 28 ---------
.../lib/Drupal/block/BlockViewBuilder.php | 42 ++++++++++---
.../Drupal/block/Tests/BlockHtmlIdTest.php | 56 -----------------
.../lib/Drupal/block/Tests/BlockHtmlTest.php | 61 +++++++++++++++++++
.../block/Tests/BlockInvalidRegionTest.php | 2 +-
.../block/Tests/BlockStorageUnitTest.php | 12 ++--
.../config/block.block.test_block.yml | 4 +-
.../block_test/Plugin/Block/TestHtmlBlock.php | 32 ++++++++++
.../Plugin/Block/TestHtmlIdBlock.php | 19 ------
core/modules/menu/menu.module | 2 +-
.../Tests/Entity/EntityCrudHookTest.php | 4 +-
core/modules/system/system.module | 19 ++++++
12 files changed, 159 insertions(+), 122 deletions(-)
delete mode 100644 core/modules/block/lib/Drupal/block/Tests/BlockHtmlIdTest.php
create mode 100644 core/modules/block/lib/Drupal/block/Tests/BlockHtmlTest.php
create mode 100644 core/modules/block/tests/modules/block_test/lib/Drupal/block_test/Plugin/Block/TestHtmlBlock.php
delete mode 100644 core/modules/block/tests/modules/block_test/lib/Drupal/block_test/Plugin/Block/TestHtmlIdBlock.php
diff --git a/core/modules/block/block.module b/core/modules/block/block.module
index ba915a93e46f..daea82cc973e 100644
--- a/core/modules/block/block.module
+++ b/core/modules/block/block.module
@@ -7,7 +7,6 @@
use Drupal\block\BlockInterface;
use Drupal\Component\Plugin\Exception\PluginException;
-use Drupal\Component\Utility\NestedArray;
use Symfony\Cmf\Component\Routing\RouteObjectInterface;
/**
@@ -240,23 +239,6 @@ function _block_get_renderable_region($list = array()) {
),
);
}
-
- // Add contextual links for this block; skip the main content block, since
- // contextual links are basically output as tabs/local tasks already. Also
- // skip the help block, since we assume that most users do not need or want
- // to perform contextual actions on the help block, and the links needlessly
- // draw attention on it.
- if (isset($build[$key]) && !in_array($block->get('plugin'), array('system_help_block', 'system_main_block'))) {
- $build[$key]['#contextual_links']['block'] = array(
- 'route_parameters' => array('block' => $key),
- );
-
- // If there are any nested contextual links, move them to the top level.
- if (isset($build[$key]['content']['#contextual_links'])) {
- $build[$key]['#contextual_links'] += $build[$key]['content']['#contextual_links'];
- unset($build[$key]['content']['#contextual_links']);
- }
- }
}
return $build;
}
@@ -501,16 +483,6 @@ function template_preprocess_block(&$variables) {
$variables['attributes']['class'][] = 'block';
$variables['attributes']['class'][] = drupal_html_class('block-' . $variables['configuration']['module']);
- // The block template provides a wrapping element for the content. Render the
- // #attributes of the content on this wrapping element rather than passing
- // them through to the content's #theme function/template. This allows the
- // content to not require a function/template at all, or if it does use one,
- // to not require it to output an extra wrapping element.
- if (isset($variables['content']['#attributes'])) {
- $variables['content_attributes'] = NestedArray::mergeDeep($variables['content_attributes'], $variables['content']['#attributes']);
- unset($variables['content']['#attributes']);
- }
-
// Add default class for block content.
$variables['content_attributes']['class'][] = 'content';
diff --git a/core/modules/block/lib/Drupal/block/BlockViewBuilder.php b/core/modules/block/lib/Drupal/block/BlockViewBuilder.php
index 73cabaae29e0..200b428ddde7 100644
--- a/core/modules/block/lib/Drupal/block/BlockViewBuilder.php
+++ b/core/modules/block/lib/Drupal/block/BlockViewBuilder.php
@@ -36,7 +36,8 @@ public function view(EntityInterface $entity, $view_mode = 'full', $langcode = N
*/
public function viewMultiple(array $entities = array(), $view_mode = 'full', $langcode = NULL) {
$build = array();
- foreach ($entities as $entity_id => $entity) {
+ foreach ($entities as $key => $entity) {
+ $entity_id = $entity->id();
$plugin = $entity->getPlugin();
$plugin_id = $plugin->getPluginId();
$base_id = $plugin->getBasePluginId();
@@ -44,24 +45,51 @@ public function viewMultiple(array $entities = array(), $view_mode = 'full', $la
if ($content = $plugin->build()) {
$configuration = $plugin->getConfiguration();
- $build[$entity_id] = array(
+
+ // Create the render array for the block as a whole.
+ // @see template_preprocess_block().
+ $build[$key] = array(
'#theme' => 'block',
- 'content' => $content,
+ '#attributes' => array(),
+ '#contextual_links' => array(
+ 'block' => array(
+ 'route_parameters' => array('block' => $entity_id),
+ ),
+ ),
'#configuration' => $configuration,
'#plugin_id' => $plugin_id,
'#base_plugin_id' => $base_id,
'#derivative_plugin_id' => $derivative_id,
);
- $build[$entity_id]['#configuration']['label'] = check_plain($configuration['label']);
+ $build[$key]['#configuration']['label'] = check_plain($configuration['label']);
+
+ // Place the $content returned by the block plugin into a 'content'
+ // child element, as a way to allow the plugin to have complete control
+ // of its properties and rendering (e.g., its own #theme) without
+ // conflicting with the properties used above, or alternate ones used
+ // by alternate block rendering approaches in contrib (e.g., Panels).
+ // However, the use of a child element is an implementation detail of
+ // this particular block rendering approach. Semantically, the content
+ // returned by the plugin "is the" block, and in particular,
+ // #attributes and #contextual_links is information about the *entire*
+ // block. Therefore, we must move these properties from $content and
+ // merge them into the top-level element.
+ foreach (array('#attributes', '#contextual_links') as $property) {
+ if (isset($content[$property])) {
+ $build[$key][$property] += $content[$property];
+ unset($content[$property]);
+ }
+ }
+ $build[$key]['content'] = $content;
}
else {
- $build[$entity_id] = array();
+ $build[$key] = array();
}
- $this->moduleHandler()->alter(array('block_view', "block_view_$base_id"), $build[$entity_id], $plugin);
+ $this->moduleHandler()->alter(array('block_view', "block_view_$base_id"), $build[$key], $plugin);
// @todo Remove after fixing http://drupal.org/node/1989568.
- $build[$entity_id]['#block'] = $entity;
+ $build[$key]['#block'] = $entity;
}
return $build;
}
diff --git a/core/modules/block/lib/Drupal/block/Tests/BlockHtmlIdTest.php b/core/modules/block/lib/Drupal/block/Tests/BlockHtmlIdTest.php
deleted file mode 100644
index de18e91f6f1d..000000000000
--- a/core/modules/block/lib/Drupal/block/Tests/BlockHtmlIdTest.php
+++ /dev/null
@@ -1,56 +0,0 @@
-<?php
-
-/**
- * @file
- * Definition of Drupal\block\Tests\BlockHtmlIdTest.
- */
-
-namespace Drupal\block\Tests;
-
-use Drupal\simpletest\WebTestBase;
-
-/**
- * Tests block HTML ID validity.
- */
-class BlockHtmlIdTest extends WebTestBase {
-
- /**
- * Modules to enable.
- *
- * @var array
- */
- public static $modules = array('block', 'block_test');
-
- public static function getInfo() {
- return array(
- 'name' => 'Block HTML ID',
- 'description' => 'Tests block HTML ID validity.',
- 'group' => 'Block',
- );
- }
-
- function setUp() {
- parent::setUp();
-
- $this->drupalLogin($this->root_user);
-
- // Make sure the block has some content so it will appear.
- $current_content = $this->randomName();
- \Drupal::state()->set('block_test.content', $current_content);
-
- // Enable our test blocks.
- $this->drupalPlaceBlock('system_menu_block:tools');
- $this->drupalPlaceBlock('test_html_id', array('id' => 'test_id_block'));
- }
-
- /**
- * Tests for a valid HTML ID for a block.
- */
- function testHtmlId() {
- $this->drupalGet('');
- $this->assertRaw('id="block-test-id-block"', 'HTML ID for test block is valid.');
- $elements = $this->xpath('//div[contains(@class, :div-class)]/div/ul[contains(@class, :ul-class)]/li', array(':div-class' => 'block-system', ':ul-class' => 'menu'));
- $this->assertTrue(!empty($elements), 'The proper block markup was found.');
- }
-
-}
diff --git a/core/modules/block/lib/Drupal/block/Tests/BlockHtmlTest.php b/core/modules/block/lib/Drupal/block/Tests/BlockHtmlTest.php
new file mode 100644
index 000000000000..a6bf31686040
--- /dev/null
+++ b/core/modules/block/lib/Drupal/block/Tests/BlockHtmlTest.php
@@ -0,0 +1,61 @@
+<?php
+
+/**
+ * @file
+ * Definition of Drupal\block\Tests\BlockHtmlTest.
+ */
+
+namespace Drupal\block\Tests;
+
+use Drupal\simpletest\WebTestBase;
+
+/**
+ * Tests block HTML ID validity.
+ */
+class BlockHtmlTest extends WebTestBase {
+
+ /**
+ * Modules to enable.
+ *
+ * @var array
+ */
+ public static $modules = array('block', 'block_test');
+
+ public static function getInfo() {
+ return array(
+ 'name' => 'Block HTML',
+ 'description' => 'Tests block HTML validity.',
+ 'group' => 'Block',
+ );
+ }
+
+ function setUp() {
+ parent::setUp();
+
+ $this->drupalLogin($this->root_user);
+
+ // Enable the test_html block, to test HTML ID and attributes.
+ \Drupal::state()->set('block_test.attributes', array('data-custom-attribute' => 'foo'));
+ \Drupal::state()->set('block_test.content', $this->randomName());
+ $this->drupalPlaceBlock('test_html', array('id' => 'test_html_block'));
+
+ // Enable a menu block, to test more complicated HTML.
+ $this->drupalPlaceBlock('system_menu_block:tools');
+ }
+
+ /**
+ * Tests for valid HTML for a block.
+ */
+ function testHtml() {
+ $this->drupalGet('');
+
+ // Ensure that a block's ID is converted to an HTML valid ID, and that
+ // block-specific attributes are added to the same DOM element.
+ $this->assertFieldByXPath('//div[@id="block-test-html-block" and @data-custom-attribute="foo"]', NULL, 'HTML ID and attributes for test block are valid and on the same DOM element.');
+
+ // Ensure expected markup for a menu block.
+ $elements = $this->xpath('//div[contains(@class, :div-class)]/div/ul[contains(@class, :ul-class)]/li', array(':div-class' => 'block-system', ':ul-class' => 'menu'));
+ $this->assertTrue(!empty($elements), 'The proper block markup was found.');
+ }
+
+}
diff --git a/core/modules/block/lib/Drupal/block/Tests/BlockInvalidRegionTest.php b/core/modules/block/lib/Drupal/block/Tests/BlockInvalidRegionTest.php
index 1faddcb128d7..b44c4e26423d 100644
--- a/core/modules/block/lib/Drupal/block/Tests/BlockInvalidRegionTest.php
+++ b/core/modules/block/lib/Drupal/block/Tests/BlockInvalidRegionTest.php
@@ -45,7 +45,7 @@ function setUp() {
*/
function testBlockInInvalidRegion() {
// Enable a test block and place it in an invalid region.
- $block = $this->drupalPlaceBlock('test_html_id');
+ $block = $this->drupalPlaceBlock('test_html');
$block->set('region', 'invalid_region');
$block->save();
diff --git a/core/modules/block/lib/Drupal/block/Tests/BlockStorageUnitTest.php b/core/modules/block/lib/Drupal/block/Tests/BlockStorageUnitTest.php
index e88f0ba7a1b0..d9cb5b10a1f1 100644
--- a/core/modules/block/lib/Drupal/block/Tests/BlockStorageUnitTest.php
+++ b/core/modules/block/lib/Drupal/block/Tests/BlockStorageUnitTest.php
@@ -9,7 +9,7 @@
use Drupal\Core\Config\Entity\ConfigStorageController;
use Drupal\simpletest\DrupalUnitTestBase;
-use Drupal\block_test\Plugin\Block\TestHtmlIdBlock;
+use Drupal\block_test\Plugin\Block\TestHtmlBlock;
use Drupal\Component\Plugin\Exception\PluginException;
use Drupal\block\Entity\Block;
use Drupal\block\BlockInterface;
@@ -78,7 +78,7 @@ protected function createTests() {
$entity = $this->controller->create(array(
'id' => 'test_block',
'theme' => 'stark',
- 'plugin' => 'test_html_id',
+ 'plugin' => 'test_html',
));
$entity->save();
@@ -97,18 +97,18 @@ protected function createTests() {
'langcode' => language_default()->id,
'theme' => 'stark',
'region' => '-1',
- 'plugin' => 'test_html_id',
+ 'plugin' => 'test_html',
'settings' => array(
- 'cache' => 1,
'label' => '',
'module' => 'block_test',
'label_display' => BlockInterface::BLOCK_LABEL_VISIBLE,
+ 'cache' => DRUPAL_NO_CACHE,
),
'visibility' => NULL,
);
$this->assertIdentical($actual_properties, $expected_properties);
- $this->assertTrue($entity->getPlugin() instanceof TestHtmlIdBlock, 'The entity has an instance of the correct block plugin.');
+ $this->assertTrue($entity->getPlugin() instanceof TestHtmlBlock, 'The entity has an instance of the correct block plugin.');
}
/**
@@ -153,7 +153,7 @@ protected function renderTests() {
$entity = $this->controller->create(array(
'id' => 'test_block2',
'theme' => 'stark',
- 'plugin' => 'test_html_id',
+ 'plugin' => 'test_html',
'settings' => array(
'label' => 'Powered by Bananas',
),
diff --git a/core/modules/block/tests/modules/block_test/config/block.block.test_block.yml b/core/modules/block/tests/modules/block_test/config/block.block.test_block.yml
index bb651744481b..7c75e4d13a77 100644
--- a/core/modules/block/tests/modules/block_test/config/block.block.test_block.yml
+++ b/core/modules/block/tests/modules/block_test/config/block.block.test_block.yml
@@ -4,9 +4,9 @@ weight: 0
status: true
langcode: en
region: '-1'
-plugin: test_html_id
+plugin: test_html
settings:
- label: 'Test block html id'
+ label: 'Test HTML block'
module: block_test
label_display: 'hidden'
cache: 1
diff --git a/core/modules/block/tests/modules/block_test/lib/Drupal/block_test/Plugin/Block/TestHtmlBlock.php b/core/modules/block/tests/modules/block_test/lib/Drupal/block_test/Plugin/Block/TestHtmlBlock.php
new file mode 100644
index 000000000000..1c6138fe00c1
--- /dev/null
+++ b/core/modules/block/tests/modules/block_test/lib/Drupal/block_test/Plugin/Block/TestHtmlBlock.php
@@ -0,0 +1,32 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\block_test\Plugin\Block\TestHtmlBlock.
+ */
+
+namespace Drupal\block_test\Plugin\Block;
+
+use Drupal\block\BlockBase;
+
+/**
+ * Provides a block to test HTML.
+ *
+ * @Block(
+ * id = "test_html",
+ * admin_label = @Translation("Test HTML block")
+ * )
+ */
+class TestHtmlBlock extends BlockBase {
+
+ /**
+ * {@inheritdoc}
+ */
+ public function build() {
+ return array(
+ '#attributes' => \Drupal::state()->get('block_test.attributes'),
+ '#children' => \Drupal::state()->get('block_test.content'),
+ );
+ }
+
+}
diff --git a/core/modules/block/tests/modules/block_test/lib/Drupal/block_test/Plugin/Block/TestHtmlIdBlock.php b/core/modules/block/tests/modules/block_test/lib/Drupal/block_test/Plugin/Block/TestHtmlIdBlock.php
deleted file mode 100644
index 5d3437aec06c..000000000000
--- a/core/modules/block/tests/modules/block_test/lib/Drupal/block_test/Plugin/Block/TestHtmlIdBlock.php
+++ /dev/null
@@ -1,19 +0,0 @@
-<?php
-
-/**
- * @file
- * Contains \Drupal\block_test\Plugin\Block\TestHtmlIdBlock.
- */
-
-namespace Drupal\block_test\Plugin\Block;
-
-/**
- * Provides a block to test HTML IDs.
- *
- * @Block(
- * id = "test_html_id",
- * admin_label = @Translation("Test block html id")
- * )
- */
-class TestHtmlIdBlock extends TestCacheBlock {
-}
diff --git a/core/modules/menu/menu.module b/core/modules/menu/menu.module
index 64a171b4dcc1..e849a3a47656 100644
--- a/core/modules/menu/menu.module
+++ b/core/modules/menu/menu.module
@@ -335,7 +335,7 @@ function menu_block_view_system_menu_block_alter(array &$build, BlockPluginInter
$menu_name = $block->getDerivativeId();
if (isset($menus[$menu_name]) && isset($build['content'])) {
foreach (element_children($build['content']) as $key) {
- $build['content']['#contextual_links']['menu'] = array(
+ $build['#contextual_links']['menu'] = array(
'route_parameters' => array('menu' => $build['content'][$key]['#original_link']['menu_name']),
);
}
diff --git a/core/modules/system/lib/Drupal/system/Tests/Entity/EntityCrudHookTest.php b/core/modules/system/lib/Drupal/system/Tests/Entity/EntityCrudHookTest.php
index 784b0acee5dd..bd36e10cb970 100644
--- a/core/modules/system/lib/Drupal/system/Tests/Entity/EntityCrudHookTest.php
+++ b/core/modules/system/lib/Drupal/system/Tests/Entity/EntityCrudHookTest.php
@@ -78,8 +78,8 @@ protected function assertHookMessageOrder($messages) {
*/
public function testBlockHooks() {
$entity = entity_create('block', array(
- 'id' => 'stark.test_html_id',
- 'plugin' => 'test_html_id',
+ 'id' => 'stark.test_html',
+ 'plugin' => 'test_html',
));
$this->assertHookMessageOrder(array(
diff --git a/core/modules/system/system.module b/core/modules/system/system.module
index 20e229da1098..a311ea1c2f53 100644
--- a/core/modules/system/system.module
+++ b/core/modules/system/system.module
@@ -9,6 +9,7 @@
use Drupal\Core\Cache\Cache;
use Drupal\Core\Language\Language;
use Drupal\Core\Utility\ModuleInfo;
+use Drupal\block\BlockPluginInterface;
use Drupal\menu_link\MenuLinkInterface;
use Drupal\user\UserInterface;
use Symfony\Component\HttpFoundation\JsonResponse;
@@ -3091,6 +3092,24 @@ function system_page_alter(&$page) {
}
}
+/**
+ * Implements hook_block_view_BASE_BLOCK_ID_alter().
+ */
+function system_block_view_system_main_block_alter(array &$build, BlockPluginInterface $block) {
+ // Contextual links on the system_main block would basically duplicate the
+ // tabs/local tasks, so reduce the clutter.
+ unset($build['#contextual_links']);
+}
+
+/**
+ * Implements hook_block_view_BASE_BLOCK_ID_alter().
+ */
+function system_block_view_system_help_block_alter(array &$build, BlockPluginInterface $block) {
+ // Assume that most users do not need or want to perform contextual actions on
+ // the help block, so don't needlessly draw attention to it.
+ unset($build['#contextual_links']);
+}
+
/**
* Run the automated cron if enabled.
*/
--
GitLab