diff --git a/core/core.services.yml b/core/core.services.yml index 65882af..cf60a29 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -176,7 +176,7 @@ services: arguments: ['@container.namespaces', '@controller_resolver', '@request', '@module_handler', '@cache.cache', '@language_manager'] plugin.manager.menu.local_task: class: Drupal\Core\Menu\LocalTaskManager - arguments: ['@container.namespaces', '@controller_resolver', '@request', '@router.route_provider', '@module_handler', '@cache.cache', '@language_manager'] + arguments: ['@container.namespaces', '@controller_resolver', '@request', '@router.route_provider', '@module_handler', '@cache.cache', '@language_manager', '@access_manager'] request: class: Symfony\Component\HttpFoundation\Request # @TODO the synthetic setting must be uncommented whenever drupal_session_initialize() diff --git a/core/includes/menu.inc b/core/includes/menu.inc index 0b59cfe..4ce2d5d 100644 --- a/core/includes/menu.inc +++ b/core/includes/menu.inc @@ -970,7 +970,8 @@ function _menu_link_translate(&$item, $translate = FALSE) { */ function menu_item_route_access(Route $route, $href, &$map) { $request = Request::create('/' . $href); - $request->attributes->set('_system_path', $href); + $system_path = $href = \Drupal::service('path_processor_manager')->processInbound($href, $request); + $request->attributes->set('_system_path', $system_path); // Attempt to match this path to provide a fully built request to the // access checker. try { @@ -1716,8 +1717,15 @@ function theme_menu_local_task($variables) { $link['localized_options']['html'] = TRUE; $link_text = t('!local-task-title!active', array('!local-task-title' => $link['title'], '!active' => $active)); } + if (!empty($link['href'])) { + // @todo - remove this once all pages are converted to routes. + $a_tag = l($link_text, $link['href'], $link['localized_options']); + } + else { + $a_tag = Drupal::l($link_text, $link['route_name'], $link['route_parameters'], $link['localized_options']); + } - return '' . l($link_text, $link['href'], $link['localized_options']) . ''; + return '' . $a_tag . ''; } /** @@ -1957,6 +1965,20 @@ function menu_local_tasks($level = 0) { $actions = $empty['actions']; $tabs = array(); + // Look for route-based tabs. + $data['tabs'] = array(); + $data['actions'] = array(); + + $route_name = Drupal::request()->attributes->get('_route'); + if (!empty($route_name)) { + $manager = Drupal::service('plugin.manager.menu.local_task'); + $local_tasks = $manager->getTasksBuild($route_name); + foreach ($local_tasks as $level => $items) { + $data['tabs'][$level] = empty($data['tabs'][$level]) ? $items : array_merge($data['tabs'][$level], $items); + } + } + + // @todo Remove the code below once the old menu router system got removed. $router_item = menu_get_item(); // If this router item points to its parent, start from the parents to @@ -2142,16 +2164,7 @@ function menu_local_tasks($level = 0) { ksort($tabs); // Remove the depth, we are interested only in their relative placement. $tabs = array_values($tabs); - $data['tabs'] = $tabs; - // Look for route-based tabs. - $route_name = Drupal::request()->attributes->get('_route'); - if (!empty($route_name)) { - $manager = Drupal::service('plugin.manager.menu.local_task'); - $local_tasks = $manager->getTasksBuild($route_name); - foreach ($local_tasks as $level => $items) { - $data['tabs'][$level] = empty($data['tabs'][$level]) ? $items : array_merge($data['tabs'][$level], $items); - } - } + $data['tabs'] += $tabs; // Allow modules to dynamically add further tasks. $module_handler = Drupal::moduleHandler(); diff --git a/core/lib/Drupal/Core/Annotation/Menu/LocalTask.php b/core/lib/Drupal/Core/Annotation/Menu/LocalTask.php index 52ef6a1..bb97814 100644 --- a/core/lib/Drupal/Core/Annotation/Menu/LocalTask.php +++ b/core/lib/Drupal/Core/Annotation/Menu/LocalTask.php @@ -40,6 +40,13 @@ class LocalTask extends Plugin { public $route_name; /** + * The route name. + * + * @var array + */ + public $route_parameters = array(); + + /** * The plugin ID of the root tab. * * @var array @@ -49,7 +56,7 @@ class LocalTask extends Plugin { /** * The plugin ID of the parent tab (or NULL for a top-level tab). * - * @var array|NULL + * @var string|NULL */ public $tab_parent_id; diff --git a/core/lib/Drupal/Core/Menu/LocalTaskBase.php b/core/lib/Drupal/Core/Menu/LocalTaskBase.php index 7568d2b..7ba3d18 100644 --- a/core/lib/Drupal/Core/Menu/LocalTaskBase.php +++ b/core/lib/Drupal/Core/Menu/LocalTaskBase.php @@ -9,8 +9,10 @@ use Drupal\Component\Plugin\PluginBase; use Drupal\Core\Plugin\ContainerFactoryPluginInterface; +use Drupal\Core\Routing\RouteProviderInterface; use Drupal\Core\StringTranslation\Translator\TranslatorInterface; use Symfony\Component\DependencyInjection\ContainerInterface; +use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; /** @@ -26,11 +28,11 @@ protected $t; /** - * URL generator object. + * The route provider to load routes by name. * - * @var \Symfony\Component\Routing\Generator\UrlGeneratorInterface + * @var \Drupal\Core\Routing\RouteProviderInterface */ - protected $generator; + protected $routeProvider; /** * TRUE if this plugin is forced active for options attributes. @@ -50,13 +52,13 @@ * The plugin implementation definition. * @param \Drupal\Core\StringTranslation\Translator\TranslatorInterface $string_translation * The string translation object. - * @param \Symfony\Component\Routing\Generator\UrlGeneratorInterface $generator - * The url generator object. + * @param \Drupal\Core\Routing\RouteProviderInterface $route_provider + * The route provider to load routes by name. */ - public function __construct(array $configuration, $plugin_id, array $plugin_definition, TranslatorInterface $string_translation, UrlGeneratorInterface $generator) { + public function __construct(array $configuration, $plugin_id, array $plugin_definition, TranslatorInterface $string_translation, RouteProviderInterface $route_provider) { // This is available for subclasses that need to translate a dynamic title. $this->t = $string_translation; - $this->generator = $generator; + $this->routeProvider = $route_provider; parent::__construct($configuration, $plugin_id, $plugin_definition); } @@ -69,7 +71,7 @@ public static function create(ContainerInterface $container, array $configuratio $plugin_id, $plugin_definition, $container->get('string_translation'), - $container->get('url_generator') + $container->get('router.route_provider') ); } @@ -83,25 +85,43 @@ public function getRouteName() { /** * {@inheritdoc} */ - public function getTitle() { - // Subclasses may pull in the request or specific attributes as parameters. - return $this->pluginDefinition['title']; + public function getRouteParameters(Request $request) { + $parameters = isset($this->pluginDefinition['route_parameters']) ? $this->pluginDefinition['route_parameters'] : array(); + $route = $this->routeProvider->getRouteByName($this->getRouteName()); + $variables = $route->compile()->getVariables(); + + // Normally the \Drupal\Core\ParamConverter\ParamConverterManager has + // processed the Request attributes, and in that case the _raw_variables + // attribute holds the original path strings keyed to the corresponding + // slugs in the path patterns. For example, if the route's path pattern is + // /filter/tips/{filter_format} and the path is /filter/tips/plain_text then + // $raw_variables->get('filter_format') == 'plain_text'. + + $raw_variables = $request->attributes->get('_raw_variables'); + + foreach ($variables as $name) { + if (isset($parameters[$name])) { + continue; + } + + if ($raw_variables && $raw_variables->has($name)) { + $parameters[$name] = $raw_variables->get($name); + } + elseif ($request->attributes->has($name)) { + $parameters[$name] = $request->attributes->get($name); + } + } + // The UrlGenerator will throw an exception if expected parameters are + // missing. This method should be overridden if that is possible. + return $parameters; } /** * {@inheritdoc} */ - public function getPath() { - // Subclasses may set a request into the generator or use any desired method - // to generate the path. - // @todo - use the new method from https://drupal.org/node/2031353 - $path = $this->generator->generate($this->getRouteName()); - // In order to get the Drupal path the base URL has to be stripped off. - $base_url = $this->generator->getContext()->getBaseUrl(); - if (!empty($base_url) && strpos($path, $base_url) === 0) { - $path = substr($path, strlen($base_url)); - } - return trim($path, '/'); + public function getTitle() { + // Subclasses may pull in the request or specific attributes as parameters. + return $this->pluginDefinition['title']; } /** @@ -127,7 +147,7 @@ public function getWeight() { /** * {@inheritdoc} */ - public function getOptions() { + public function getOptions(Request $request) { $options = $this->pluginDefinition['options']; if ($this->active) { if (empty($options['attributes']['class']) || !in_array('active', $options['attributes']['class'])) { diff --git a/core/lib/Drupal/Core/Menu/LocalTaskInterface.php b/core/lib/Drupal/Core/Menu/LocalTaskInterface.php index b7f581d..790e801 100644 --- a/core/lib/Drupal/Core/Menu/LocalTaskInterface.php +++ b/core/lib/Drupal/Core/Menu/LocalTaskInterface.php @@ -7,6 +7,8 @@ namespace Drupal\Core\Menu; +use Symfony\Component\HttpFoundation\Request; + /** * Defines an interface for menu local tasks. */ @@ -32,15 +34,17 @@ public function getRouteName(); public function getTitle(); /** - * Returns an internal Drupal path to use when creating the link for the tab. + * Returns the route parameters needed to render a link for the local task. * - * Subclasses may add optional arguments like NodeInterface $node = NULL that - * will be supplied by the ControllerResolver. + * @param \Symfony\Component\HttpFoundation\Request $request + * The HttpRequest object representing the current request. * - * @return string - * The path of this local task. + * @return array + * An array of parameter names and values. + * + * @see \Drupal\Core\Utility\LinkGeneratorInterface::generate() */ - public function getPath(); + public function getRouteParameters(Request $request); /** * Returns the weight of the local task. @@ -51,14 +55,17 @@ public function getPath(); public function getWeight(); /** - * Returns an array of options suitable to pass to l(). + * Returns options for rendering a link to the local task. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The HttpRequest object representing the current request. * * @return array - * Associative array of options. + * An array of options. * - * @see l() + * @see \Drupal\Core\Utility\LinkGeneratorInterface::generate() */ - public function getOptions(); + public function getOptions(Request $request); /** * Sets the active status. diff --git a/core/lib/Drupal/Core/Menu/LocalTaskManager.php b/core/lib/Drupal/Core/Menu/LocalTaskManager.php index 17b5128..349caf3 100644 --- a/core/lib/Drupal/Core/Menu/LocalTaskManager.php +++ b/core/lib/Drupal/Core/Menu/LocalTaskManager.php @@ -7,14 +7,16 @@ namespace Drupal\Core\Menu; +use Drupal\Core\Access\AccessManager; use Drupal\Core\Cache\CacheBackendInterface; +use Drupal\Core\Controller\ControllerResolverInterface; use Drupal\Core\Extension\ModuleHandlerInterface; use Drupal\Core\Language\Language; use Drupal\Core\Language\LanguageManager; use Drupal\Core\Plugin\DefaultPluginManager; use Drupal\Core\Routing\RouteProviderInterface; +use Drupal\Core\Routing\UrlGeneratorInterface; use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\HttpKernel\Controller\ControllerResolverInterface; /** * Manages discovery and instantiation of menu local task plugins. @@ -28,7 +30,7 @@ class LocalTaskManager extends DefaultPluginManager { /** * A controller resolver object. * - * @var \Symfony\Component\HttpKernel\Controller\ControllerResolverInterface + * @var \Drupal\Core\Controller\ControllerResolverInterface */ protected $controllerResolver; @@ -54,12 +56,19 @@ class LocalTaskManager extends DefaultPluginManager { protected $routeProvider; /** + * The access manager. + * + * @var \Drupal\Core\Access\AccessManager + */ + protected $accessManager; + + /** * Constructs a \Drupal\Core\Menu\LocalTaskManager object. * * @param \Traversable $namespaces * An object that implements \Traversable which contains the root paths * keyed by the corresponding namespace to look for plugin implementations, - * @param \Symfony\Component\HttpKernel\Controller\ControllerResolverInterface $controller_resolver + * @param \Drupal\Core\Controller\ControllerResolverInterface $controller_resolver * An object to use in introspecting route methods. * @param \Symfony\Component\HttpFoundation\Request $request * The request object to use for building titles and paths for plugin instances. @@ -71,12 +80,15 @@ class LocalTaskManager extends DefaultPluginManager { * The cache backend. * @param \Drupal\Core\Language\LanguageManager $language_manager * The language manager. + * @param \Drupal\Core\Access\AccessManager $access_manager + * The access manager. */ - public function __construct(\Traversable $namespaces, ControllerResolverInterface $controller_resolver, Request $request, RouteProviderInterface $route_provider, ModuleHandlerInterface $module_handler, CacheBackendInterface $cache, LanguageManager $language_manager) { + public function __construct(\Traversable $namespaces, ControllerResolverInterface $controller_resolver, Request $request, RouteProviderInterface $route_provider, ModuleHandlerInterface $module_handler, CacheBackendInterface $cache, LanguageManager $language_manager, AccessManager $access_manager) { parent::__construct('Plugin/Menu/LocalTask', $namespaces, array(), 'Drupal\Core\Annotation\Menu\LocalTask'); $this->controllerResolver = $controller_resolver; $this->request = $request; $this->routeProvider = $route_provider; + $this->accessManager = $access_manager; $this->alterInfo($module_handler, 'local_tasks'); $this->setCacheBackend($cache, $language_manager, 'local_task', array('local_task' => TRUE)); } @@ -97,21 +109,6 @@ public function getTitle(LocalTaskInterface $local_task) { } /** - * Gets the Drupal path for a local task. - * - * @param \Drupal\Core\Menu\LocalTaskInterface $local_task - * The local task plugin instance to get the path for. - * - * @return string - * The path. - */ - public function getPath(LocalTaskInterface $local_task) { - $controller = array($local_task, 'getPath'); - $arguments = $this->controllerResolver->getArguments($this->request, $controller); - return call_user_func_array($controller, $arguments); - } - - /** * Find all local tasks that appear on a named route. * * @param string $route_name @@ -205,14 +202,14 @@ public function getLocalTasksForRoute($route_name) { /** * Gets the render array for all local tasks. * - * @param string $route_name + * @param string $current_route_name * The route for which to make renderable local tasks. * * @return array * A render array as expected by theme_menu_local_tasks. */ - public function getTasksBuild($route_name) { - $tree = $this->getLocalTasksForRoute($route_name); + public function getTasksBuild($current_route_name) { + $tree = $this->getLocalTasksForRoute($current_route_name); $build = array(); // Collect all route names. @@ -222,31 +219,34 @@ public function getTasksBuild($route_name) { $route_names[] = $child->getRouteName(); } } - // Fetches all routes involved in the tree. + // Pre-fetch all routes involved in the tree. This reduces the number + // of SQL queries that would otherwise be triggered by the access manager. $routes = $route_names ? $this->routeProvider->getRoutesByNames($route_names) : array(); foreach ($tree as $level => $instances) { - foreach ($instances as $child) { - $path = $this->getPath($child); + 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. - $route = $routes[$child->getRouteName()]; - $map = array(); - // @todo - replace this call when we have a real service for it. - $access = menu_item_route_access($route, $path, $map); + $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 = ($route_name == $child->getRouteName() || $child->getActive()); + $active = ($current_route_name == $route_name || $child->getActive()); // @todo It might make sense to use menu link entities instead of // arrays. - $menu_link = array( + + $link = array( 'title' => $this->getTitle($child), - 'href' => $path, - 'localized_options' => $child->getOptions(), + 'route_name' => $route_name, + 'route_parameters' => $route_parameters, + 'localized_options' => $child->getOptions($this->request), ); - $build[$level][$path] = array( + $build[$level][$plugin_id] = array( '#theme' => 'menu_local_task', - '#link' => $menu_link, + '#link' => $link, '#active' => $active, '#weight' => $child->getWeight(), '#access' => $access, diff --git a/core/modules/comment/comment.module b/core/modules/comment/comment.module index 973bccb..436ab96 100644 --- a/core/modules/comment/comment.module +++ b/core/modules/comment/comment.module @@ -206,28 +206,11 @@ function comment_menu() { 'title' => 'Comment permalink', 'route_name' => 'comment_permalink', ); - $items['comment/%comment/view'] = array( - 'title' => 'View comment', - 'type' => MENU_DEFAULT_LOCAL_TASK, - ); - // Every other comment path uses %, but this one loads the comment directly, - // so we don't end up loading it twice (in the page and access callback). - $items['comment/%comment/edit'] = array( - 'title' => 'Edit', - 'type' => MENU_LOCAL_TASK, - 'route_name' => 'comment_edit_page', - ); $items['comment/%comment/approve'] = array( 'title' => 'Approve', 'weight' => 10, 'route_name' => 'comment_approve', ); - $items['comment/%comment/delete'] = array( - 'title' => 'Delete', - 'type' => MENU_LOCAL_TASK, - 'route_name' => 'comment_confirm_delete', - 'weight' => 20, - ); $items['comment/reply/%node'] = array( 'title' => 'Add new comment', 'page callback' => 'comment_reply', diff --git a/core/modules/comment/lib/Drupal/comment/Plugin/Menu/LocalTask/CommentDeleteTask.php b/core/modules/comment/lib/Drupal/comment/Plugin/Menu/LocalTask/CommentDeleteTask.php new file mode 100644 index 0000000..265ec92 --- /dev/null +++ b/core/modules/comment/lib/Drupal/comment/Plugin/Menu/LocalTask/CommentDeleteTask.php @@ -0,0 +1,25 @@ +xpath('//ul[contains(@class, "tabs")]//a[contains(@class, "active")]'); + $result = $this->xpath('//ul[contains(@class, "tabs")]//li[contains(@class, "active")]/a'); $this->assertEqual(1, count($result), 'There is just a single active tab.'); $this->assertEqual('View', (string) $result[0], 'The view tab is active.'); @@ -167,7 +167,7 @@ public function testPluginLocalTask() { 'menu-local-task-test/tasks/settings/sub2', ), 1); - $result = $this->xpath('//ul[contains(@class, "tabs")]//a[contains(@class, "active")]'); + $result = $this->xpath('//ul[contains(@class, "tabs")]//li[contains(@class, "active")]/a'); $this->assertEqual(1, count($result), 'There is just a single active tab.'); $this->assertEqual('Settings', (string) $result[0], 'The settings tab is active.'); @@ -178,10 +178,28 @@ public function testPluginLocalTask() { 'menu-local-task-test/tasks/settings/sub2', ), 1); - $result = $this->xpath('//ul[contains(@class, "tabs")]//a[contains(@class, "active")]'); + $result = $this->xpath('//ul[contains(@class, "tabs")]//li[contains(@class, "active")]/a'); $this->assertEqual(2, count($result), 'There are tabs active on both levels.'); $this->assertEqual('Settings', (string) $result[0], 'The settings tab is active.'); $this->assertEqual('sub1', (string) $result[1], 'The sub1 tab is active.'); + + $this->drupalGet('menu-local-task-test-dynamic/test/sub1'); + $this->assertLocalTasks(array( + 'menu-local-task-test-dynamic/test/sub1', + 'menu-local-task-test-dynamic/test/sub2', + )); + $result = $this->xpath('//ul[contains(@class, "tabs")]//li[contains(@class, "active")]/a'); + $this->assertEqual(1, count($result), 'There is just a single active tab.'); + $this->assertEqual('placeholder sub1', (string) $result[0], 'The sub1 tab is active.'); + + $this->drupalGet('menu-local-task-test-dynamic/test/sub2'); + $this->assertLocalTasks(array( + 'menu-local-task-test-dynamic/test/sub1', + 'menu-local-task-test-dynamic/test/sub2', + )); + $result = $this->xpath('//ul[contains(@class, "tabs")]//li[contains(@class, "active")]/a'); + $this->assertEqual(1, count($result), 'There is just a single active tab.'); + $this->assertEqual('placeholder sub2', (string) $result[0], 'The sub2 tab is active.'); } } diff --git a/core/modules/system/tests/modules/menu_test/menu_test.routing.yml b/core/modules/system/tests/modules/menu_test/menu_test.routing.yml index 47d3a87..3014858 100644 --- a/core/modules/system/tests/modules/menu_test/menu_test.routing.yml +++ b/core/modules/system/tests/modules/menu_test/menu_test.routing.yml @@ -108,6 +108,20 @@ menu_local_task_test_tasks_settings_sub2: requirements: _access: 'TRUE' +menu_local_task_test_placeholder_sub1: + pattern: '/menu-local-task-test-dynamic/{placeholder}/sub1' + defaults: + _content: '\Drupal\menu_test\TestControllers::test1' + requirements: + _access: 'TRUE' + +menu_local_task_test_placeholder_sub2: + pattern: '/menu-local-task-test-dynamic/{placeholder}/sub2' + defaults: + _content: '\Drupal\menu_test\TestControllers::test1' + requirements: + _access: 'TRUE' + menu_test_optional_placeholder: pattern: '/menu-test/optional/{placeholder}' defaults: diff --git a/core/tests/Drupal/Tests/Core/Menu/LocalTaskManagerTest.php b/core/tests/Drupal/Tests/Core/Menu/LocalTaskManagerTest.php index 8ac918d..ce5d08e 100644 --- a/core/tests/Drupal/Tests/Core/Menu/LocalTaskManagerTest.php +++ b/core/tests/Drupal/Tests/Core/Menu/LocalTaskManagerTest.php @@ -71,6 +71,13 @@ class LocalTaskManagerTest extends UnitTestCase { */ protected $cacheBackend; + /** + * The mocked access manager. + * + * @var \Drupal\Core\Access\AccessManager|\PHPUnit_Framework_MockObject_MockObject + */ + protected $accessManager; + public static function getInfo() { return array( 'name' => 'Local tasks manager.', @@ -91,6 +98,9 @@ protected function setUp() { $this->pluginDiscovery = $this->getMock('Drupal\Component\Plugin\Discovery\DiscoveryInterface'); $this->factory = $this->getMock('Drupal\Component\Plugin\Factory\FactoryInterface'); $this->cacheBackend = $this->getMock('Drupal\Core\Cache\CacheBackendInterface'); + $this->accessManager = $this->getMockBuilder('Drupal\Core\Access\AccessManager') + ->disableOriginalConstructor() + ->getMock(); $this->setupLocalTaskManager(); } @@ -210,24 +220,6 @@ public function testGetTitle() { } /** - * Tests the getPath method. - * - * @see \Drupal\system\Plugin\Type\MenuLocalTaskManager::getPath() - */ - public function testGetPath() { - $menu_local_task = $this->getMock('Drupal\Core\Menu\LocalTaskInterface'); - $menu_local_task->expects($this->once()) - ->method('getPath'); - - $this->controllerResolver->expects($this->once()) - ->method('getArguments') - ->with($this->request, array($menu_local_task, 'getPath')) - ->will($this->returnValue(array())); - - $this->manager->getPath($menu_local_task); - } - - /** * Setups the local task manager for the test. */ protected function setupLocalTaskManager() { @@ -245,6 +237,10 @@ protected function setupLocalTaskManager() { $property->setAccessible(TRUE); $property->setValue($this->manager, $this->request); + $property = new \ReflectionProperty('Drupal\Core\Menu\LocalTaskManager', 'accessManager'); + $property->setAccessible(TRUE); + $property->setValue($this->manager, $this->accessManager); + $property = new \ReflectionProperty('Drupal\Core\Menu\LocalTaskManager', 'discovery'); $property->setAccessible(TRUE); $property->setValue($this->manager, $this->pluginDiscovery);