Issue #2068471 by dawehner, Crell, tim.plunkett, jibran, fubhy, larowlan:...

Issue #2068471 by dawehner, Crell, tim.plunkett, jibran, fubhy, larowlan: Normalize Controller/View-listener behavior with a Page object.

core/core.services.yml

@@ -384,16 +384,16 @@ services:
     class: Drupal\Core\Routing\Enhancer\AjaxEnhancer
     arguments: ['@content_negotiation']
     tags:
-      - { name: route_enhancer, priority: 20 }
-      - { name: legacy_route_enhancer, priority: 20 }
+      - { name: route_enhancer, priority: 15 }
+      - { name: legacy_route_enhancer, priority: 15 }
   route_enhancer.entity:
     class: Drupal\Core\Entity\Enhancer\EntityRouteEnhancer
-    arguments: ['@content_negotiation']
+    arguments: ['@controller_resolver', '@entity.manager', '@form_builder']
     tags:
-      - { name: route_enhancer, priority: 15 }
+      - { name: route_enhancer, priority: 20 }
   route_enhancer.form:
     class: Drupal\Core\Routing\Enhancer\FormEnhancer
-    arguments: ['@content_negotiation']
+    arguments: ['@service_container', '@controller_resolver', '@form_builder']
     tags:
       - { name: route_enhancer, priority: 10 }
   route_special_attributes_subscriber:
@@ -402,10 +402,16 @@ services:
       - { name: event_subscriber }
   controller.page:
     class: Drupal\Core\Controller\HtmlPageController
-    arguments: ['@http_kernel', '@controller_resolver', '@string_translation', '@title_resolver']
+    arguments: ['@controller_resolver', '@string_translation', '@title_resolver']
+  controller.ajax:
+    class: Drupal\Core\Controller\AjaxController
+    arguments: ['@controller_resolver']
+  controller.entityform:
+    class: Drupal\Core\Entity\HtmlEntityFormController
+    arguments: ['@controller_resolver', '@service_container', '@entity.manager']
   controller.dialog:
     class: Drupal\Core\Controller\DialogController
-    arguments: ['@http_kernel', '@title_resolver']
+    arguments: ['@controller_resolver', '@title_resolver']
   router_listener:
     class: Symfony\Component\HttpKernel\EventListener\RouterListener
     tags:
@@ -418,6 +424,14 @@ services:
     tags:
       - { name: event_subscriber }
     arguments: ['@content_negotiation', '@title_resolver']
+  html_view_subscriber:
+    class: Drupal\Core\EventSubscriber\HtmlViewSubscriber
+    tags:
+      - { name: event_subscriber }
+    arguments: ['@html_page_renderer']
+  html_page_renderer:
+    class: Drupal\Core\Page\DefaultHtmlPageRenderer
+    arguments: ['@language_manager']
   private_key:
     class: Drupal\Core\PrivateKey
     arguments: ['@state']
@@ -521,7 +535,7 @@ services:
     arguments: ['@language_manager', '@string_translation']
   exception_controller:
     class: Drupal\Core\Controller\ExceptionController
-    arguments: ['@content_negotiation']
+    arguments: ['@content_negotiation', '@string_translation', '@title_resolver', '@html_page_renderer']
     calls:
       - [setContainer, ['@service_container']]
   exception_listener:

core/includes/batch.inc

