Issue #2032535 by dawehner, katbailey, pwolanin, Crell: Resolve 'title/title callback' by adding a _title propery on the route and allow a title be part of the render array.

core/core.services.yml

@@ -350,7 +350,7 @@ services:
       - { name: event_subscriber }
   controller.page:
     class: Drupal\Core\Controller\HtmlPageController
-    arguments: ['@http_kernel']
+    arguments: ['@http_kernel', '@controller_resolver']
   controller.dialog:
     class: Drupal\Core\Controller\DialogController
     arguments: ['@http_kernel']

core/includes/bootstrap.inc

@@ -10,6 +10,7 @@ use Drupal\Component\Utility\Url;
 use Drupal\Core\DrupalKernel;
 use Drupal\Core\Database\Database;
 use Drupal\Core\DependencyInjection\ContainerBuilder;
+use Drupal\Core\Utility\Title;
 use Symfony\Component\ClassLoader\ClassLoader;
 use Symfony\Component\ClassLoader\ApcClassLoader;
 use Symfony\Component\DependencyInjection\ContainerInterface;
@@ -201,13 +202,10 @@ const DRUPAL_EXTENSION_NAME_MAX_LENGTH = 50;
  */
 define('REQUEST_TIME', (int) $_SERVER['REQUEST_TIME']);
 
-/**
- * Flag for drupal_set_title(); text is not sanitized, so run check_plain().
- */
-const CHECK_PLAIN = 0;
-
 /**
  * Flag for drupal_set_title(); text has already been sanitized.
+ *
+ * @todo Move to the Title class.
  */
 const PASS_THROUGH = -1;
 
