Issue #2941208 by AdamPS, dpi, sja112, aleevas, andypost, anmolgoyal74,...

Issue #2941208 by AdamPS, dpi, sja112, aleevas, andypost, anmolgoyal74, Spokje, jofitz, Rinku Jacob 13, jonathanshaw, plach, larowlan, alexpott, catch, lauriii: Title formatting broken due to flawed EntityViewController->buildTitle

core/lib/Drupal/Core/Entity/Controller/EntityViewController.php

@@ -55,6 +55,19 @@ public static function create(ContainerInterface $container) {
   /**
    * Pre-render callback to build the page title.
    *
+   * There are two possibilities, depending on the value of the additional
+   * entity type property 'enable_page_title_template'.
+   * - FALSE (default): use the output of the related field formatter if it
+   *   exists. This approach only works correctly for the node entity type and
+   *   with the 'string' formatter. In other cases it likely produces illegal
+   *   markup and possibly incorrect display. This option has been retained for
+   *   backward-compatibility to support sites that expect attributes set on
+   *   the field to propagate to the page title.
+   * - TRUE: use the output from the entity_page_title template. This approach
+   *   works correctly in all cases, without relying on a particular field
+   *   formatter or special templates and is the preferred option for the
+   *   future.
+   *
    * @param array $page
    *   A page render array.
    *
@@ -64,12 +77,31 @@ public static function create(ContainerInterface $container) {
   public function buildTitle(array $page) {
     $entity_type = $page['#entity_type'];
     $entity = $page['#' . $entity_type];
-    // If the entity's label is rendered using a field formatter, set the
-    // rendered title field formatter as the page title instead of the default
-    // plain text title. This allows attributes set on the field to propagate
-    // correctly (e.g. in-place editing).
+
+    // If the entity has a label field, build the page title based on it.
     if ($entity instanceof FieldableEntityInterface) {
       $label_field = $entity->getEntityType()->getKey('label');
+      $template_enabled = $entity->getEntityType()->get('enable_page_title_template');
+      if ($label_field && $template_enabled) {
+        // Set page title to the output from the entity_page_title template.
+        $page_title = [
+          '#theme' => 'entity_page_title',
+          '#title' => $entity->label(),
+          '#entity' => $entity,
+          '#view_mode' => $page['#view_mode'],
+        ];
+        $page['#title'] = $this->renderer->render($page_title);
+
+        // Prevent output of the label field in the main content.
+        $page[$label_field]['#access'] = FALSE;
+        return $page;
+      }
+
+      // Set page title to the rendered title field formatter instead of
+      // the default plain text title.
+      //
+      // @todo https://www.drupal.org/project/drupal/issues/3015623
+      //   Eventually delete this code and always use the first approach.
       if (isset($page[$label_field])) {
         // Allow templates and theme functions to generate different markup
         // for the page title, which must be inline markup as it will be placed

core/modules/node/tests/modules/node_display_configurable_test/node_display_configurable_test.module

@@ -25,4 +25,5 @@ function node_display_configurable_test_entity_base_field_info_alter(&$base_fiel
 function node_display_configurable_test_entity_type_build(array &$entity_types) {
   // Allow skipping of extra preprocessing for configurable display.
   $entity_types['node']->set('enable_base_field_custom_preprocess_skipping', TRUE);
+  $entity_types['node']->set('enable_page_title_template', TRUE);
 }

core/modules/node/tests/src/Functional/NodeDisplayConfigurableTest.php

@@ -69,7 +69,7 @@ public function testDisplayConfigurable(string $theme, string $metadata_region,
 
     // Check the node with Drupal default non-configurable display.
     $this->drupalGet($node->toUrl());
-    $this->assertNodeHtml($node, $user, TRUE, $metadata_region, $field_classes);
+    $this->assertNodeHtml($node, $user, TRUE, $metadata_region, $field_classes, $field_classes);
 
     // Enable module to make base fields' displays configurable.
     \Drupal::service('module_installer')->install(['node_display_configurable_test']);
@@ -82,12 +82,13 @@ public function testDisplayConfigurable(string $theme, string $metadata_region,
         'label' => 'above',
         'settings' => ['link' => FALSE],
       ])
+      ->removeComponent('title')
       ->save();
 
     // Recheck the node with configurable display.
     $this->drupalGet($node->toUrl());
 
-    $this->assertNodeHtml($node, $user, FALSE, $metadata_region, $field_classes);
+    $this->assertNodeHtml($node, $user, FALSE, $metadata_region, $field_classes, FALSE);
 
     $assert->elementExists('css', 'div[rel="schema:author"]');
 
@@ -113,15 +114,17 @@ public function testDisplayConfigurable(string $theme, string $metadata_region,
    * @param string $metadata_region
    *   The region of the node html content where meta data is expected.
    * @param bool $field_classes
-   *   If TRUE, check for field--name-XXX classes.
+   *   If TRUE, check for field--name-XXX classes on created/uid fields.
+   * @param bool $title_classes
+   *   If TRUE, check for field--name-XXX classes on title field.
    *
    * @internal
    */
-  protected function assertNodeHtml(NodeInterface $node, UserInterface $user, bool $is_inline, string $metadata_region, bool $field_classes): void {
+  protected function assertNodeHtml(NodeInterface $node, UserInterface $user, bool $is_inline, string $metadata_region, bool $field_classes, bool $title_classes): void {
     $assert = $this->assertSession();
 
     $html_element = $is_inline ? 'span' : 'div';
-    $title_selector = 'h1 span' . ($field_classes ? '.field--name-title' : '');
+    $title_selector = 'h1 span' . ($title_classes ? '.field--name-title' : '');
     $assert->elementTextContains('css', $title_selector, $node->getTitle());
 
     // With field classes, the selector can be very specific.

core/modules/quickedit/quickedit.module

@@ -128,6 +128,23 @@ function quickedit_preprocess_page_title(&$variables) {
   }
 }
 
+/**
+ * Implements hook_preprocess_entity_page_title().
+ */
+function quickedit_preprocess_entity_page_title(&$variables) {
+  $variables['#cache']['contexts'][] = 'user.permissions';
+  $entity = $variables['entity'];
+  if (!\Drupal::currentUser()->hasPermission('access in-place editing')) {
+    return;
+  }
+  if (($entity instanceof RevisionableInterface) && !$entity->isLatestRevision()) {
+    return;
+  }
+
+  $label_field = $entity->getEntityType()->getKey('label');
+  $variables['attributes']['data-quickedit-field-id'] = $entity->getEntityTypeId() . '/' . $entity->id() . '/' . $label_field . '/' . $entity->language()->getId() . '/' . $variables['view_mode'];
+}
+
 /**
  * Implements hook_preprocess_HOOK() for field templates.
  */

core/modules/system/system.module

@@ -253,6 +253,14 @@ function system_theme() {
       'file' => 'system.theme.inc',
       'variables' => ['error_message' => []],
     ],
+    'entity_page_title' => [
+      'variables' => [
+        'attributes' => [],
+        'title' => NULL,
+        'entity' => NULL,
+        'view_mode' => NULL,
+      ],
+    ],
   ]);
 }
 

