Commit 4b12bbf0 authored by alexpott's avatar alexpott

Issue #2338081 by pwolanin, dawehner, effulgentsia, kgoel, vijaycs85,...

Issue #2338081 by pwolanin, dawehner, effulgentsia, kgoel, vijaycs85, mpdonadio, chx, brandon.holtsclaw, alexpott, Gábor Hojtsy, Fabianx, catch, YesCT, JvE: Local Tasks, Actions, and Contextual links mark strings from derivatives (or alter hooks) as safe and translated
parent 61603f58
......@@ -40,10 +40,6 @@ function template_preprocess_menu_local_task(&$variables) {
$active = SafeMarkup::format('<span class="visually-hidden">@label</span>', array('@label' => t('(active tab)')));
$link_text = t('@local-task-title@active', array('@local-task-title' => $link_text, '@active' => $active));
}
else {
// @todo Remove this once https://www.drupal.org/node/2338081 is fixed.
$link_text = SafeMarkup::checkPlain($link_text);
}
$link['localized_options']['set_active_class'] = TRUE;
......
......@@ -7,7 +7,7 @@
namespace Drupal\Core\Menu;
use Drupal\Core\Plugin\PluginBase;
use Drupal\Component\Plugin\PluginBase;
use Symfony\Component\HttpFoundation\Request;
/**
......@@ -17,21 +17,10 @@ class ContextualLinkDefault extends PluginBase implements ContextualLinkInterfac
/**
* {@inheritdoc}
*
* @todo: It might be helpful at some point to move this getTitle logic into
* a trait.
*/
public function getTitle(Request $request = NULL) {
$options = array();
if (!empty($this->pluginDefinition['title_context'])) {
$options['context'] = $this->pluginDefinition['title_context'];
}
$args = array();
if (isset($this->pluginDefinition['title_arguments']) && $title_arguments = $this->pluginDefinition['title_arguments']) {
$args = (array) $title_arguments;
}
return $this->t($this->pluginDefinition['title'], $args, $options);
// The title from YAML file discovery may be a TranslationWrapper object.
return (string) $this->pluginDefinition['title'];
}
/**
......
......@@ -118,8 +118,9 @@ public function __construct(ControllerResolverInterface $controller_resolver, Mo
*/
protected function getDiscovery() {
if (!isset($this->discovery)) {
$this->discovery = new YamlDiscovery('links.contextual', $this->moduleHandler->getModuleDirectories());
$this->discovery = new ContainerDerivativeDiscoveryDecorator($this->discovery);
$yaml_discovery = new YamlDiscovery('links.contextual', $this->moduleHandler->getModuleDirectories());
$yaml_discovery->addTranslatableProperty('title', 'title_context');
$this->discovery = new ContainerDerivativeDiscoveryDecorator($yaml_discovery);
}
return $this->discovery;
}
......
......@@ -7,8 +7,9 @@
namespace Drupal\Core\Menu;
use Drupal\Component\Plugin\PluginBase;
use Drupal\Core\DependencyInjection\DependencySerializationTrait;
use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
use Drupal\Core\Plugin\PluginBase;
use Drupal\Core\Routing\RouteMatchInterface;
use Drupal\Core\Routing\RouteProviderInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;
......@@ -19,6 +20,8 @@
*/
class LocalActionDefault extends PluginBase implements LocalActionInterface, ContainerFactoryPluginInterface {
use DependencySerializationTrait;
/**
* The route provider to load routes by name.
*
......@@ -68,15 +71,8 @@ public function getRouteName() {
*/
public function getTitle(Request $request = NULL) {
// Subclasses may pull in the request or specific attributes as parameters.
$options = array();
if (!empty($this->pluginDefinition['title_context'])) {
$options['context'] = $this->pluginDefinition['title_context'];
}
$args = array();
if (isset($this->pluginDefinition['title_arguments']) && $title_arguments = $this->pluginDefinition['title_arguments']) {
$args = (array) $title_arguments;
}
return $this->t($this->pluginDefinition['title'], $args, $options);
// The title from YAML file discovery may be a TranslationWrapper object.
return (string) $this->pluginDefinition['title'];
}
/**
......
......@@ -142,8 +142,9 @@ public function __construct(ControllerResolverInterface $controller_resolver, Re
*/
protected function getDiscovery() {
if (!isset($this->discovery)) {
$this->discovery = new YamlDiscovery('links.action', $this->moduleHandler->getModuleDirectories());
$this->discovery = new ContainerDerivativeDiscoveryDecorator($this->discovery);
$yaml_discovery = new YamlDiscovery('links.action', $this->moduleHandler->getModuleDirectories());
$yaml_discovery->addTranslatableProperty('title', 'title_context');
$this->discovery = new ContainerDerivativeDiscoveryDecorator($yaml_discovery);
}
return $this->discovery;
}
......
......@@ -7,7 +7,8 @@
namespace Drupal\Core\Menu;
use Drupal\Core\Plugin\PluginBase;
use Drupal\Component\Plugin\PluginBase;
use Drupal\Core\DependencyInjection\DependencySerializationTrait;
use Drupal\Core\Routing\RouteMatchInterface;
use Symfony\Component\HttpFoundation\Request;
......@@ -16,6 +17,8 @@
*/
class LocalTaskDefault extends PluginBase implements LocalTaskInterface {
use DependencySerializationTrait;
/**
* The route provider to load routes by name.
*
......@@ -75,16 +78,8 @@ public function getRouteParameters(RouteMatchInterface $route_match) {
* {@inheritdoc}
*/
public function getTitle(Request $request = NULL) {
// Subclasses may pull in the request or specific attributes as parameters.
$options = array();
if (!empty($this->pluginDefinition['title_context'])) {
$options['context'] = $this->pluginDefinition['title_context'];
}
$args = array();
if (isset($this->pluginDefinition['title_arguments']) && $title_arguments = $this->pluginDefinition['title_arguments']) {
$args = (array) $title_arguments;
}
return $this->t($this->pluginDefinition['title'], $args, $options);
// The title from YAML file discovery may be a TranslationWrapper object.
return (string) $this->pluginDefinition['title'];
}
/**
......
......@@ -142,8 +142,9 @@ public function __construct(ControllerResolverInterface $controller_resolver, Re
*/
protected function getDiscovery() {
if (!isset($this->discovery)) {
$this->discovery = new YamlDiscovery('links.task', $this->moduleHandler->getModuleDirectories());
$this->discovery = new ContainerDerivativeDiscoveryDecorator($this->discovery);
$yaml_discovery = new YamlDiscovery('links.task', $this->moduleHandler->getModuleDirectories());
$yaml_discovery->addTranslatableProperty('title', 'title_context');
$this->discovery = new ContainerDerivativeDiscoveryDecorator($yaml_discovery);
}
return $this->discovery;
}
......
......@@ -10,9 +10,18 @@
use Drupal\Component\Plugin\Discovery\DiscoveryInterface;
use Drupal\Component\Discovery\YamlDiscovery as ComponentYamlDiscovery;
use Drupal\Component\Plugin\Discovery\DiscoveryTrait;
use Drupal\Core\StringTranslation\TranslationWrapper;
/**
* Allows YAML files to define plugin definitions.
*
* If the value of a key (like title) in the definition is translatable then
* the addTranslatableProperty() method can be used to mark it as such and also
* to add translation context. Then
* \Drupal\Core\StringTranslation\TranslationWrapper will be used to translate
* the string and also to mark it safe. Only strings written in the YAML files
* should be marked as safe, strings coming from dynamic plugin definitions
* potentially containing user input should not.
*/
class YamlDiscovery implements DiscoveryInterface {
......@@ -25,6 +34,15 @@ class YamlDiscovery implements DiscoveryInterface {
*/
protected $discovery;
/**
* Contains an array of translatable properties passed along to t().
*
* @see \Drupal\Core\Plugin\Discovery\YamlDiscovery::addTranslatableProperty()
*
* @var array
*/
protected $translatableProperties = [];
/**
* Construct a YamlDiscovery object.
*
......@@ -38,6 +56,23 @@ function __construct($name, array $directories) {
$this->discovery = new ComponentYamlDiscovery($name, $directories);
}
/**
* Set one of the YAML values as being translatable.
*
* @param string $value_key
* The key corresponding to the value in the YAML that contains a
* translatable string.
* @param string $context_key
* (Optional) the translation context for the value specified by the
* $value_key.
*
* @return $this
*/
public function addTranslatableProperty($value_key, $context_key = '') {
$this->translatableProperties[$value_key] = $context_key;
return $this;
}
/**
* {@inheritdoc}
*/
......@@ -48,6 +83,20 @@ public function getDefinitions() {
$definitions = array();
foreach ($plugins as $provider => $list) {
foreach ($list as $id => $definition) {
// Add translation wrappers.
foreach ($this->translatableProperties as $property => $context_key) {
if (isset($definition[$property])) {
$options = [];
// Move the t() context from the definition to the translation
// wrapper.
if ($context_key && isset($definition[$context_key])) {
$options['context'] = $definition[$context_key];
unset($definition[$context_key]);
}
$definition[$property] = new TranslationWrapper($definition[$property], [], $options);
}
}
// Add ID and provider.
$definitions[$id] = $definition + array(
'provider' => $provider,
'id' => $id,
......
......@@ -84,6 +84,16 @@ public function getOption($name) {
return isset($this->options[$name]) ? $this->options[$name] : '';
}
/**
* Gets all options from this translation wrapper.
*
* @return mixed[]
* The array of options.
*/
public function getOptions() {
return $this->options;
}
/**
* Implements the magic __toString() method.
*/
......
......@@ -10,12 +10,14 @@
use Drupal\comment\CommentStorageInterface;
use Drupal\Core\Menu\LocalTaskDefault;
use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
use Drupal\Core\StringTranslation\StringTranslationTrait;
use Symfony\Component\DependencyInjection\ContainerInterface;
/**
* Provides a local task that shows the amount of unapproved comments.
*/
class UnapprovedComments extends LocalTaskDefault implements ContainerFactoryPluginInterface {
use StringTranslationTrait;
/**
* The comment storage service.
......@@ -57,7 +59,7 @@ public static function create(ContainerInterface $container, array $configuratio
* {@inheritdoc}
*/
public function getTitle() {
return t('Unapproved comments (@count)', array('@count' => $this->commentStorage->getUnapprovedCount()));
return $this->t('Unapproved comments (@count)', array('@count' => $this->commentStorage->getUnapprovedCount()));
}
}
config_translation.contextual_links:
title: 'Translate @type_name'
deriver: 'Drupal\config_translation\Plugin\Derivative\ConfigTranslationContextualLinks'
weight: 100
config_translation.local_tasks:
title: 'Translate @type_name'
deriver: 'Drupal\config_translation\Plugin\Derivative\ConfigTranslationLocalTasks'
weight: 100
......@@ -9,11 +9,13 @@
use Drupal\Component\Utility\Unicode;
use Drupal\Core\Menu\ContextualLinkDefault;
use Drupal\Core\StringTranslation\StringTranslationTrait;
/**
* Defines a contextual link plugin with a dynamic title.
*/
class ConfigTranslationContextualLink extends ContextualLinkDefault {
use StringTranslationTrait;
/**
* The mapper plugin discovery service.
......@@ -26,17 +28,12 @@ class ConfigTranslationContextualLink extends ContextualLinkDefault {
* {@inheritdoc}
*/
public function getTitle() {
$options = array();
if (!empty($this->pluginDefinition['title_context'])) {
$options['context'] = $this->pluginDefinition['title_context'];
}
// Take custom 'config_translation_plugin_id' plugin definition key to
// retrieve title. We need to retrieve a runtime title (as opposed to
// storing the title on the plugin definition for the link) because
// it contains translated parts that we need in the runtime language.
// Use the custom 'config_translation_plugin_id' plugin definition key to
// retrieve the title. We need to retrieve a runtime title (as opposed to
// storing the title on the plugin definition for the link) because it
// contains translated parts that we need in the runtime language.
$type_name = Unicode::strtolower($this->mapperManager()->createInstance($this->pluginDefinition['config_translation_plugin_id'])->getTypeLabel());
return $this->t($this->pluginDefinition['title'], array('@type_name' => $type_name), $options);
return $this->t('Translate @type_name', array('@type_name' => $type_name));
}
/**
......
......@@ -9,11 +9,13 @@
use Drupal\Component\Utility\Unicode;
use Drupal\Core\Menu\LocalTaskDefault;
use Drupal\Core\StringTranslation\StringTranslationTrait;
/**
* Defines a local task plugin with a dynamic title.
*/
class ConfigTranslationLocalTask extends LocalTaskDefault {
use StringTranslationTrait;
/**
* The mapper plugin discovery service.
......@@ -26,17 +28,12 @@ class ConfigTranslationLocalTask extends LocalTaskDefault {
* {@inheritdoc}
*/
public function getTitle() {
$options = array();
if (!empty($this->pluginDefinition['title_context'])) {
$options['context'] = $this->pluginDefinition['title_context'];
}
// Take custom 'config_translation_plugin_id' plugin definition key to
// retrieve title. We need to retrieve a runtime title (as opposed to
// storing the title on the plugin definition for the link) because
// it contains translated parts that we need in the runtime language.
$type_name = Unicode::strtolower($this->mapperManager()->createInstance($this->pluginDefinition['config_translation_plugin_id'])->getTypeLabel());
return $this->t($this->pluginDefinition['title'], array('@type_name' => $type_name), $options);
return $this->t('Translate @type_name', array('@type_name' => $type_name));
}
/**
......
......@@ -8,6 +8,7 @@
namespace Drupal\contextual\Tests;
use Drupal\Component\Serialization\Json;
use Drupal\Core\Url;
use Drupal\language\Entity\ConfigurableLanguage;
use Drupal\simpletest\WebTestBase;
use Drupal\Core\Template\Attribute;
......@@ -46,7 +47,7 @@ class ContextualDynamicContextTest extends WebTestBase {
*
* @var array
*/
public static $modules = array('contextual', 'node', 'views', 'views_ui', 'language');
public static $modules = array('contextual', 'node', 'views', 'views_ui', 'language', 'menu_test');
protected function setUp() {
parent::setUp();
......@@ -137,6 +138,11 @@ function testDifferentPermissions() {
$id = 'node:node=' . $node3->id() . ':changed=' . $node3->getChangedTime() . '&langcode=it';
$this->drupalGet('node', ['language' => ConfigurableLanguage::createFromLangcode('it')]);
$this->assertContextualLinkPlaceHolder($id);
// Get a page where contextual links are directly rendered.
$this->drupalGet(Url::fromRoute('menu_test.contextual_test'));
$this->assertEscaped("<script>alert('Welcome to the jungle!')</script>");
$this->assertLink('Edit menu - contextual');
}
/**
......
......@@ -30,6 +30,8 @@ public function testLocalAction() {
// Ensure that both menu and route based actions are shown.
$this->assertLocalAction([
[Url::fromRoute('menu_test.local_action4'), 'My dynamic-title action'],
[Url::fromRoute('menu_test.local_action4'), htmlspecialchars("<script>alert('Welcome to the jungle!')</script>", ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8')],
[Url::fromRoute('menu_test.local_action4'), htmlspecialchars("<script>alert('Welcome to the derived jungle!')</script>", ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8')],
[Url::fromRoute('menu_test.local_action2'), 'My hook_menu action'],
[Url::fromRoute('menu_test.local_action3'), 'My YAML discovery action'],
[Url::fromRoute('menu_test.local_action5'), 'Title override'],
......@@ -50,7 +52,11 @@ protected function assertLocalAction(array $actions) {
foreach ($actions as $action) {
/** @var \Drupal\Core\Url $url */
list($url, $title) = $action;
$this->assertEqual((string) $elements[$index], $title);
// SimpleXML gives us the unescaped text, not the actual escaped markup,
// so use a pattern instead to check the raw content.
// This behaviour is a bug in libxml, see
// https://bugs.php.net/bug.php?id=49437.
$this->assertPattern('@<a [^>]*class="[^"]*button-action[^"]*"[^>]*>' . preg_quote($title, '@') . '</@');
$this->assertEqual($elements[$index]['href'], $url->toString());
$index++;
}
......
......@@ -47,18 +47,54 @@ protected function assertLocalTasks(array $routes, $level = 0) {
}
}
/**
* Ensures that some local task appears.
*
* @param string $title
* The expected title.
*
* @return bool
* TRUE if the local task exists on the page.
*/
protected function assertLocalTaskAppers($title) {
// SimpleXML gives us the unescaped text, not the actual escaped markup,
// so use a pattern instead to check the raw content.
// This behaviour is a bug in libxml, see
// https://bugs.php.net/bug.php?id=49437.
return $this->assertPattern('@<a [^>]*>' . preg_quote($title, '@') . '</a>@');
}
/**
* Tests the plugin based local tasks.
*/
public function testPluginLocalTask() {
// Verify local tasks defined in the hook.
$this->drupalGet(Url::fromRoute('menu_test.tasks_default'));
$this->assertLocalTasks([
['menu_test.tasks_default', []],
['menu_test.router_test1', ['bar' => 'unsafe']],
['menu_test.router_test1', ['bar' => '1']],
['menu_test.router_test2', ['bar' => '2']],
]);
// Verify that script tags are escaped on output.
$title = htmlspecialchars("Task 1 <script>alert('Welcome to the jungle!')</script>", ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8');
$this->assertLocalTaskAppers($title);
$title = htmlspecialchars("<script>alert('Welcome to the derived jungle!')</script>", ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8');
$this->assertLocalTaskAppers($title);
// Verify that local tasks appear as defined in the router.
$this->drupalGet(Url::fromRoute('menu_test.local_task_test_tasks_view'));
$this->assertLocalTasks([
['menu_test.local_task_test_tasks_view', []],
['menu_test.local_task_test_tasks_edit', []],
['menu_test.local_task_test_tasks_settings', []],
['menu_test.local_task_test_tasks_settings_dynamic', []],
]);
$title = htmlspecialchars("<script>alert('Welcome to the jungle!')</script>", ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8');
$this->assertLocalTaskAppers($title);
// Ensure the view tab is 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.');
......
......@@ -72,6 +72,8 @@ protected function doTestHookMenuIntegration() {
// Confirm local task links are displayed.
$this->assertLink('Local task A');
$this->assertLink('Local task B');
$this->assertNoLink('Local task C');
$this->assertEscaped("<script>alert('Welcome to the jungle!')</script>", ENT_QUOTES, 'UTF-8');
// Confirm correct local task href.
$this->assertLinkByHref(Url::fromRoute('menu_test.router_test1', ['bar' => $machine_name])->toString());
$this->assertLinkByHref(Url::fromRoute('menu_test.router_test2', ['bar' => $machine_name])->toString());
......
......@@ -26,6 +26,22 @@ menu_test.local_action4:
appears_on:
- menu_test.local_action1
menu_test.local_action_derivative:
route_name: menu_test.local_action4
weight: -20
deriver: Drupal\menu_test\Plugin\Derivative\LocalActionTest
class: Drupal\Core\Menu\LocalActionDefault
appears_on:
- menu_test.local_action1
menu_test.local_action6:
route_name: menu_test.local_action4
title: 'Dynamic local action with user input'
weight: -15
class: '\Drupal\menu_test\Plugin\Menu\LocalAction\TestLocalAction5'
appears_on:
- menu_test.local_action1
menu_test.hidden_menu_add:
route_name: menu_test.hidden_menu_add
title: 'Add menu'
......
menu_test.hidden_manage:
title: 'List links'
menu_test.contextual_hidden_manage:
title: 'List links - contextual'
group: menu_test_menu
route_name: menu_test.hidden_manage
route_name: menu_test.contextual_hidden_manage
class: '\Drupal\menu_test\Plugin\Menu\ContextualLink\TestContextualLink'
menu_test.hidden_manage_edit:
title: 'Edit menu'
menu_test.contextual_hidden_manage_edit:
title: 'Edit menu - contextual'
group: menu_test_menu
route_name: menu_test.hidden_manage_edit
route_name: menu_test.contextual_hidden_manage_edit
menu_test.hidden_block_configure:
title: 'Configure block'
......
......@@ -10,6 +10,10 @@ menu_test.local_task_test_tasks_settings:
route_name: menu_test.local_task_test_tasks_settings
title: Settings
base_route: menu_test.local_task_test_tasks_view
menu_test.local_task_test_tasks_settings_dynamic:
route_name: menu_test.local_task_test_tasks_settings_dynamic
base_route: menu_test.local_task_test_tasks_view
class: \Drupal\menu_test\Plugin\Menu\LocalTask\TestTaskWithUserInput
menu_test.local_task_test_tasks_settings_sub1:
route_name: menu_test.local_task_test_tasks_settings_sub1
title: sub1
......@@ -53,6 +57,11 @@ menu_test.tasks_default_tab:
route_name: menu_test.tasks_default
title: 'View'
base_route: menu_test.tasks_default
menu_test.tasks_default_derived:
route_name: menu_test.router_test1
title: 'Derived'
base_route: menu_test.tasks_default
deriver: '\Drupal\menu_test\Plugin\Derivative\LocalTaskTestWithUnsafeTitle'
menu_test.tasks_tasks_tab:
route_name: menu_test.tasks_tasks
......@@ -82,3 +91,8 @@ menu_test.router_test3:
route_name: menu_test.router_test3
title: 'Local task C'
base_route: menu_test.router_test1
menu_test.router_test4:
route_name: menu_test.router_test4
base_route: menu_test.router_test1
class: \Drupal\menu_test\Plugin\Menu\LocalTask\TestTaskWithUserInput
......@@ -5,6 +5,8 @@
* Module that implements various hooks for menu tests.
*/
use Drupal\Core\Url;
/**
* Implements hook_menu_links_discovered_alter().
*/
......@@ -26,20 +28,14 @@ function menu_test_menu_links_discovered_alter(&$links) {
/**
* Implements hook_menu_local_tasks().
*
* If the menu_test.settings configuration 'tasks.add' has been set, adds
* several local tasks to menu-test/tasks.
*/
function menu_test_menu_local_tasks(&$data, $route_name) {
if (!\Drupal::config('menu_test.settings')->get('tasks.add')) {
return;
}
if (in_array($route_name, array('menu_test.tasks_default', 'menu_test.tasks_empty', 'menu_test.tasks_tasks'))) {
if (in_array($route_name, array('menu_test.tasks_default'))) {
$data['tabs'][0]['foo'] = array(
'#theme' => 'menu_local_task',
'#link' => array(
'title' => 'Task 1',
'href' => 'task/foo',
'title' => "Task 1 <script>alert('Welcome to the jungle!')</script>",
'url' => Url::fromRoute('menu_test.router_test1', array('bar' => '1')),
),
'#weight' => 10,
);
......@@ -47,7 +43,7 @@ function menu_test_menu_local_tasks(&$data, $route_name) {
'#theme' => 'menu_local_task',
'#link' => array(
'title' => 'Task 2',
'href' => 'task/bar',
'url' => Url::fromRoute('menu_test.router_test2', array('bar' => '2')),
),
'#weight' => 20,
);
......
......@@ -67,6 +67,13 @@ menu_test.router_test3:
requirements:
_access: 'FALSE'
menu_test.router_test4:
path: '/foo/{bar}/d'
defaults:
_controller: '\Drupal\menu_test\TestControllers::test2'
requirements:
_access: 'TRUE'
menu_test.local_action1:
path: '/menu-test-local-action'
defaults:
......@@ -102,6 +109,27 @@ menu_test.local_action5:
requirements:
_access: 'TRUE'