Issue #2095139 by dawehner, pwolanin, kfritsche, YesCT, tstoeckler: Fixed...

Issue #2095139 by dawehner, pwolanin, kfritsche, YesCT, tstoeckler: Fixed Checking for the active tab should use raw variables.

core/lib/Drupal/Core/Menu/LocalTaskManager.php

@@ -261,16 +261,17 @@ public function getTasksBuild($current_route_name) {
 
     foreach ($tree as $level => $instances) {
       foreach ($instances as $plugin_id => $child) {
-        // In order to get the Drupal path the base URL has to be stripped off.
         $route_name = $child->getRouteName();
         $route_parameters = $child->getRouteParameters($this->request);
 
         // Find out whether the user has access to the task.
         $access = $this->accessManager->checkNamedRoute($route_name, $route_parameters);
         if ($access) {
-          // Need to flag the list element as active for a tab for the current
-          // route or if the plugin is set active (i.e. the parent tab).
-          $active = (($current_route_name == $route_name && (array_intersect_assoc($route_parameters, $this->request->attributes->all()) == $route_parameters)) || $child->getActive());
+          $active = $this->isRouteActive($current_route_name, $route_name, $route_parameters);
+
+          // The plugin may have been set active in getLocalTasksForRoute() if
+          // one of its child tabs is the active tab.
+          $active = $active || $child->getActive();
           // @todo It might make sense to use menu link entities instead of
           //   arrays.
 
@@ -293,4 +294,33 @@ public function getTasksBuild($current_route_name) {
     return $build;
   }
 
+  /**
+   * Determines whether the route of a certain local task is currently active.
+   *
+   * @param string $current_route_name
+   *   The route name of the current main request.
+   * @param string $route_name
+   *   The route name of the local task to determine the active status.
+   * @param array $route_parameters
+   *
+   * @return bool
+   *   Returns TRUE if the passed route_name and route_parameters is considered
+   *   as the same as the one from the request, otherwise FALSE.
+   */
+  protected function isRouteActive($current_route_name, $route_name, $route_parameters) {
+    // Flag the list element as active if this tab's route and parameters match
+    // the current request's route and route variables.
+    $active = $current_route_name == $route_name;
+    if ($active) {
+      // The request is injected, so we need to verify that we have the expected
+      // _raw_variables attribute.
+      $raw_variables_bag = $this->request->attributes->get('_raw_variables');
+      // If we don't have _raw_variables, we assume the attributes are still the
+      // original values.
+      $raw_variables = $raw_variables_bag ? $raw_variables_bag->all() : $this->request->attributes->all();
+      $active = array_intersect_assoc($route_parameters, $raw_variables) == $route_parameters;
+    }
+    return $active;
+  }
+
 }

core/modules/system/lib/Drupal/system/Tests/Menu/LocalTasksTest.php

@@ -14,7 +14,7 @@
  */
 class LocalTasksTest extends WebTestBase {
 
-  public static $modules = array('menu_test');
+  public static $modules = array('menu_test', 'entity_test');
 
   public static function getInfo() {
     return array(
@@ -198,6 +198,35 @@ public function testPluginLocalTask() {
     $this->assertEqual($definitions['menu_local_task_test_tasks_settings_sub1']['provider'], 'menu_test');
     $this->assertEqual($definitions['menu_local_task_test_tasks_settings_sub2']['provider'], 'menu_test');
     $this->assertEqual($definitions['menu_local_task_test_tasks_settings_sub3']['provider'], 'menu_test');
+
+    // Test that we we correctly apply the active class to tabs where one of the
+    // request attributes is upcast to an entity object.
+    $entity = \Drupal::entityManager()->getStorageController('entity_test')->create(array('bundle' => 'test'));
+    $entity->save();
+
+    $this->drupalGet('menu-local-task-test-upcasting/1/sub1');
+
+    $tasks = array(
+      'menu-local-task-test-upcasting/1/sub1',
+      'menu-local-task-test-upcasting/1/sub2',
+    );
+    $this->assertLocalTasks($tasks, 0);
+
+    $result = $this->xpath('//ul[contains(@class, "tabs")]//li[contains(@class, "active")]');
+    $this->assertEqual(1, count($result), 'There is one active tab.');
+    $this->assertEqual('upcasting sub1', (string) $result[0]->a, 'The "upcasting sub1" tab is active.');
+
+    $this->drupalGet('menu-local-task-test-upcasting/1/sub2');
+
+    $tasks = array(
+      'menu-local-task-test-upcasting/1/sub1',
+      'menu-local-task-test-upcasting/1/sub2',
+    );
+    $this->assertLocalTasks($tasks, 0);
+
+    $result = $this->xpath('//ul[contains(@class, "tabs")]//li[contains(@class, "active")]');
+    $this->assertEqual(1, count($result), 'There is one active tab.');
+    $this->assertEqual('upcasting sub2', (string) $result[0]->a, 'The "upcasting sub2" tab is active.');
   }
 
 }

core/modules/system/tests/modules/menu_test/lib/Drupal/menu_test/menu_test.local_tasks.yml

-menu_local_task_test_placeholder_sub1:
- route_name: menu_local_task_test_placeholder_sub1
- title: 'placeholder sub1'
- tab_root_id: menu_local_task_test_placeholder_sub1
-menu_local_task_test_placeholder_sub2:
- route_name: menu_local_task_test_placeholder_sub2
- title: 'placeholder sub2'
- tab_root_id: menu_local_task_test_placeholder_sub1

core/modules/system/tests/modules/menu_test/menu_test.local_tasks.yml

@@ -35,3 +35,20 @@ menu_local_task_test_tasks_settings_derived:
   tab_parent_id: menu_local_task_test_tasks_settings
   derivative: Drupal\menu_test\Plugin\Derivative\LocalTaskTest
   weight: 50
+menu_local_task_test.placeholder_sub1:
+  route_name: menu_test.local_task_test_placeholder_sub1
+  title: 'placeholder sub1'
+  tab_root_id: menu_local_task_test_placeholder_sub1
+menu_local_task_test_placeholder_sub2:
+  route_name: menu_test.local_task_test_placeholder_sub2
+  title: 'placeholder sub2'
+  tab_root_id: menu_local_task_test_placeholder_sub1
+menu_local_task_test.upcasting_sub1:
+  route_name: menu_test.local_task_test_upcasting_sub1
+  title: 'upcasting sub1'
+  tab_root_id: menu_local_task_test_upcasting_sub1
+menu_local_task_test_upcasting_sub2:
+  route_name: menu_test.local_task_test_upcasting_sub2
+  title: 'upcasting sub2'
+  tab_root_id: menu_local_task_test_upcasting_sub1
+  weight: 10

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

@@ -138,6 +138,20 @@ menu_test.local_task_test_placeholder_sub2:
   requirements:
     _access: 'TRUE'
 
+menu_test.local_task_test_upcasting_sub1:
+  path: '/menu-local-task-test-upcasting/{entity_test}/sub1'
+  defaults:
+    _content: '\Drupal\menu_test\TestControllers::test1'
+  requirements:
+    _access: 'TRUE'
+
+menu_test.local_task_test_upcasting_sub2:
+  path: '/menu-local-task-test-upcasting/{entity_test}/sub2'
+  defaults:
+    _content: '\Drupal\menu_test\TestControllers::test1'
+  requirements:
+    _access: 'TRUE'
+
 menu_test.optional_placeholder:
   path: '/menu-test/optional/{placeholder}'
   defaults: