.../Drupal/block/Tests/Views/DisplayBlockTest.php | 4 +-- core/modules/contextual/contextual.module | 26 +++++++++++--------- .../lib/Drupal/contextual/ContextualController.php | 4 +-- .../menu/lib/Drupal/menu/Tests/MenuTest.php | 25 ++++++++++++++++--- core/modules/views_ui/views_ui.module | 12 --------- 5 files changed, 40 insertions(+), 31 deletions(-) diff --git a/core/modules/block/lib/Drupal/block/Tests/Views/DisplayBlockTest.php b/core/modules/block/lib/Drupal/block/Tests/Views/DisplayBlockTest.php index e0a4d8c..8695a46 100644 --- a/core/modules/block/lib/Drupal/block/Tests/Views/DisplayBlockTest.php +++ b/core/modules/block/lib/Drupal/block/Tests/Views/DisplayBlockTest.php @@ -152,7 +152,7 @@ public function testViewsBlockForm() { * Tests the contextual links on a Views block. */ public function testBlockContextualLinks() { - $this->drupalLogin($this->drupalCreateUser(array('administer views', 'access contextual links'))); + $this->drupalLogin($this->drupalCreateUser(array('administer views', 'access contextual links', 'administer blocks'))); $block = $this->drupalPlaceBlock('views_block:test_view_block-block_1'); $this->drupalGet('test-page'); @@ -174,7 +174,7 @@ public function testBlockContextualLinks() { )); $this->assertResponse(200); $json = drupal_json_decode($response); - $this->assertIdentical($json[$id], ''); + $this->assertIdentical($json[$id], ''); } } diff --git a/core/modules/contextual/contextual.module b/core/modules/contextual/contextual.module index d7f5cb0..2505e77 100644 --- a/core/modules/contextual/contextual.module +++ b/core/modules/contextual/contextual.module @@ -10,6 +10,7 @@ * * @todo Add an event subscriber to the Ajax system to automatically set the * base page theme for all Ajax requests, and then remove this one off. + * See http://drupal.org/node/1954892. */ function contextual_custom_theme() { if (substr(current_path(), 0, 11) === 'contextual/') { @@ -174,7 +175,7 @@ function contextual_element_info() { * Implements hook_preprocess(). * * @see contextual_pre_render_placeholder() - * @see contextual_page_alter() + * @see contextual_page_build() * @see \Drupal\contextual\ContextualController::render() */ function contextual_preprocess(&$variables, $hook) { @@ -197,8 +198,10 @@ function contextual_preprocess(&$variables, $hook) { $variables['attributes']['class'][] = 'contextual-region'; // Renders a contextual links placeholder unconditionally, thus not breaking - // the render cache. The drupal.contextual-links library's JavaScript must - // only be loaded if the user has the 'access contextual links' permission. + // the render cache. Although the empty placeholder is rendered for all + // users, contextual_page_build() only adds the drupal.contextual-links + // library for users with the 'access contextual links' permission, thus + // preventing unnecessary HTTP requests for users without that permission. $variables['title_suffix']['contextual_links'] = array( '#type' => 'contextual_links_placeholder', '#id' => _contextual_links_to_id($element['#contextual_links']), @@ -294,7 +297,7 @@ function contextual_contextual_links_view_alter(&$element, $items) { } /** - * Serializes #contextual_links property metadata to a "contextual id". + * Serializes #contextual_links property value array to a string. * * Examples: * - node:node:1: @@ -302,16 +305,17 @@ function contextual_contextual_links_view_alter(&$element, $items) { * - menu:admin/structure/menu/manage:tools:|block:admin/structure/block/manage:bartik.tools: * * So, expressed in a pattern: - * ::: + * ::: * - * The (dynamic) path args are joined with slashes. The metadata is encoded as a - * query string + * The (dynamic) path args are joined with slashes. The options are encoded as a + * query string. * * @param array $contextual_links * The $element['#contextual_links'] value for some render element. * * @return string - * A contextual id. + * A serialized representation of a #contextual_links property value array for + * use in a data- attribute. */ function _contextual_links_to_id($contextual_links) { $id = ''; @@ -329,14 +333,12 @@ function _contextual_links_to_id($contextual_links) { } /** - * Serializes a contextual id back to #contextual_links property metadata. - * - * The inverse operation of _contextual_links_to_id(). + * Unserializes the result of _contextual_links_to_id(). * * @see _contextual_links_to_id * * @param string $id - * A contextual id. + * A serialized representation of a #contextual_links property value array. * * @return array * The value for a #contextual_links property. diff --git a/core/modules/contextual/lib/Drupal/contextual/ContextualController.php b/core/modules/contextual/lib/Drupal/contextual/ContextualController.php index b454ea7..aca277d 100644 --- a/core/modules/contextual/lib/Drupal/contextual/ContextualController.php +++ b/core/modules/contextual/lib/Drupal/contextual/ContextualController.php @@ -11,7 +11,6 @@ use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; -use Drupal\Core\Ajax\AjaxResponse; use Drupal\Core\Entity\EntityInterface; /** @@ -22,7 +21,8 @@ class ContextualController extends ContainerAware { /** * Returns the requested rendered contextual links. * - * Given a list of contextual links IDs, render them. + * Given a list of contextual links IDs, render them. Hence this must be + * robust to handle arbitrary input. * * @see contextual_preprocess() * diff --git a/core/modules/menu/lib/Drupal/menu/Tests/MenuTest.php b/core/modules/menu/lib/Drupal/menu/Tests/MenuTest.php index f3a4391..847e4a9 100644 --- a/core/modules/menu/lib/Drupal/menu/Tests/MenuTest.php +++ b/core/modules/menu/lib/Drupal/menu/Tests/MenuTest.php @@ -306,11 +306,30 @@ function testSystemMenuRename() { * Tests the contextual links on a menu block. */ public function testBlockContextualLinks() { - $this->drupalLogin($this->drupalCreateUser(array('administer menu', 'access contextual links'))); + $this->drupalLogin($this->drupalCreateUser(array('administer menu', 'access contextual links', 'administer blocks'))); $this->addMenuLink(); - $this->drupalPlaceBlock('system_menu_block:menu-tools', array('label' => 'Tools', 'module' => 'system')); + $block = $this->drupalPlaceBlock('system_menu_block:menu-tools', array('label' => 'Tools', 'module' => 'system')); $this->drupalGet('test-page'); - $this->assertLinkByHref("admin/structure/menu/manage/tools/edit"); + + $id = 'block:admin/structure/block/manage:' . $block->id() . ':|menu:admin/structure/menu/manage:tools:'; + // @see \Drupal\contextual\Tests\ContextualDynamicContextTest:assertContextualLinkPlaceHolder() + $this->assertRaw('
', format_string('Contextual link placeholder with id @id exists.', array('@id' => $id))); + + // Get server-rendered contextual links. + // @see \Drupal\contextual\Tests\ContextualDynamicContextTest:renderContextualLinks() + $post = urlencode('ids[0]') . '=' . urlencode($id); + $response = $this->curlExec(array( + CURLOPT_URL => url('contextual/render', array('absolute' => TRUE, 'query' => array('destination' => 'test-page'))), + CURLOPT_POST => TRUE, + CURLOPT_POSTFIELDS => $post, + CURLOPT_HTTPHEADER => array( + 'Accept: application/json', + 'Content-Type: application/x-www-form-urlencoded', + ), + )); + $this->assertResponse(200); + $json = drupal_json_decode($response); + $this->assertIdentical($json[$id], ''); } /** diff --git a/core/modules/views_ui/views_ui.module b/core/modules/views_ui/views_ui.module index 467a1a1..0d461be 100644 --- a/core/modules/views_ui/views_ui.module +++ b/core/modules/views_ui/views_ui.module @@ -55,28 +55,16 @@ function views_ui_menu() { // views_ui_menu_local_tasks_alter(). $items['admin/structure/views/view/%'] = array( 'route_name' => 'views_ui.edit', - // @todo Remove this. Route access checking does not work for contextual - // links generation yet. @see http://drupal.org/node/1938960#comment-7201292 - 'access callback' => 'user_access', - 'access arguments' => array('administer views'), ); $items['admin/structure/views/view/%/edit'] = array( 'title' => 'Edit view', 'type' => MENU_DEFAULT_LOCAL_TASK, 'context' => MENU_CONTEXT_PAGE | MENU_CONTEXT_INLINE, - // @todo Remove this. Route access checking does not work for contextual - // links generation yet. @see http://drupal.org/node/1938960#comment-7201292 - 'access callback' => 'user_access', - 'access arguments' => array('administer views'), ); $items['admin/structure/views/view/%/preview/%'] = array( 'route_name' => 'views_ui.preview', 'context' => MENU_CONTEXT_PAGE | MENU_CONTEXT_INLINE, 'type' => MENU_VISIBLE_IN_BREADCRUMB, - // @todo Remove this. Route access checking does not work for contextual - // links generation yet. @see http://drupal.org/node/1938960#comment-7201292 - 'access callback' => 'user_access', - 'access arguments' => array('administer views'), ); // Additional pages for acting on a View.