Issue #2216437 by Wim Leers: Entity labels are not in-place editable on 'full...

Issue #2216437 by Wim Leers: Entity labels are not in-place editable on 'full entity page' (prime example: node title).

core/includes/theme.inc

@@ -1997,7 +1997,7 @@ function template_preprocess_html(&$variables) {
   // Construct page title.
   if ($page->hasTitle()) {
     $head_title = array(
-      'title' => strip_tags($page->getTitle()),
+      'title' => trim(strip_tags($page->getTitle())),
       'name' => String::checkPlain($site_config->get('name')),
     );
   }

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

@@ -9,6 +9,7 @@
 
 use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
 use Drupal\Core\Entity\EntityInterface;
+use Drupal\Core\Entity\ContentEntityInterface;
 use Drupal\Core\Entity\EntityManagerInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
@@ -61,9 +62,28 @@ public static function create(ContainerInterface $container) {
    *   A render array as expected by drupal_render().
    */
   public function view(EntityInterface $_entity, $view_mode = 'full', $langcode = NULL) {
-    return $this->entityManager
+    $page = $this->entityManager
       ->getViewBuilder($_entity->getEntityTypeId())
       ->view($_entity, $view_mode, $langcode);
+
+    // 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. RDFa, in-place editing).
+    if ($_entity instanceof ContentEntityInterface) {
+      $label_field = $_entity->getEntityType()->getKey('label');
+      if ($label_field && $_entity->getFieldDefinition($label_field)->getDisplayOptions('view')) {
+        // We must render the label field, because rendering the entity may be
+        // a cache hit, in which case we can't extract the rendered label field
+        // from the $page renderable array.
+        $build = $this->entityManager->getTranslationFromContext($_entity)
+          ->get($label_field)
+          ->view($view_mode);
+        $page['#title'] = drupal_render($build, TRUE);
+      }
+    }
+
+    return $page;
   }
 
 }

core/modules/entity/lib/Drupal/entity/Tests/EntityDisplayTest.php

@@ -42,9 +42,10 @@ public function testEntityDisplayCRUD() {
     $expected = array();
 
     // Check that providing no 'weight' results in the highest current weight
-    // being assigned.
-    $expected['component_1'] = array('weight' => 0);
-    $expected['component_2'] = array('weight' => 1);
+    // being assigned. The 'name' field's formatter has weight -5, therefore
+    // these follow.
+    $expected['component_1'] = array('weight' => -4);
+    $expected['component_2'] = array('weight' => -3);
     $display->setComponent('component_1');
     $display->setComponent('component_2');
     $this->assertEqual($display->getComponent('component_1'), $expected['component_1']);
@@ -63,6 +64,12 @@ public function testEntityDisplayCRUD() {
     }
 
     // Check that getComponents() returns options for all components.
+    $expected['name'] = array(
+      'label' => 'hidden',
+      'type' => 'string',
+      'weight' => -5,
+      'settings' => array(),
+    );
     $this->assertEqual($display->getComponents(), $expected);
 
     // Check that a component can be removed.
@@ -164,7 +171,7 @@ public function testFieldComponent() {
     $default_formatter = $field_type_info['default_formatter'];
     $formatter_settings =  \Drupal::service('plugin.manager.field.formatter')->getDefaultSettings($default_formatter);
     $expected = array(
-      'weight' => 0,
+      'weight' => -4,
       'label' => 'above',
       'type' => $default_formatter,
       'settings' => $formatter_settings,

core/modules/entity_reference/lib/Drupal/entity_reference/Tests/EntityReferenceFormatterTest.php

@@ -171,6 +171,12 @@ public function testEntityFormatter() {
     $items = $referencing_entity->get($field_name);
     $build = $items->view(array('type' => $formatter));
 
+    $expected_rendered_name_field = '<div class="field field-entity-test--name field-name-name field-type-string field-label-hidden">
+    <div class="field-items">
+          <div class="field-item">' . $this->referencedEntity->label() . '</div>
+      </div>
+</div>
+';
     $expected_rendered_body_field = '<div class="field field-entity-test--body field-name-body field-type-text field-label-above">
       <div class="field-label">Body:&nbsp;</div>
     <div class="field-items">
@@ -178,7 +184,7 @@ public function testEntityFormatter() {
       </div>
 </div>
 ';
-    $this->assertEqual($build[0]['#markup'], 'default | ' . $this->referencedEntity->label() .  $expected_rendered_body_field, format_string('The markup returned by the @formatter formatter is correct.', array('@formatter' => $formatter)));
+    $this->assertEqual($build[0]['#markup'], 'default | ' . $this->referencedEntity->label() .  $expected_rendered_name_field . $expected_rendered_body_field, format_string('The markup returned by the @formatter formatter is correct.', array('@formatter' => $formatter)));
     $expected_cache_tags = array(
       $this->entityType . '_view' => TRUE,
       $this->entityType => array($this->referencedEntity->id() => $this->referencedEntity->id()),

core/modules/node/lib/Drupal/node/Controller/NodeController.php

@@ -9,6 +9,7 @@
 
 use Drupal\Component\Utility\String;
 use Drupal\Core\Controller\ControllerBase;
+use Drupal\node\Controller\NodeViewController;
 use Drupal\node\NodeTypeInterface;
 use Drupal\node\NodeInterface;
 
@@ -88,9 +89,9 @@ public function add(NodeTypeInterface $node_type) {
    */
   public function revisionShow($node_revision) {
     $node = $this->entityManager()->getStorage('node')->loadRevision($node_revision);
-    $page = $this->buildPage($node);
+    $node_view_controller = new NodeViewController($this->entityManager);
+    $page = $node_view_controller->view($node);
     unset($page['nodes'][$node->id()]['#cache']);
-
     return $page;
   }
 
@@ -116,67 +117,6 @@ public function revisionOverview(NodeInterface $node) {
     return node_revision_overview($node);
   }
 
-  /**
-   * Displays a node.
-   *
-   * @param \Drupal\node\NodeInterface $node
-   *   The node we are displaying.
-   *
-   * @return array
-   *   An array suitable for drupal_render().
-   */
-  public function page(NodeInterface $node) {
-    $build = $this->buildPage($node);
-
-    foreach ($node->uriRelationships() as $rel) {
-      // Set the node path as the canonical URL to prevent duplicate content.
-      $build['#attached']['drupal_add_html_head_link'][] = array(
-        array(
-        'rel' => $rel,
-        'href' => $node->url($rel),
-        )
-        , TRUE);
-
-      if ($rel == 'canonical') {
-        // Set the non-aliased canonical path as a default shortlink.
-        $build['#attached']['drupal_add_html_head_link'][] = array(
-          array(
-            'rel' => 'shortlink',
-            'href' => $node->url($rel, array('alias' => TRUE)),
-          )
-        , TRUE);
-      }
-    }
-
-    return $build;
-  }
-
-  /**
-   * The _title_callback for the node.view route.
-   *
-   * @param NodeInterface $node
-   *   The current node.
-   *
-   * @return string
-   *   The page title.
-   */
-  public function pageTitle(NodeInterface $node) {
-    return String::checkPlain($this->entityManager()->getTranslationFromContext($node)->label());
-  }
-
-  /**
-   * Builds a node page render array.
-   *
-   * @param \Drupal\node\NodeInterface $node
-   *   The node we are displaying.
-   *
-   * @return array
-   *   An array suitable for drupal_render().
-   */
-  protected function buildPage(NodeInterface $node) {
-    return array('nodes' => $this->entityManager()->getViewBuilder('node')->view($node));
-  }
-
   /**
    * The _title_callback for the node.add route.
    *

core/modules/node/lib/Drupal/node/Controller/NodeViewController.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\node\Controller\NodeViewController.
+ */
+
+namespace Drupal\node\Controller;
+
+use Drupal\Component\Utility\String;
+use Drupal\Core\Entity\EntityInterface;
+use Drupal\Core\Entity\Controller\EntityViewController;
+
+/**
+ * Defines a controller to render a single node.
+ */
+class NodeViewController extends EntityViewController {
+
+  /**
+   * {@inheritdoc}
+   */
+  public function view(EntityInterface $node, $view_mode = 'full', $langcode = NULL) {
+    $build = array('nodes' => parent::view($node));
+
+    $build['#title'] = $build['nodes']['#title'];
+    unset($build['nodes']['#title']);
+
+    foreach ($node->uriRelationships() as $rel) {
+      // Set the node path as the canonical URL to prevent duplicate content.
+      $build['#attached']['drupal_add_html_head_link'][] = array(
+        array(
+        'rel' => $rel,
+        'href' => $node->url($rel),
+        )
+        , TRUE);
+
+      if ($rel == 'canonical') {
+        // Set the non-aliased canonical path as a default shortlink.
+        $build['#attached']['drupal_add_html_head_link'][] = array(
+          array(
+            'rel' => 'shortlink',
+            'href' => $node->url($rel, array('alias' => TRUE)),
+          )
+        , TRUE);
+      }
+    }
+
+    return $build;
+  }
+
+  /**
+   * The _title_callback for the page that renders a single node.
+   *
+   * @param \Drupal\Core\Entity\EntityInterface $node
+   *   The current node.
+   *
+   * @return string
+   *   The page title.
+   */
+  public function title(EntityInterface $node) {
+    return String::checkPlain($this->entityManager->getTranslationFromContext($node)->label());
+  }
+
+}

core/modules/node/node.routing.yml

@@ -47,8 +47,8 @@ node.add:
 node.view:
   path: '/node/{node}'
   defaults:
-    _content: '\Drupal\node\Controller\NodeController::page'
-    _title_callback: '\Drupal\node\Controller\NodeController::pageTitle'
+    _content: '\Drupal\node\Controller\NodeViewController::view'
+    _title_callback: '\Drupal\node\Controller\NodeViewController::title'
   requirements:
     _entity_access: 'node.view'
 

core/modules/quickedit/js/views/EntityToolbarView.js

@@ -203,9 +203,19 @@
             delay = 250;
             break;
           default:
-            // Position against the entity, or as a last resort, the body element.
-            of = this.$entity || 'body';
-            delay = 750;
+            var fieldModels = this.model.get('fields').models;
+            var topMostPosition = 1000000;
+            var topMostField = null;
+            // Position against the topmost field.
+            for (var i = 0; i < fieldModels.length; i++) {
+              var pos = fieldModels[i].get('el').getBoundingClientRect().top;
+              if (pos < topMostPosition) {
+                topMostPosition = pos;
+                topMostField = fieldModels[i];
+              }
+            }
+            of = topMostField.get('el');
+            delay = 50;
             break;
         }
         // Prepare to check the next possible element to position against.

core/modules/quickedit/lib/Drupal/quickedit/Tests/QuickEditLoadingTest.php

@@ -298,6 +298,11 @@ public function testTitleBaseField() {
     $this->drupalLogin($this->editor_user);
     $this->drupalGet('node/1');
 
+    // Ensure that the full page title is actually in-place editable
+    $node = entity_load('node', 1);
+    $elements = $this->xpath('//h1/span[@data-quickedit-field-id="node/1/title/und/full" and normalize-space(text())=:title]', array(':title' => $node->label()));
+    $this->assertTrue(!empty($elements), 'Title with data-quickedit-field-id attribute found.');
+
     // Retrieving the metadata should result in a 200 JSON response.
     $htmlPageDrupalSettings = $this->drupalSettings;
     $post = array('fields[0]' => 'node/1/title/und/full');

core/modules/system/lib/Drupal/system/Tests/Entity/EntityViewControllerTest.php

@@ -53,9 +53,19 @@ function setUp() {
    * Tests EntityViewController.
    */
   function testEntityViewController() {
+    $get_label_markup = function($label) {
+      return '<h1><div class="field field-entity-test--name field-name-name field-type-string field-label-hidden">
+    <div class="field-items">
+          <div class="field-item">' . $label . '</div>
+      </div>
+</div>
+</h1>';
+    };
+
     foreach ($this->entities as $entity) {
       $this->drupalGet('entity_test/' . $entity->id());
       $this->assertRaw($entity->label());
+      $this->assertRaw($get_label_markup($entity->label()));
       $this->assertRaw('full');
 
       $this->drupalGet('entity_test_converter/' . $entity->id());

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

@@ -2,6 +2,7 @@ entity_test.render:
   path: '/entity_test/{entity_test}'
   defaults:
     _entity_view: 'entity_test.full'
+    _title: 'Test full view mode'
   requirements:
     _access: 'TRUE'
 

core/modules/system/tests/modules/entity_test/lib/Drupal/entity_test/Entity/EntityTest.php

@@ -82,7 +82,12 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
       ->setLabel(t('Name'))
       ->setDescription(t('The name of the test entity.'))
       ->setTranslatable(TRUE)
-      ->setSetting('max_length', 32);
+      ->setSetting('max_length', 32)
+      ->setDisplayOptions('view', array(
+        'label' => 'hidden',
+        'type' => 'string',
+        'weight' => -5,
+      ));
 
     // @todo: Add allowed values validation.
     $fields['type'] = FieldDefinition::create('string')

core/modules/system/tests/modules/entity_test/lib/Drupal/entity_test/EntityTestViewBuilder.php

@@ -24,12 +24,15 @@ public function buildComponents(array &$build, array $entities, array $displays,
 
     foreach ($entities as $id => $entity) {
       $build[$id]['label'] = array(
+        '#weight' => -100,
         '#markup' => check_plain($entity->label()),
       );
       $build[$id]['separator'] = array(
+        '#weight' => -150,
         '#markup' => ' | ',
       );
       $build[$id]['view_mode'] = array(
+        '#weight' => -200,
         '#markup' => check_plain($view_mode),
       );
     }

core/tests/Drupal/Tests/Core/Entity/Controller/EntityViewControllerTest.php

@@ -49,10 +49,34 @@ public function testView() {
       ->method('getViewBuilder')
       ->will($this->returnValue($render_controller));
 
+    // Mock the 'entity_test' entity type.
+    $entity_type = $this->getMockBuilder('Drupal\Core\Entity\ContentEntityType')
+      ->disableOriginalConstructor()
+      ->getMock();
+    $entity_type->expects($this->once())
+      ->method('getKey')
+      ->with('label')
+      ->will($this->returnValue('name'));
+
+    // Mock the 'name' field's definition.
+    $field_definition = $this->getMock('Drupal\Core\Field\FieldDefinition');
+    $field_definition->expects($this->any())
+      ->method('getDisplayOptions')
+      ->with('view')
+      ->will($this->returnValue(NULL));
+
     // Mock an 'entity_test' entity.
     $entity = $this->getMockBuilder('Drupal\entity_test\Entity\EntityTest')
       ->disableOriginalConstructor()
       ->getMock();
+    $entity->expects($this->once())
+      ->method('getEntityType')
+      ->will($this->returnValue($entity_type));
+    $entity->expects($this->any())
+      ->method('getFieldDefinition')
+      ->with('name')
+      ->will($this->returnValue($field_definition));
+
 
     // Initialize the controller to test.
     $controller = new EntityViewController($entity_manager);