@@ -150,30 +150,46 @@ function _batch_progress_page() {
   $batch['url_options']['query']['op'] = $new_op;
 
   $url = url($batch['url'], $batch['url_options']);
-  $element = array(
-    // Redirect through a 'Refresh' meta tag if JavaScript is disabled.
-    '#prefix' => '<noscript>',
-    '#suffix' => '</noscript>',
-    '#tag' => 'meta',
-    '#attributes' => array(
-      'http-equiv' => 'Refresh',
-      'content' => '0; URL=' . $url,
-    ),
-  );
-  drupal_add_html_head($element, 'batch_progress_meta_refresh');
-
-  // Adds JavaScript code and settings for clients where JavaScript is enabled.
-  $js_setting = array(
-    'batch' => array(
-      'errorMessage' => $current_set['error_message'] . '<br />' . $batch['error_message'],
-      'initMessage' => $current_set['init_message'],
-      'uri' => $url,
+
+  $build = array(
+    '#theme' => 'progress_bar',
+    '#percent' => $percentage,
+    '#message' => $message,
+    '#label' => $label,
+    '#attached' => array(
+      'drupal_add_html_head' => array(
+        array(
+          array(
+            // Redirect through a 'Refresh' meta tag if JavaScript is disabled.
+            '#tag' => 'meta',
+            '#noscript' => TRUE,
+            '#attributes' => array(
+              'http-equiv' => 'Refresh',
+              'content' => '0; URL=' . $url,
+            ),
+          ),
+          'batch_progress_meta_refresh',
+        ),
+      ),
+      // Adds JavaScript code and settings for clients where JavaScript is enabled.
+      'js' => array(
+        array(
+          'type' => 'setting',
+          'data' => array(
+            'batch' => array(
+              'errorMessage' => $current_set['error_message'] . '<br />' . $batch['error_message'],
+              'initMessage' => $current_set['init_message'],
+              'uri' => $url,
+            ),
+          ),
+        ),
+      ),
+      'library' => array(
+        array('system', 'drupal.batch'),
+      ),
     ),
   );
-  drupal_add_js($js_setting, 'setting');
-  drupal_add_library('system', 'drupal.batch');
-
-  return theme('progress_bar', array('percent' => $percentage, 'message' => $message, 'label' => $label));
+  return drupal_render($build);
 }
 
 /**

core/includes/common.inc

@@ -354,7 +354,7 @@ function _drupal_default_html_head() {
 
 /**
  * Retrieves output to be displayed in the HEAD tag of the HTML page.
- */
+  */
 function drupal_get_html_head() {
   $elements = drupal_add_html_head();
   drupal_alter('html_head', $elements);
@@ -3408,7 +3408,12 @@ function drupal_pre_render_html_tag($element) {
     }
     $markup .= '</' . $element['#tag'] . ">\n";
   }
-  $element['#markup'] = $markup;
+  if (!empty($element['#noscript'])) {
+    $element['#markup'] = '<noscript>' . $markup . '</noscript>';
+  }
+  else {
+    $element['#markup'] = $markup;
+  }
   return $element;
 }
 
@@ -3592,7 +3597,7 @@ function drupal_pre_render_dropbutton($element) {
 }
 
 /**
- * Renders the page, including all theming.
+ * Processes the page render array, enhancing it as necessary.
  *
  * @param $page
  *   A string or array representing the content of a page. The array consists of
@@ -3602,10 +3607,13 @@ function drupal_pre_render_dropbutton($element) {
  *   - #show_messages: Suppress drupal_get_message() items. Used by Batch
  *     API (optional).
  *
+ * @return array
+ *   The processed render array for the page.
+ *
  * @see hook_page_alter()
  * @see element_info()
  */
-function drupal_render_page($page) {
+function drupal_prepare_page($page) {
   $main_content_display = &drupal_static('system_main_content_added', FALSE);
 
   // Pull out the page title to set it back later.
@@ -3642,6 +3650,28 @@ function drupal_render_page($page) {
     $page['#title'] = $title;
   }
 
+  return $page;
+}
+
+/**
+ * Renders the page, including all theming.
+ *
+ * @param string|array $page
+ *   A string or array representing the content of a page. The array consists of
+ *   the following keys:
+ *   - #type: Value is always 'page'. This pushes the theming through
+ *     the page template (required).
+ *   - #show_messages: Suppress drupal_get_message() items. Used by Batch
+ *     API (optional).
+ *
+ * @return string
+ *   Returns the rendered string.
+ *
+ * @see hook_page_alter()
+ * @see element_info()
+ */
+function drupal_render_page($page) {
+  $page = drupal_prepare_page($page);
   return drupal_render($page);
 }
 

core/includes/theme.inc

@@ -2054,15 +2054,22 @@ function _template_preprocess_default_variables() {
  * @see system_elements()
  */
 function template_preprocess_html(&$variables) {
-  $language_interface = language(Language::TYPE_INTERFACE);
+  /** @var $page \Drupal\Core\Page\HtmlPage */
+  $page = $variables['page_object'];
+
+  $variables['html_attributes'] = $page->getHtmlAttributes();
+  $variables['attributes'] = $page->getBodyAttributes();
+  $variables['page'] = $page->getContent();
 
   // Compile a list of classes that are going to be applied to the body element.
   // This allows advanced theming based on context (home page, node of certain type, etc.).
-  $variables['attributes']['class'][] = 'html';
+  $body_classes = $variables['attributes']['class'];
+  $body_classes[] = 'html';
   // Add a class that tells us whether we're on the front page or not.
-  $variables['attributes']['class'][] = $variables['is_front'] ? 'front' : 'not-front';
+  $body_classes[] = $variables['is_front'] ? 'front' : 'not-front';
   // Add a class that tells us whether the page is viewed by an authenticated user or not.
-  $variables['attributes']['class'][] = $variables['logged_in'] ? 'logged-in' : 'not-logged-in';
+  $body_classes[] = $variables['logged_in'] ? 'logged-in' : 'not-logged-in';
+  $variables['attributes']['class'] = $body_classes;
 
   // Populate the body classes.
   if ($suggestions = theme_get_suggestions(arg(), 'page', '-')) {
@@ -2080,6 +2087,7 @@ function template_preprocess_html(&$variables) {
   $variables['html_attributes'] = new Attribute;
 
   // HTML element attributes.
+  $language_interface = \Drupal::service('language_manager')->getLanguage();
   $variables['html_attributes']['lang'] = $language_interface->id;
   $variables['html_attributes']['dir'] = $language_interface->direction ? 'rtl' : 'ltr';
 
@@ -2097,9 +2105,9 @@ function template_preprocess_html(&$variables) {
 
   $site_config = \Drupal::config('system.site');
   // Construct page title.
-  if (!empty($variables['page']['#title'])) {
+  if ($page->hasTitle()) {
     $head_title = array(
-      'title' => strip_tags($variables['page']['#title']),
+      'title' => strip_tags($page->getTitle()),
       'name' => String::checkPlain($site_config->get('name')),
     );
   }
@@ -2149,15 +2157,8 @@ function template_preprocess_html(&$variables) {
 
   drupal_add_library('system', 'html5shiv', TRUE);
 
-  // Render page_top and page_bottom into top level variables.
-  $variables['page_top'] = array();
-  if (isset($variables['page']['page_top'])) {
-    $variables['page_top'] = drupal_render($variables['page']['page_top']);
-  }
-  $variables['page_bottom'] = array();
-  if (isset($variables['page']['page_bottom'])) {
-    $variables['page_bottom'][]['#markup'] = drupal_render($variables['page']['page_bottom']);
-  }
+  $variables['page_top'][] = array('#markup' => $page->getBodyTop());
+  $variables['page_bottom'][] = array('#markup' => $page->getBodyBottom());
 
   // Add footer scripts as '#markup' so they can be rendered with other
   // elements in page_bottom.
@@ -2557,7 +2558,7 @@ function drupal_common_theme() {
   return array(
     // From theme.inc.
     'html' => array(
-      'render element' => 'page',
+      'variables' => array('page_object' => NULL),
       'template' => 'html',
     ),
     'page' => array(
@@ -2631,7 +2632,7 @@ function drupal_common_theme() {
     ),
     // From theme.maintenance.inc.
     'maintenance_page' => array(
-      'variables' => array('content' => NULL, 'show_messages' => TRUE),
+      'variables' => array('content' => NULL, 'show_messages' => TRUE, 'page' => array()),
       'template' => 'maintenance-page',
     ),
     'install_page' => array(

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

@@ -10,14 +10,34 @@
 use Drupal\Core\Ajax\AjaxResponse;
 use Drupal\Core\Ajax\InsertCommand;
 use Drupal\Core\Ajax\PrependCommand;
+use Drupal\Core\Page\HtmlFragment;
+use Drupal\Core\Page\HtmlPage;
 use Symfony\Component\DependencyInjection\ContainerAware;
 use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
 
 /**
  * Default controller for ajax requests.
  */
 class AjaxController extends ContainerAware {
 
+  /**
+   * The controller resolver.
+   *
+   * @var \Drupal\Core\Controller\ControllerResolverInterface
+   */
+  protected $controllerResolver;
+
+  /**
+   * Constructs a new AjaxController instance.
+   *
+   * @param \Drupal\Core\Controller\ControllerResolverInterface $controller_resolver
+   *   The controller resolver.
+   */
+  public function __construct(ControllerResolverInterface $controller_resolver) {
+    $this->controllerResolver = $controller_resolver;
+  }
+
   /**
    * Controller method for AJAX content.
    *
@@ -30,50 +50,66 @@ class AjaxController extends ContainerAware {
    *   A response object.
    */
   public function content(Request $request, $_content) {
+    $content = $this->getContentResult($request, $_content);
+    // If there is already an AjaxResponse, then return it without
+    // manipulation.
+    if ($content instanceof AjaxResponse && $content->isOk()) {
+      return $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;
+    // Allow controllers to return a HtmlFragment or a Response object directly.
+    if ($content instanceof HtmlFragment) {
+      $content = $content->getContent();
+    }
+    if ($content instanceof Response) {
+      $content = $content->getContent();
+    }
 
-    // We need to clean up the derived information and such so that the
-    // subrequest can be processed properly without leaking data through.
-    $attributes->remove('_system_path');
-    $attributes->remove('_content');
-    $attributes->remove('_legacy');
+    // Most controllers return a render array, but some return a string.
+    if (!is_array($content)) {
+      $content = array(
+        '#markup' => $content,
+      );
+    }
 
-    // Remove the accept header so the subrequest does not end up back in this
-    // controller.
-    $request->headers->remove('accept');
-    // Remove the header in order to let the subrequest not think that it's an
-    // ajax request, see \Drupal\Core\ContentNegotiation.
-    $request->headers->remove('x-requested-with');
+    $html = drupal_render($content);
 
-    $response = $this->container->get('http_kernel')->forward($controller, $attributes->all(), $request->query->all());
-    // For successful (HTTP status 200) responses.
-    if ($response->isOk()) {
-      // If there is already an AjaxResponse, then return it without
-      // manipulation.
-      if (!($response instanceof AjaxResponse)) {
-        // Pull the content out of the response.
-        $content = $response->getContent();
-        // A page callback could return a render array or a string.
-        $html = is_string($content) ? $content : drupal_render($content);
-        $response = new AjaxResponse();
-        // The selector for the insert command is NULL as the new content will
-        // replace the element making the ajax call. The default 'replaceWith'
-        // behavior can be changed with #ajax['method'].
-        $response->addCommand(new InsertCommand(NULL, $html));
-        $status_messages = array('#theme' => 'status_messages');
-        $output = drupal_render($status_messages);
-        if (!empty($output)) {
-          $response->addCommand(new PrependCommand(NULL, $output));
-        }
-      }
+    $response = new AjaxResponse();
+    // The selector for the insert command is NULL as the new content will
+    // replace the element making the ajax call. The default 'replaceWith'
+    // behavior can be changed with #ajax['method'].
+    $response->addCommand(new InsertCommand(NULL, $html));
+    $status_messages = array('#theme' => 'status_messages');
+    $output = drupal_render($status_messages);
+    if (!empty($output)) {
+      $response->addCommand(new PrependCommand(NULL, $output));
     }
     return $response;
   }
+
+  /**
+   * Returns the result of invoking the sub-controller.
+   *
+   * @param \Symfony\Component\HttpFoundation\Request $request
+   *   The request object.
+   * @param mixed $controller_definition
+   *   A controller definition string, or a callable object/closure.
+   *
+   * @return mixed
+   *   The result of invoking the controller. Render arrays, strings, HtmlPage,
+   *   and HtmlFragment objects are possible.
+   */
+  public function getContentResult(Request $request, $controller_definition) {
+    if ($controller_definition instanceof \Closure) {
+      $callable = $controller_definition;
+    }
+    else {
+      $callable = $this->controllerResolver->getControllerFromDefinition($controller_definition);
+    }
+    $arguments = $this->controllerResolver->getArguments($request, $callable);
+    $page_content = call_user_func_array($callable, $arguments);
+
+    return $page_content;
+  }
+
 }

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

@@ -9,9 +9,10 @@
 
 use Drupal\Core\Ajax\AjaxResponse;
 use Drupal\Core\Ajax\OpenDialogCommand;
+use Drupal\Core\Page\HtmlPage;
 use Symfony\Cmf\Component\Routing\RouteObjectInterface;
 use Symfony\Component\HttpFoundation\Request;
-use Symfony\Component\HttpKernel\HttpKernelInterface;
+use Symfony\Component\HttpFoundation\Response;
 
 /**
  * Defines a default controller for dialog requests.
@@ -19,11 +20,11 @@
 class DialogController {
 
   /**
-   * The HttpKernel object to use for subrequests.
+   * The controller resolver service.
    *
-   * @var \Symfony\Component\HttpKernel\HttpKernelInterface
+   * @var \Drupal\Core\Controller\ControllerResolverInterface
    */
-  protected $httpKernel;
+  protected $controllerResolver;
 
   /**
    * The title resolver.
@@ -35,112 +36,126 @@ class DialogController {
   /**
    * Constructs a new DialogController.
    *
-   * @param \Symfony\Component\HttpKernel\HttpKernelInterface $kernel
-   *   The kernel.
+   * @param \Drupal\Core\Controller\ControllerResolverInterface $controller_resolver
+   *   The controller resolver service.
    * @param \Drupal\Core\Controller\TitleResolverInterface $title_resolver
    *   The title resolver.
    */
-  public function __construct(HttpKernelInterface $kernel, TitleResolverInterface $title_resolver) {
-    $this->httpKernel = $kernel;
+  public function __construct(ControllerResolverInterface $controller_resolver, TitleResolverInterface $title_resolver) {
+    $this->controllerResolver = $controller_resolver;
     $this->titleResolver = $title_resolver;
   }
 
-  /**
-   * Forwards request to a subrequest.
-   *
-   * @param \Symfony\Component\HttpFoundation\RequestRequest $request
-   *   The request object.
-   *
-   * @return \Symfony\Component\HttpFoundation\Response
-   *   A response object.
-   */
-  protected function forward(Request $request) {
-    // @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;
-    // We need to clean up the derived information and such so that the
-    // subrequest can be processed properly without leaking data through.
-    $attributes->remove('_system_path');
-    $attributes->set('dialog', TRUE);
-
-    // Remove the accept header so the subrequest does not end up back in this
-    // controller.
-    $request->headers->remove('accept');
-    // Remove the X-Requested-With header so the subrequest is not mistaken for
-    // an ajax request.
-    $request->headers->remove('x-requested-with');
-
-    return $this->httpKernel->forward(NULL, $attributes->all(), $request->query->all());
-  }
-
   /**
    * Displays content in a modal dialog.
    *
-   * @param \Symfony\Component\HttpFoundation\RequestRequest $request
+   * @param \Symfony\Component\HttpFoundation\Request $request
    *   The request object.
+   * @param mixed $_content
+   *   A controller definition string, or a callable object/closure.
    *
    * @return \Drupal\Core\Ajax\AjaxResponse
    *   AjaxResponse to return the content wrapper in a modal dialog.
    */
-  public function modal(Request $request) {
-    return $this->dialog($request, TRUE);
+  public function modal(Request $request, $_content) {
+    return $this->dialog($request, $_content, TRUE);
   }
 
   /**
    * Displays content in a dialog.
    *
-   * @param \Symfony\Component\HttpFoundation\RequestRequest $request
+   * @param \Symfony\Component\HttpFoundation\Request $request
    *   The request object.
+   * @param mixed $_content
+   *   A controller definition string, or a callable object/closure.
    * @param bool $modal
    *   (optional) TRUE to render a modal dialog. Defaults to FALSE.
    *
    * @return \Drupal\Core\Ajax\AjaxResponse
    *   AjaxResponse to return the content wrapper in a dialog.
    */
-  public function dialog(Request $request, $modal = FALSE) {
-    $subrequest = $this->forward($request);
-    if ($subrequest->isOk()) {
-      $content = $subrequest->getContent();
-      if (!$title = $this->titleResolver->getTitle($request, $request->attributes->get(RouteObjectInterface::ROUTE_OBJECT))) {
-        // @todo Remove use of drupal_get_title() when
-        //  http://drupal.org/node/1871596 is in.
-        $title = drupal_get_title();
-      }
-      $response = new AjaxResponse();
-      // Fetch any modal options passed in from data-dialog-options.
-      if (!($options = $request->request->get('dialogOptions'))) {
-        $options = array();
-      }
-      // Set modal flag and re-use the modal ID.
-      if ($modal) {
-        $options['modal'] = TRUE;
-        $target = '#drupal-modal';
+  public function dialog(Request $request, $_content, $modal = FALSE) {
+    $page_content = $this->getContentResult($request, $_content);
+
+    // Allow controllers to return a HtmlPage or a Response object directly.
+    if ($page_content instanceof HtmlPage) {
+      $page_content = $page_content->getContent();
+    }
+    if ($page_content instanceof Response) {
+      $page_content = $page_content->getContent();
+    }
+
+    // Most controllers return a render array, but some return a string.
+    if (!is_array($page_content)) {
+      $page_content = array(
+        '#markup' => $page_content,
+      );
+    }
+
+    $content = drupal_render($page_content);
+
+    // @todo Remove use of drupal_get_title() when
+    //  http://drupal.org/node/1871596 is in.
+    if (!$title = $this->titleResolver->getTitle($request, $request->attributes->get(RouteObjectInterface::ROUTE_OBJECT))) {
+      // @todo Remove use of drupal_get_title() when
+      //  http://drupal.org/node/1871596 is in.
+      $title = drupal_get_title();
+    }
+    $response = new AjaxResponse();
+    // Fetch any modal options passed in from data-dialog-options.
+    if (!($options = $request->request->get('dialogOptions'))) {
+      $options = array();
+    }
+    // Set modal flag and re-use the modal ID.
+    if ($modal) {
+      $options['modal'] = TRUE;
+      $target = '#drupal-modal';
+    }
+    else {
+      // Generate the target wrapper for the dialog.
+      if (isset($options['target'])) {
+        // If the target was nominated in the incoming options, use that.
+        $target = $options['target'];
+        // Ensure the target includes the #.
+        if (substr($target, 0, 1) != '#') {
+          $target = '#' . $target;
+        }
+        // This shouldn't be passed on to jQuery.ui.dialog.
+        unset($options['target']);
       }
       else {
-        // Generate the target wrapper for the dia