Issue #2005546 by mdrummond, joelpittet, Manuel Garcia, lauriii, lokapujya,...

Issue #2005546 by mdrummond, joelpittet, Manuel Garcia, lauriii, lokapujya, epari.siva, hosef, rpayanm, emma.maria, HOG, zetagraph, wmortada, rteijeiro, Cottser, hussainweb, madhavvyas: Use branding block in place of page template branding variables (site name, slogan, site logo)

core/includes/theme.inc

@@ -326,14 +326,11 @@ function theme_get_setting($setting_name, $theme = NULL) {
       }
 
       // Generate the path to the logo image.
-      if ($cache[$theme]->get('features.logo')) {
-        $logo_path = $cache[$theme]->get('logo.path');
-        if ($cache[$theme]->get('logo.use_default')) {
-          $cache[$theme]->set('logo.url', file_create_url($theme_object->getPath() . '/logo.svg'));
-        }
-        elseif ($logo_path) {
-          $cache[$theme]->set('logo.url', file_create_url($logo_path));
-        }
+      if ($cache[$theme]->get('logo.use_default')) {
+        $cache[$theme]->set('logo.url', file_create_url($theme_object->getPath() . '/logo.svg'));
+      }
+      elseif ($logo_path = $cache[$theme]->get('logo.path')) {
+        $cache[$theme]->set('logo.url', file_create_url($logo_path));
       }
 
       // Generate the path to the favicon.
@@ -1292,7 +1289,6 @@ function template_preprocess_html(&$variables) {
  */
 function template_preprocess_page(&$variables) {
   $language_interface = \Drupal::languageManager()->getCurrentLanguage();
-  $site_config = \Drupal::config('system.site');
 
   // Move some variables to the top level for themer convenience and template cleanliness.
   $variables['title'] = $variables['page']['#title'];
@@ -1306,9 +1302,6 @@ function template_preprocess_page(&$variables) {
   $variables['base_path']         = base_path();
   $variables['front_page']        = \Drupal::url('<front>');
   $variables['language']          = $language_interface;
-  $variables['logo']              = theme_get_setting('logo.url');
-  $variables['site_name']         = (theme_get_setting('features.name') ? $site_config->get('name') : '');
-  $variables['site_slogan']['#markup'] = (theme_get_setting('features.slogan') ? $site_config->get('slogan') : '');
 
   // An exception might be thrown.
   try {
@@ -1413,6 +1406,13 @@ function template_preprocess_maintenance_page(&$variables) {
 
   // @see system_page_attachments()
   $variables['#attached']['library'][] = 'system/maintenance';
+
+  // Maintenance page and install page need branding info in variables because
+  // there is no blocks.
+  $site_config = \Drupal::config('system.site');
+  $variables['logo'] = theme_get_setting('logo.url');
+  $variables['site_name'] = $site_config->get('name');
+  $variables['site_slogan'] = $site_config->get('slogan');
 }
 
 /**

core/lib/Drupal/Core/Extension/ThemeHandler.php

@@ -22,10 +22,7 @@ class ThemeHandler implements ThemeHandlerInterface {
    * @var array
    */
   protected $defaultFeatures = array(
-    'logo',
     'favicon',
-    'name',
-    'slogan',
     'node_user_picture',
     'comment_user_picture',
     'comment_user_verification',

core/modules/color/color.module

@@ -8,6 +8,7 @@
 use Drupal\Core\Asset\CssOptimizer;
 use Drupal\Component\Utility\Bytes;
 use Drupal\Component\Utility\Environment;
+use Drupal\Core\Block\BlockPluginInterface;
 use Drupal\Core\Cache\Cache;
 use Drupal\Core\Form\FormStateInterface;
 use Drupal\Core\Language\LanguageInterface;
@@ -106,18 +107,25 @@ function color_library_info_alter(&$libraries, $extension) {
 }
 
 /**
- * Implements hook_preprocess_page().
- *
- * Replace the logo with the colored version if available.
+ * Implements hook_block_view_BASE_BLOCK_ID_alter().
+ */
+function color_block_view_system_branding_block_alter(array &$build, BlockPluginInterface $block) {
+  $build['#pre_render'][] = 'color_block_view_pre_render';
+}
+
+/**
+ * #pre_render callback: Sets color preset logo.
  */
-function color_preprocess_page(&$variables) {
+function color_block_view_pre_render(array $build) {
   $theme_key = \Drupal::theme()->getActiveTheme()->getName();
 
   // Override logo.
   $logo = \Drupal::config('color.theme.' . $theme_key)->get('logo');
-  if ($logo && $variables['logo'] && preg_match('!' . $theme_key . '/logo.svg$!', $variables['logo'])) {
-    $variables['logo'] = file_create_url($logo);
+  if ($logo && $build['content']['site_logo'] && preg_match('!' . $theme_key . '/logo.svg$!', $build['content']['site_logo']['#uri'])) {
+    $build['content']['site_logo']['#uri'] = file_create_url($logo);
   }
+
+  return $build;
 }
 
 /**

core/modules/config/src/Tests/ConfigLanguageOverrideWebTest.php

@@ -23,7 +23,11 @@ class ConfigLanguageOverrideWebTest extends WebTestBase {
    *
    * @var array
    */
-  public static $modules = array('language', 'system');
+  public static $modules = [
+    'block',
+    'language',
+    'system'
+  ];
 
   /**
    * {@inheritdoc}
@@ -54,6 +58,9 @@ function testSiteNameTranslation() {
       ->set('name', 'XX site name')
       ->save();
 
+    // Place branding block with site name into header region.
+    $this->drupalPlaceBlock('system_branding_block', ['region' => 'header']);
+
     $this->drupalLogout();
 
     // The home page in English should not have the override.

core/modules/config/src/Tests/ConfigSingleImportExportTest.php

@@ -22,7 +22,11 @@ class ConfigSingleImportExportTest extends WebTestBase {
    *
    * @var array
    */
-  public static $modules = array('config', 'config_test');
+  public static $modules = [
+    'block',
+    'config',
+    'config_test'
+  ];
 
   /**
    * Tests importing a single configuration file.
@@ -126,6 +130,10 @@ public function testImport() {
   public function testImportSimpleConfiguration() {
     $this->drupalLogin($this->drupalCreateUser(array('import configuration')));
     $config = $this->config('system.site')->set('name', 'Test simple import');
+
+    // Place branding block with site name into header region.
+    $this->drupalPlaceBlock('system_branding_block', ['region' => 'header']);
+
     $edit = array(
       'config_type' => 'system.simple',
       'config_name' => $config->getName(),

core/modules/config_translation/src/Tests/ConfigTranslationUiTest.php

@@ -176,6 +176,9 @@ public function testSiteInformationTranslationUi() {
     $this->assertFieldByName('translation[config_names][system.site][name]', $fr_site_name);
     $this->assertFieldByName('translation[config_names][system.site][slogan]', $fr_site_slogan);
 
+    // Place branding block with site name and slogan into header region.
+    $this->drupalPlaceBlock('system_branding_block', ['region' => 'header']);
+
     // Check French translation of site name and slogan are in place.
     $this->drupalGet('fr');
     $this->assertRaw($fr_site_name);

core/modules/language/src/Tests/LanguageUILanguageNegotiationTest.php

@@ -393,6 +393,9 @@ function testUrlLanguageFallback() {
     // it is set by JavaScript.
     $this->drupalLogout();
 
+    // Place a site branding block in the header region.
+    $this->drupalPlaceBlock('system_branding_block', ['region' => 'header']);
+
     // Access the front page without specifying any valid URL language prefix
     // and having as browser language preference a non-default language.
     $http_header = array("Accept-Language: $langcode_browser_fallback;q=1");
@@ -406,7 +409,7 @@ function testUrlLanguageFallback() {
     $this->assertTrue($fields[0] == $languages[$langcode_browser_fallback]->getName(), 'The browser language is the URL active language');
 
     // Check that URLs are rewritten using the given browser language.
-    $fields = $this->xpath('//strong[@class="site-name"]/a[@rel="home" and @href=:url]', $args);
+    $fields = $this->xpath('//div[@class="site-name"]/a[@rel="home" and @href=:url]', $args);
     $this->assertTrue($fields[0] == 'Drupal', 'URLs are rewritten using the browser language.');
   }
 

core/modules/page_cache/src/Tests/PageCacheTagsIntegrationTest.php

@@ -91,6 +91,7 @@ function testPageCacheTags() {
       'rendered',
       'block_view',
       'config:block_list',
+      'config:block.block.bartik_branding',
       'config:block.block.bartik_breadcrumbs',
       'config:block.block.bartik_content',
       'config:block.block.bartik_tools',
@@ -127,6 +128,7 @@ function testPageCacheTags() {
       'rendered',
       'block_view',
       'config:block_list',
+      'config:block.block.bartik_branding',
       'config:block.block.bartik_breadcrumbs',
       'config:block.block.bartik_content',
       'config:block.block.bartik_tools',

core/modules/system/config/install/system.theme.global.yml

@@ -7,10 +7,7 @@ features:
   comment_user_picture: true
   comment_user_verification: true
   favicon: true
-  logo: true
-  name: true
   node_user_picture: true
-  slogan: true
 logo:
   path: ''
   url: ''

core/modules/system/src/Form/ThemeSettingsForm.php

@@ -141,9 +141,6 @@ public function buildForm(array $form, FormStateInterface $form_state, $theme =
 
     // Toggle settings
     $toggles = array(
-      'logo' => t('Logo'),
-      'name' => t('Site name'),
-      'slogan' => t('Site slogan'),
       'node_user_picture' => t('User pictures in posts'),
       'comment_user_picture' => t('User pictures in comments'),
       'comment_user_verification' => t('User verification status in comments'),

core/modules/system/src/Plugin/Block/SystemBrandingBlock.php

@@ -160,10 +160,9 @@ public function build() {
     $build = array();
     $site_config = $this->configFactory->get('system.site');
 
-    $logo = theme_get_setting('logo');
     $build['site_logo'] = array(
       '#theme' => 'image',
-      '#uri' => $logo['url'],
+      '#uri' => theme_get_setting('logo.url'),
       '#alt' => $this->t('Home'),
       '#access' => $this->configuration['use_site_logo'],
     );

core/modules/system/src/Tests/System/PageTitleTest.php

@@ -23,7 +23,7 @@ class PageTitleTest extends WebTestBase {
    *
    * @var array
    */
-  public static $modules = array('node', 'test_page_test', 'form_test');
+  public static $modules = ['node', 'test_page_test', 'form_test', 'block'];
 
   protected $contentUser;
   protected $savedTitle;
@@ -71,13 +71,6 @@ function testTitleXSS() {
     $slogan = '<script type="text/javascript">alert("Slogan XSS!");</script>';
     $slogan_filtered = Xss::filterAdmin($slogan);
 
-    // Activate needed appearance settings.
-    $edit = array(
-      'toggle_name'           => TRUE,
-      'toggle_slogan'         => TRUE,
-    );
-    $this->drupalPostForm('admin/appearance/settings', $edit, t('Save configuration'));
-
     // Set title and slogan.
     $edit = array(
       'site_name'    => $title,
@@ -85,6 +78,9 @@ function testTitleXSS() {
     );
     $this->drupalPostForm('admin/config/system/site-information', $edit, t('Save configuration'));
 
+    // Place branding block with site name and slogan into header region.
+    $this->drupalPlaceBlock('system_branding_block', ['region' => 'header']);
+
     // Load frontpage.
     $this->drupalGet('');
 

core/modules/system/src/Tests/System/ThemeTest.php

@@ -123,9 +123,11 @@ function testThemeSettings() {
       $this->assertEqual((string) $elements[1], $explicit_file);
       $this->assertEqual((string) $elements[2], $local_file);
 
-      // Verify the actual 'src' attribute of the logo being output.
+      // Verify the actual 'src' attribute of the logo being output in a site
+      // branding block.
+      $this->drupalPlaceBlock('system_branding_block', ['region' => 'header']);
       $this->drupalGet('');
-      $elements = $this->xpath('//header/a[@rel=:rel]/img', array(
+      $elements = $this->xpath('//header//a[@rel=:rel]/img', array(
           ':rel' => 'home',
         )
       );
@@ -175,8 +177,9 @@ function testThemeSettings() {
     $fields = $this->xpath($this->constructFieldXpath('name', 'logo_path'));
     $uploaded_filename = 'public://' . $fields[0]['value'];
 
+    $this->drupalPlaceBlock('system_branding_block', ['region' => 'header']);
     $this->drupalGet('');
-    $elements = $this->xpath('//header/a[@rel=:rel]/img', array(
+    $elements = $this->xpath('//header//a[@rel=:rel]/img', array(
         ':rel' => 'home',
       )
     );

core/modules/system/src/Tests/Update/SiteBrandingConvertedIntoBlockUpdateTest.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\system\Tests\Update\LocalActionsAndTasksConvertedIntoBlocksUpdateTest.
+ */
+
+namespace Drupal\system\Tests\Update;
+
+use Drupal\node\Entity\Node;
+
+/**
+ * Tests the upgrade path for local actions/tasks being converted into blocks.
+ *
+ * @see https://www.drupal.org/node/507488
+ *
+ * @group system
+ */
+class SiteBrandingConvertedIntoBlockUpdateTest extends UpdatePathTestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setDatabaseDumpFiles() {
+    $this->databaseDumpFiles = [
+      __DIR__ . '/../../../../system/tests/fixtures/update/drupal-8.bare.standard.php.gz',
+      __DIR__ . '/../../../../system/tests/fixtures/update/drupal-8.site-branding-into-block-2005546.php',
+    ];
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp() {
+    parent::setUp();
+    /** @var \Drupal\Core\Extension\ThemeHandlerInterface $theme_handler */
+    $theme_handler = \Drupal::service('theme_handler');
+    $theme_handler->refreshInfo();
+  }
+
+  /**
+   * Tests that site branding elements are being converted into blocks.
+   */
+  public function testUpdateHookN() {
+    $this->runUpdates();
+
+    /** @var \Drupal\block\BlockInterface $block_storage */
+    $block_storage = \Drupal::entityManager()->getStorage('block');
+
+    $this->assertRaw('Because your site has custom theme(s) installed, we had to set the branding block into the content region. Please manually review the block configuration and remove the site name, slogan, and logo variables from your templates.');
+
+    // Disable maintenance mode.
+    // @todo Can be removed once maintenance mode is automatically turned off
+    // after updates in https://www.drupal.org/node/2435135.
+    \Drupal::state()->set('system.maintenance_mode', FALSE);
+
+    // We finished updating so we can login the user now.
+    $this->drupalLogin($this->rootUser);
+
+    // Site branding is visible on the home page.
+    $this->drupalGet('/node');
+    $this->assertRaw('site-branding__logo');
+    $this->assertRaw('site-branding__name');
+    $this->assertNoRaw('site-branding__slogan');
+
+    $this->drupalGet('admin/structure/block/list/bartik');
+
+    /** @var \Drupal\Core\Config\StorageInterface $config_storage */
+    $config_storage = \Drupal::service('config.storage');
+    $this->assertTrue($config_storage->exists('block.block.test_theme_branding'), 'Site branding block has been created for the custom theme.');
+  }
+
+}

core/modules/system/system.install

@@ -1320,7 +1320,8 @@ function system_update_8004() {
   // https://www.drupal.org/node/2542748. Regenerate the related schemas to
   // ensure they match the currently expected status.
   $manager = \Drupal::entityDefinitionUpdateManager();
-  foreach (array_keys(\Drupal::entityManager()->getDefinitions()) as $entity_type_id) {
+  foreach (array_keys(\Drupal::entityManager()
+    ->getDefinitions()) as $entity_type_id) {
     $manager->updateEntityType($manager->getEntityType($entity_type_id));
   }
 }
@@ -1475,7 +1476,78 @@ function system_update_8005() {
 }
 
 /**
- * Helper function to create block configuration objects for the update.
+ * Place branding blocks in every theme.
+ */
+function system_update_8006() {
+  // When block module is not installed, there is nothing that could be done
+  // except showing a warning.
+  if (!\Drupal::moduleHandler()->moduleExists('block')) {
+    return t('Block module is not enabled so site branding elements, which have been converted to a block, are not visible anymore.');
+  }
+
+  /** @var \Drupal\Core\Extension\ThemeHandlerInterface $theme_handler */
+  $theme_handler = \Drupal::service('theme_handler');
+  $custom_themes_installed = FALSE;
+  $message = NULL;
+  $langcode = \Drupal::service('language_manager')->getCurrentLanguage()->getId();
+
+  $site_branding_default_settings = [
+    'plugin' => 'system_branding_block',
+    'region' => 'content',
+    'settings.label' => 'Site Branding',
+    'settings.label_display' => 0,
+    'settings.cache.max_age' => 0,
+    'visibility' => [],
+    'weight' => 0,
+    'langcode' => $langcode,
+  ];
+  foreach ($theme_handler->listInfo() as $theme) {
+    $theme_name = $theme->getName();
+    switch ($theme_name) {
+      case 'bartik':
+        $name = 'block.block.bartik_branding';
+        $values = [
+            'id' => 'bartik_branding',
+            'region' => 'header',
+          ] + $site_branding_default_settings;
+        _system_update_create_block($name, $theme_name, $values);
+        break;
+
+      case 'stark':
+        $name = 'block.block.stark_branding';
+        $values = [
+            'id' => 'stark_branding',
+            'region' => 'header',
+          ] + $site_branding_default_settings;
+        _system_update_create_block($name, $theme_name, $values);
+        break;
+
+      case 'seven':
+      case 'classy':
+        // Don't place any blocks or trigger custom themes installed warning.
+        break;
+      default:
+        $custom_themes_installed = TRUE;
+        $name = sprintf('block.block.%s_branding', $theme_name);
+        $values = [
+            'id' => sprintf('%s_branding', $theme_name),
+            'region' => 'content',
+            'weight' => '-50',
+          ] + $site_branding_default_settings;
+        _system_update_create_block($name, $theme_name, $values);
+        break;
+    }
+  }
+
+  if ($custom_themes_installed) {
+    $message = t('Because your site has custom theme(s) installed, we had to set the branding block into the content region. Please manually review the block configuration and remove the site name, slogan, and logo variables from your templates.');
+  }
+
+  return $message;
+}
+
+/**
+ * Helper function to create block configuration objects for an update.
  *
  * @param string $name
  *   The name of the config object.

core/modules/system/system.module

@@ -1063,10 +1063,7 @@ function system_rebuild_module_data() {
  */
 function _system_default_theme_features() {
   return array(
-    'logo',
     'favicon',
-    'name',
-    'slogan',
     'node_user_picture',
     'comment_user_picture',
     'comment_user_verification',

core/modules/system/templates/block--system-branding-block.html.twig

@@ -17,12 +17,12 @@
 #}
 {% block content %}
   {% if site_logo %}
-    <a href="{{ url('<front>') }}" title="{{ 'Home'|t }}" rel="home">
+    <a href="{{ path('<front>') }}" title="{{ 'Home'|t }}" rel="home">
       <img src="{{ site_logo }}" alt="{{ 'Home'|t }}" />
     </a>
   {% endif %}
   {% if site_name %}
-    <a href="{{ url('<front>') }}" title="{{ 'Home'|t }}" rel="home">{{ site_name }}</a>
+    <a href="{{ path('<front>') }}" title="{{ 'Home'|t }}" rel="home">{{ site_name }}</a>
   {% endif %}
   {{ site_slogan }}
 {% endblock %}

core/modules/system/templates/page.html.twig

@@ -58,32 +58,6 @@
 <div class="layout-container">
 
   <header role="banner">
-    {% if logo %}
-      <a href="{{ front_page }}" title="{{ 'Home'|t }}" rel="home">
-        <img src="{{ logo }}" alt="{{ 'Home'|t }}"/>
-      </a>
-    {% endif %}
-
-    {% if site_name or site_slogan %}
-      <div class="name-and-slogan">
-
-        {# Use h1 when the content title is empty #}
-        {% if title %}
-          <strong class="site-name">
-            <a href="{{ front_page }}" title="{{ 'Home'|t }}" rel="home">{{ site_name }}</a>
-          </strong>
-        {% else %}
-          <h1 class="site-name">
-            <a href="{{ front_page }}" title="{{ 'Home'|t }}" rel="home">{{ site_name }}</a>
-          </h1>
-        {% endif %}
-
-        {% if site_slogan %}
-          <div class="site-slogan">{{ site_slogan }}</div>
-        {% endif %}
-      </div>{# ./name-and-slogan #}
-    {% endif %}
-
     {{ page.header }}
   </header>
 

core/modules/system/tests/fixtures/update/block.block.testfor2005546.yml

+uuid: b299258f-6fb5-47f8-b42f-473787d02e22
+langcode: en
+status: true
+dependencies:
+  module:
+    - system
+  theme:
+    - bartik
+id: bartik_branding
+theme: bartik
+region: header
+weight: 0
+provider: null
+plugin: system_branding_block
+settings:
+  id: system_branding_block
+  label: 'Site Branding'
+  provider: system
+  label_display: '0'
+  cache:
+    max_age: -1
+  use_site_logo: true
+  use_site_name: true
+  use_site_slogan: true
+visibility: {  }

core/modules/system/tests/fixtures/update/drupal-8.site-branding-into-block-2005546.php

+<?php
+
+/**
+ * @file
+ * Contains database additions to drupal-8.bare.standard.php.gz for testing the
+ * upgrade path of https://www.drupal.org/node/2005546.
+ */
+
+use Drupal\Core\Database\Database;
+
+$connection = Database::getConnection();
+
+// Structure of a custom block with visibility settings.
+$block_configs[] = \Drupal\Component\Serialization\Yaml::decode(file_get_contents(__DIR__ . '/block.block.testfor2005546.yml'));
+
+foreach ($block_configs as $block_config) {
+  $connection->insert('config')
+    ->fields([
+      'collection',
+      'name',
+      'data',
+    ])
+    ->values([
+      'collection' => '',
+      'name' => 'block.block.' . $block_config['id'],
+      'data' => serialize($block_config),
+    ])
+    ->execute();
+}
+
+// Update the config entity query "index".
+$existing_blocks = $connection->select('key_value')
+  ->fields('key_value', ['value'])
+  ->condition('collection', 'config.entity.key_store.block')
+  ->condition('name', 'theme:bartik')
+  ->execute()
+  ->fetchField();
+$existing_blocks = unserialize($existing_blocks);
+
+$connection->update('key_value')
+  ->fields([
+    'value' => serialize(array_merge($existing_blocks, ['block.block.bartik_branding']))
+  ])
+  ->condition('collection', 'config.entity.key_store.block')
+  ->condition('name', 'theme:bartik')
+  ->execute();
+
+// Enable test theme.
+$extensions = $connection->select('config')
+  ->fields('config', ['data'])
+  ->condition('name', 'core.extension')
+  ->execute()
+  ->fetchField();
+$extensions = unserialize($extensions);
+$connection->update('config')
+  ->fields([
+    'data' => serialize(array_merge_recursive($extensions, ['theme' => ['test_theme' => 0]]))
+  ])
+  ->condition('name', 'core.extension')
+  ->execute();

core/profiles/minimal/config/install/block.block.stark_branding.yml

+id: stark_branding
+theme: stark
+weight: 0
+status: true
+langcode: en
+region: header
+plugin: system_branding_block
+settings:
+  id: system_branding_block
+  label: 'Site Branding'
+  provider: system
+  label_display: '0'
+dependencies:
+  module:
+    - system
+  theme:
+    - stark
+visibility: {  }

core/profiles/standard/config/install/block.block.bartik_branding.yml

+id: bartik_branding
+theme: bartik
+weight: 0
+status: true
+langcode: en
+region: header
+plugin: system_branding_block
+settings:
+  id: system_branding_block
+  label: 'Site Branding'
+  provider: system
+  label_display: '0'
+dependencies:
+  module:
+    - system
+  theme:
+    - bartik
+visibility: {  }

core/themes/bartik/bartik.libraries.yml

@@ -37,6 +37,7 @@ global-styling:
       css/components/shortcut.css: {}
       css/components/skip-link.css: {}
       css/components/sidebar.css: {}
+      css/components/site-branding.css: {}
       css/components/site-footer.css: {}
       css/components/table.css: {}
       css/components/tabs.css: {}