Issue #1931602 by damiankloip, dawehner: ViewExecutable::setDisplay() will never return an invalid display ID.

core/modules/node/tests/modules/node_test_views/test_views/views.view.test_status_extra.yml

@@ -129,7 +129,7 @@ display:
         path: test_status_extra
       display_plugin: page
       display_title: Page
-      id: page
+      id: page_1
       position: '0'
 base_field: nid
 status: '1'

core/modules/views/lib/Drupal/views/Tests/ViewExecutableTest.php

@@ -194,7 +194,14 @@ class ViewExecutableTest extends ViewUnitTestBase {
     $this->assertEqual($view->current_display, 'page_2', 'If setDisplay is called with a valid display id the appropriate display should be used.');
     $this->assertEqual(spl_object_hash($view->display_handler), spl_object_hash($view->displayHandlers->get('page_2')));
 
+    // Destroy the view, so we can start again and test an invalid display.
+    $view->destroy();
+
+    $count_before = count($this->assertions);
     $view->setDisplay('invalid');
+    $count_after = count($this->assertions);
+    $this->assertTrue($count_after - $count_before, 'Error is triggered while calling the wrong display.');
+
     $this->assertEqual($view->current_display, 'default', 'If setDisplay is called with an invalid display id the default display should be used.');
     $this->assertEqual(spl_object_hash($view->display_handler), spl_object_hash($view->displayHandlers->get('default')));
   }

core/modules/views/lib/Drupal/views/ViewExecutable.php

@@ -638,48 +638,39 @@ class ViewExecutable {
   }
 
   /**
-   * Set the display as current.
+   * Sets the current display.
    *
-   * @param $display_id
+   * @param string $display_id
-   *   The id of the display to mark as current.
+   *   The ID of the display to mark as current.
+   *
+   * @return bool
+   *   TRUE if the display was correctly set, FALSE otherwise.
    */
   public function setDisplay($display_id = NULL) {
-    // If we have not already initialized the display, do so. But be careful.
+    // If we have not already initialized the display, do so.
-    if (empty($this->current_display)) {
+    if (!isset($this->current_display)) {
+      // This will set the default display and instantiate the default display
+      // plugin.
       $this->initDisplay();
-
-      // If handlers were not initialized, and no argument was sent, set up
-      // to the default display.
-      if (empty($display_id)) {
-        $display_id = 'default';
     }
+
+    // If no display ID is passed, we either have initialized the default or
+    // already have a display set.
+    if (!isset($display_id)) {
+      return TRUE;
     }
 
     $display_id = $this->chooseDisplay($display_id);
 
-    // If no display id sent in and one wasn't chosen above, we're finished.
-    if (empty($display_id)) {
-      return FALSE;
-    }
-
     // Ensure the requested display exists.
     if (!$this->displayHandlers->has($display_id)) {
-      $display_id = 'default';
+      debug(format_string('setDisplay() called with invalid display ID "@display".', array('@display' => $display_id)));
-      if (!$this->displayHandlers->has($display_id)) {
-        debug(format_string('set_display() called with invalid display ID @display.', array('@display' => $display_id)));
       return FALSE;
     }
-    }
 
     // Set the current display.
     $this->current_display = $display_id;
-
+    // Set a shortcut.
-    // Ensure requested display has a working handler.
-    if (!$this->displayHandlers->has($display_id)) {
-      return FALSE;
-    }
-
-    // Set a shortcut
     $this->display_handler = $this->displayHandlers->get($display_id);
 
     return TRUE;