@@ -1696,7 +1694,7 @@ function drupal_get_title() {
  *   Optional string value to assign to the page title; or if set to NULL
  *   (default), leaves the current title unchanged.
  * @param $output
- *   Optional flag - normally should be left as CHECK_PLAIN. Only set to
+ *   Optional flag - normally should be left as Title::CHECK_PLAIN. Only set to
  *   PASS_THROUGH if you have already removed any possibly dangerous code
  *   from $title using a function like check_plain() or filter_xss(). With this
  *   flag the string will be passed through unchanged.
@@ -1704,7 +1702,7 @@ function drupal_get_title() {
  * @return
  *   The updated title of the current page.
  */
-function drupal_set_title($title = NULL, $output = CHECK_PLAIN) {
+function drupal_set_title($title = NULL, $output = Title::CHECK_PLAIN) {
   $stored_title = &drupal_static(__FUNCTION__);
 
   if (isset($title)) {

core/includes/common.inc

@@ -3646,6 +3646,11 @@ function drupal_pre_render_dropbutton($element) {
 function drupal_render_page($page) {
   $main_content_display = &drupal_static('system_main_content_added', FALSE);
 
+  // Pull out the page title to set it back later.
+  if (is_array($page) && isset($page['#title'])) {
+    $title = $page['#title'];
+  }
+
   // Allow menu callbacks to return strings or arbitrary arrays to render.
   // If the array returned is not of #type page directly, we need to fill
   // in the page with defaults.
@@ -3670,6 +3675,11 @@ function drupal_render_page($page) {
     $page['content']['system_main'] = drupal_set_page_content();
   }
 
+  // Set back the previously stored title.
+  if (isset($title)) {
+    $page['#title'] = $title;
+  }
+
   return drupal_render($page);
 }
 

core/includes/theme.inc

@@ -8,6 +8,7 @@
  * customized by user themes.
  */
 
+use Drupal\Component\Utility\String;
 use Drupal\Core\Cache\CacheBackendInterface;
 use Drupal\Core\Config\Config;
 use Drupal\Core\Language\Language;
@@ -2565,6 +2566,7 @@ function template_preprocess_html(&$variables) {
       $head_title['slogan'] = strip_tags(filter_xss_admin($site_config->get('slogan')));
     }
   }
+
   $variables['head_title_array'] = $head_title;
   $variables['head_title'] = implode(' | ', $head_title);
 
@@ -2675,7 +2677,13 @@ function template_preprocess_page(&$variables) {
   $variables['site_name']         = (theme_get_setting('features.name') ? check_plain($site_config->get('name')) : '');
   $variables['site_slogan']       = (theme_get_setting('features.slogan') ? filter_xss_admin($site_config->get('slogan')) : '');
   $variables['tabs']              = menu_local_tabs();
-  $variables['title']             = new RenderWrapper('drupal_get_title');
+
+  if (isset($variables['page']['#title'])) {
+    $variables['title'] = $variables['page']['#title'];
+  }
+  else {
+    $variables['title'] = new RenderWrapper('drupal_get_title');
+  }
 
   // Pass the main menu and secondary menu to the template as render arrays.
   if (!empty($variables['main_menu'])) {
@@ -2844,11 +2852,17 @@ function template_preprocess_maintenance_page(&$variables) {
   $site_name = $site_config->get('name');
   $site_slogan = $site_config->get('slogan');
 
-  // Construct page title
-  if (drupal_get_title()) {
+   // Construct the page title.
+  if (isset($variables['page']['#title'])) {
+    $head_title = array(
+      'title' => strip_tags($variables['page']['#title']),
+      'name' => String::checkPlain($site_config->get('name')),
+    );
+  }
+  elseif (drupal_get_title()) {
     $head_title = array(
       'title' => strip_tags(drupal_get_title()),
-      'name' => check_plain($site_name),
+      'name' => String::checkPlain($site_config->get('name')),
     );
   }
   else {
@@ -2889,7 +2903,6 @@ function template_preprocess_maintenance_page(&$variables) {
   $variables['site_name']         = (theme_get_setting('features.name') ? check_plain($site_name) : '');
   $variables['site_slogan']       = (theme_get_setting('features.slogan') ? filter_xss_admin($site_slogan) : '');
   $variables['tabs']              = '';
-  $variables['title']             = drupal_get_title();
 
   // Compile a list of classes that are going to be applied to the body element.
   $variables['attributes']['class'][] = 'maintenance-page';
@@ -2925,6 +2938,14 @@ function template_preprocess_maintenance_page(&$variables) {
   // be called when printed.
   $variables['styles'] = new RenderWrapper('drupal_get_css', array($css));
   $variables['scripts'] = new RenderWrapper('drupal_get_js');
+
+  // Allow the page to define a title.
+  if (isset($variables['page']['#title'])) {
+    $variables['title'] = $variables['page']['#title'];
+  }
+  if (!isset($variables['title'])) {
+    $variables['title'] = drupal_get_title();
+  }
 }
 
 /**

core/lib/Drupal/Core/Controller/ControllerResolver.php

@@ -28,7 +28,7 @@ use Symfony\Component\DependencyInjection\ContainerInterface;
  *    controller by using a service:method notation (Symfony uses the same
  *    convention).
  */
-class ControllerResolver extends BaseControllerResolver {
+class ControllerResolver extends BaseControllerResolver implements ControllerResolverInterface {
 
   /**
    * The injection container that should be injected into all controllers.
@@ -37,6 +37,13 @@ class ControllerResolver extends BaseControllerResolver {
    */
   protected $container;
 
+  /**
+   * The PSR-3 logger. (optional)
+   *
+   * @var \Psr\Log\LoggerInterface;
+   */
+  protected $logger;
+
   /**
    * Constructs a new ControllerResolver.
    *
@@ -51,6 +58,47 @@ class ControllerResolver extends BaseControllerResolver {
     parent::__construct($logger);
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function getControllerFromDefinition($controller, $path = '') {
+    if (is_array($controller) || (is_object($controller) && method_exists($controller, '__invoke'))) {
+      return $controller;
+    }
+
+    if (strpos($controller, ':') === FALSE) {
+      if (method_exists($controller, '__invoke')) {
+        return new $controller;
+      }
+      elseif (function_exists($controller)) {
+        return $controller;
+      }
+    }
+
+    $callable = $this->createController($controller);
+
+    if (!is_callable($callable)) {
+      throw new \InvalidArgumentException(sprintf('The controller for URI "%s" is not callable.', $path));
+    }
+
+    return $callable;
+  }
+
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getController(Request $request) {
+    if (!$controller = $request->attributes->get('_controller')) {
+      if ($this->logger !== NULL) {
+        $this->logger->warning('Unable to look for the controller as the "_controller" parameter is missing');
+      }
+
+      return FALSE;
+    }
+    return $this->getControllerFromDefinition($controller, $request->getPathInfo());
+  }
+
   /**
    * Returns a callable for the given controller.
    *

core/lib/Drupal/Core/Controller/ControllerResolverInterface.php

@@ -0,0 +1,37 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Controller\ControllerResolverInterface.
+ */
+
+namespace Drupal\Core\Controller;
+
+use Symfony\Component\HttpKernel\Controller\ControllerResolverInterface as BaseControllerResolverInterface;
+
+/**
+ * Extends the ControllerResolverInterface from symfony.
+ */
+interface ControllerResolverInterface extends BaseControllerResolverInterface {
+
+  /**
+   * Returns the Controller instance with a given controller route definition.
+   *
+   * As several resolvers can exist for a single application, a resolver must
+   * return false when it is not able to determine the controller.
+   *
+   * @param mixed $controller
+   *   The controller attribute like in $request->attributes->get('_controller')
+   *
+   * @return mixed|bool
+   *   A PHP callable representing the Controller, or false if this resolver is
+   *   not able to determine the controller
+   *
+   * @throws \InvalidArgumentException|\LogicException
+   *   Thrown if the controller can't be found.
+   *
+   * @see \Symfony\Component\HttpKernel\Controller\ControllerResolverInterface::getController()
+   */
+  public function getControllerFromDefinition($controller);
+
+}

core/lib/Drupal/Core/Controller/HtmlPageController.php

@@ -23,13 +23,23 @@ class HtmlPageController {
    */
   protected $httpKernel;
 
+  /**
+   * The controller resolver.
+   *
+   * @var \Drupal\Core\Controller\ControllerResolverInterface
+   */
+  protected $controllerResolver;
+
   /**
    * Constructs a new HtmlPageController.
    *
    * @param \Symfony\Component\HttpKernel\HttpKernelInterface $kernel
+   * @param \Drupal\Core\Controller\ControllerResolverInterface $controller_resolver
+   *   The controller resolver.
    */
-  public function __construct(HttpKernelInterface $kernel) {
+  public function __construct(HttpKernelInterface $kernel, ControllerResolverInterface $controller_resolver) {
     $this->httpKernel = $kernel;
+    $this->controllerResolver = $controller_resolver;
   }
 
   /**
@@ -44,28 +54,24 @@ class HtmlPageController {
    *   A response object.
    */
   public function content(Request $request, $_content) {
-
-    // @todo When we have a Generator, we can replace the forward() call with
-    // a render() call, which would handle ESI and hInclude as well.  That will
-    // require an _internal route.  For examples, see:
-    // https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/Resources/config/routing/internal.xml
-    // https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/Controller/InternalController.php
-    $attributes = clone $request->attributes;
-    $controller = $_content;
-
-    // We need to clean off the derived information and such so that the
-    // subrequest can be processed properly without leaking data through.
-    $attributes->remove('_system_path');
-    $attributes->remove('_content');
-
-    $response = $this->httpKernel->forward($controller, $attributes->all(), $request->query->all());
-
-    // For successful (HTTP status 200) responses, decorate with blocks.
-    if ($response->isOk()) {
-      $page_content = $response->getContent();
-      $response = new Response(drupal_render_page($page_content));
+    $callable = $this->controllerResolver->getControllerFromDefinition($_content);
+    $arguments = $this->controllerResolver->getArguments($request, $callable);
+    $page_content = call_user_func_array($callable, $arguments);
+    if ($page_content instanceof Response) {
+      return $page_content;
+    }
+    if (!is_array($page_content)) {
+      $page_content = array(
+        '#markup' => $page_content,
+      );
+    }
+    // If no title was returned fall back to one defined in the route.
+    if (!isset($page_content['#title']) && $request->attributes->has('_title')) {
+      $page_content['#title'] = $request->attributes->get('_title');
     }
 
+    $response = new Response(drupal_render_page($page_content));
     return $response;
   }
+
 }

core/lib/Drupal/Core/Utility/Title.php

@@ -0,0 +1,20 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Utility\Title.
+ */
+
+namespace Drupal\Core\Utility;
+
+/**
+ * Defines some constants related with Drupal page title.
+ */
+class Title {
+
+  /**
+   * Flag for controller titles, for sanitizing via String::checkPlain
+   */
+  const CHECK_PLAIN = 0;
+
+}

core/modules/block/block.routing.yml

@@ -24,5 +24,6 @@ block_admin_add:
   pattern: '/admin/structure/block/add/{plugin_id}/{theme}'
   defaults:
     _content: '\Drupal\block\Controller\BlockAddController::blockAddConfigureForm'
+    _title: 'Configure block'
   requirements:
     _permission: 'administer blocks'

core/modules/block/lib/Drupal/block/Controller/BlockAddController.php

@@ -27,9 +27,6 @@ class BlockAddController extends ControllerBase {
    *   The block instance edit form.
    */
   public function blockAddConfigureForm($plugin_id, $theme) {
-    // Set the page title.
-    drupal_set_title(t('Configure block'));
-
     // Create a block entity.
     $entity = $this->entityManager()->getStorageController('block')->create(array('plugin' => $plugin_id, 'theme' => $theme));
 

core/modules/system/lib/Drupal/system/Tests/System/PageTitleFilteringTest.php

@@ -2,22 +2,24 @@
 
 /**
  * @file
- * Definition of Drupal\system\Tests\System\PageTitleFilteringTest.
+ * Contains \Drupal\system\Tests\System\PageTitleTest.
  */
 
 namespace Drupal\system\Tests\System;
 
+use Drupal\Component\Utility\String;
 use Drupal\Core\Language\Language;
+use Drupal\Core\Utility\Title;
 use Drupal\simpletest\WebTestBase;
 
-class PageTitleFilteringTest extends WebTestBase {
+class PageTitleTest extends WebTestBase {
 
   /**
    * Modules to enable.
    *
    * @var array
    */
-  public static $modules = array('node');
+  public static $modules = array('node', 'test_page_test');
 
   protected $content_user;
   protected $saved_title;
@@ -27,7 +29,7 @@ class PageTitleFilteringTest extends WebTestBase {
    */
   public static function getInfo() {
     return array(
-      'name' => 'HTML in page titles',
+      'name' => 'Page titles',
       'description' => 'Tests correct handling or conversion by drupal_set_title() and drupal_get_title() and checks the correct escaping of site name and slogan.',
       'group' => 'System'
     );
@@ -61,10 +63,10 @@ class PageTitleFilteringTest extends WebTestBase {
    */
   function testTitleTags() {
     $title = "string with <em>HTML</em>";
-    // drupal_set_title's $filter is CHECK_PLAIN by default, so the title should be
+    // drupal_set_title's $filter is Title::CHECK_PLAIN by default, so the title should be
     // returned with check_plain().
-    drupal_set_title($title, CHECK_PLAIN);
-    $this->assertTrue(strpos(drupal_get_title(), '<em>') === FALSE, 'Tags in title converted to entities when $output is CHECK_PLAIN.');
+    drupal_set_title($title, Title::CHECK_PLAIN);
+    $this->assertTrue(strpos(drupal_get_title(), '<em>') === FALSE, 'Tags in title converted to entities when $output is Title::CHECK_PLAIN.');
     // drupal_set_title's $filter is passed as PASS_THROUGH, so the title should be
     // returned with HTML.
     drupal_set_title($title, PASS_THROUGH);
@@ -122,4 +124,18 @@ class PageTitleFilteringTest extends WebTestBase {
     $this->assertNoRaw($slogan, 'Check for the unfiltered version of the slogan.');
     $this->assertRaw($slogan_filtered, 'Check for the filtered version of the slogan.');
   }
+
+  /**
+   * Tests the page title of render arrays.
+   *
+   * @see \Drupal\test_page_test\Controller\Test::renderTitle()
+   */
+  public function testRenderTitle() {
+    $this->drupalGet('test-render-title');
+
+    $this->assertTitle('Foo | Drupal');
+    $result = $this->xpath('//h1');
+    $this->assertEqual('Foo', (string) $result[0]);
+  }
+
 }

core/modules/system/tests/modules/test_page_test/lib/Drupal/test_page_test/Controller/Test.php

@@ -0,0 +1,29 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\test_page_test\Controller\Test.
+ */
+
+namespace Drupal\test_page_test\Controller;
+
+/**
+ * Defines a test controller for page titles.
+ */
+class Test {
+
+  /**
+   * Renders a page with a title.
+   *
+   * @return array
+   *   A render array as expected by drupal_render()
+   */
+  public function renderTitle() {
+    $build = array();
+    $build['#markup'] = 'Hello Drupal';
+    $build['#title'] = 'Foo';
+
+    return $build;
+  }
+
+}

core/modules/system/tests/modules/test_page_test/test_page_test.routing.yml

@@ -0,0 +1,6 @@
+test_page_render_title:
+  pattern: "/test-render-title"
+  defaults:
+    _content: 'Drupal\test_page_test\Controller\Test::renderTitle'
+  requirements:
+    _access: 'TRUE'