core/modules/system/templates/entity-page-title.html.twig

+{#
+/**
+ * @file
+ * Default theme implementation for entity page title.
+ *
+ * This output from this template is nested within the page-title template as
+ * the title variable. This allows a further refinement of the page title
+ * specific to an entity.
+ *
+ * This template is only used if the additional entity type property
+ * 'enable_page_title_template' is set to TRUE.
+ *
+ * Available variables:
+ * - attributes: HTML attributes for the containing span element.
+ * - title: Entity label.
+ * - entity: Entity having a label field.
+ * - view_mode: View mode; for example, "teaser" or "full".
+ *
+ * @see \Drupal\Core\Entity\Controller\EntityViewController::buildTitle()
+ *
+ * @ingroup themeable
+ */
+#}
+<span{{ attributes }}>
+  {{ title }}
+</span>

core/themes/stable/templates/field/entity-page-title.html.twig

+{#
+/**
+ * @file
+ * Theme override for entity page title.
+ *
+ * This output from this template is nested within the page-title template as
+ * the title variable. This allows a further refinement of the page title
+ * specific to an entity.
+ *
+ * This template is only used if the additional entity type property
+ * 'enable_page_title_template' is set to TRUE.
+ *
+ * Available variables:
+ * - attributes: HTML attributes for the containing span element.
+ * - title: Entity label.
+ * - entity: Entity having a label field.
+ * - view_mode: View mode; for example, "teaser" or "full".
+ */
+#}
+<span{{ attributes }}>
+  {{ title }}
+</span>

core/themes/stable9/templates/field/entity-page-title.html.twig

+{#
+/**
+ * @file
+ * Theme override for entity page title.
+ *
+ * This output from this template is nested within the page-title template as
+ * the title variable. This allows a further refinement of the page title
+ * specific to an entity.
+ *
+ * This template is only used if the additional entity type property
+ * 'enable_page_title_template' is set to TRUE.
+ *
+ * Available variables:
+ * - attributes: HTML attributes for the containing span element.
+ * - title: Entity label.
+ * - entity: Entity having a label field.
+ * - view_mode: View mode; for example, "teaser" or "full".
+ */
+#}
+<span{{ attributes }}>
+  {{ title }}
+</span>