Issue #2171269 by effulgentsia, Wim Leers, InternetDevels, snig: Handle block rendering's #attributes correctly (fixes in-place editing of custom blocks).

8.0.x
Nathaniel Catchpole 2014-02-05 13:22:50 +00:00
parent e3b24655b8
commit 4b1d3103e9
12 changed files with 159 additions and 122 deletions

View File

@ -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';

View File

@ -36,7 +36,8 @@ class BlockViewBuilder extends EntityViewBuilder {
*/
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 @@ class BlockViewBuilder extends EntityViewBuilder {
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;
}

View File

@ -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.');
}
}

View File

@ -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.');
}
}

View File

@ -45,7 +45,7 @@ class BlockInvalidRegionTest extends WebTestBase {
*/
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();

View File

@ -9,7 +9,7 @@ namespace Drupal\block\Tests;
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 @@ class BlockStorageUnitTest extends DrupalUnitTestBase {
$entity = $this->controller->create(array(
'id' => 'test_block',
'theme' => 'stark',
'plugin' => 'test_html_id',
'plugin' => 'test_html',
));
$entity->save();
@ -97,18 +97,18 @@ class BlockStorageUnitTest extends DrupalUnitTestBase {
'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 @@ class BlockStorageUnitTest extends DrupalUnitTestBase {
$entity = $this->controller->create(array(
'id' => 'test_block2',
'theme' => 'stark',
'plugin' => 'test_html_id',
'plugin' => 'test_html',
'settings' => array(
'label' => 'Powered by Bananas',
),

View File

@ -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

View File

@ -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'),
);
}
}

View File

@ -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 {
}

View File

@ -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']),
);
}

View File

@ -78,8 +78,8 @@ class EntityCrudHookTest extends EntityUnitTestBase {
*/
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(

View File

@ -9,6 +9,7 @@ use Drupal\Core\Cache\CacheBackendInterface;
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.
*/