summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAlex Pott2015-09-03 13:28:12 (GMT)
committerAlex Pott2015-09-03 13:28:12 (GMT)
commitfeb0fc2c4ddd3c4eb5731c916a51ba72403a3b3c (patch)
treeed080aa8fdc079462e680d061724618af401f23f
parenta17303166447173af60d8d4b3fbfee37f098a692 (diff)
Issue #2504139 by borisson_, Wim Leers, plach, prashantgoel, Fabianx, dawehner, David_Rothstein, effulgentsia: Blocks containing a form include the form action in the cache, so they always submit to the first URL the form was viewed at
-rw-r--r--core/lib/Drupal/Core/Form/FormBuilder.php26
-rw-r--r--core/lib/Drupal/Core/Render/Renderer.php2
-rw-r--r--core/modules/block/src/Tests/BlockFormInBlockTest.php76
-rw-r--r--core/modules/block/src/Tests/Views/DisplayBlockTest.php8
-rw-r--r--core/modules/block/tests/modules/block_test/block_test.routing.yml7
-rw-r--r--core/modules/block/tests/modules/block_test/src/Controller/TestMultipleFormController.php43
-rw-r--r--core/modules/block/tests/modules/block_test/src/Form/FavoriteAnimalTestForm.php46
-rw-r--r--core/modules/block/tests/modules/block_test/src/Form/TestForm.php55
-rw-r--r--core/modules/block/tests/modules/block_test/src/Plugin/Block/TestFormBlock.php29
-rw-r--r--core/modules/block_content/src/Tests/BlockContentTranslationUITest.php3
-rw-r--r--core/modules/content_translation/src/Tests/ContentTestTranslationUITest.php3
-rw-r--r--core/modules/menu_link_content/src/Tests/MenuLinkContentTranslationUITest.php2
-rw-r--r--core/modules/node/src/Tests/NodeBlockFunctionalTest.php2
-rw-r--r--core/modules/shortcut/src/Tests/ShortcutTranslationUITest.php2
-rw-r--r--core/modules/system/src/Tests/Common/RenderWebTest.php8
-rw-r--r--core/modules/system/tests/modules/common_test/common_test.routing.yml1
16 files changed, 298 insertions, 15 deletions
diff --git a/core/lib/Drupal/Core/Form/FormBuilder.php b/core/lib/Drupal/Core/Form/FormBuilder.php
index b341064..adf4c86 100644
--- a/core/lib/Drupal/Core/Form/FormBuilder.php
+++ b/core/lib/Drupal/Core/Form/FormBuilder.php
@@ -638,6 +638,20 @@ class FormBuilder implements FormBuilderInterface, FormValidatorInterface, FormS
}
/**
+ * #lazy_builder callback; renders a form action URL.
+ *
+ * @return array
+ * A renderable array representing the form action.
+ */
+ public function renderPlaceholderFormAction() {
+ return [
+ '#type' => 'markup',
+ '#markup' => $this->buildFormAction(),
+ '#cache' => ['contexts' => ['url.path', 'url.query_args']],
+ ];
+ }
+
+ /**
* {@inheritdoc}
*/
public function prepareForm($form_id, &$form, FormStateInterface &$form_state) {
@@ -647,7 +661,17 @@ class FormBuilder implements FormBuilderInterface, FormValidatorInterface, FormS
// Only update the action if it is not already set.
if (!isset($form['#action'])) {
- $form['#action'] = $this->buildFormAction();
+ // Instead of setting an actual action URL, we set the placeholder, which
+ // will be replaced at the very last moment. This ensures forms with
+ // dynamically generated action URLs don't have poor cacheability.
+ // Use the proper API to generate the placeholder, when we have one. See
+ // https://www.drupal.org/node/2562341.
+ $placeholder = 'form_action_' . hash('crc32b', __METHOD__);
+
+ $form['#attached']['placeholders'][$placeholder] = [
+ '#lazy_builder' => ['form_builder:renderPlaceholderFormAction', []],
+ ];
+ $form['#action'] = $placeholder;
}
// Fix the form method, if it is 'get' in $form_state, but not in $form.
diff --git a/core/lib/Drupal/Core/Render/Renderer.php b/core/lib/Drupal/Core/Render/Renderer.php
index bb8f1ff..48b3bcd 100644
--- a/core/lib/Drupal/Core/Render/Renderer.php
+++ b/core/lib/Drupal/Core/Render/Renderer.php
@@ -179,7 +179,7 @@ class Renderer implements RendererInterface {
// Replace the placeholder with its rendered markup, and merge its
// bubbleable metadata with the main elements'.
- $elements['#markup'] = str_replace($placeholder, $markup, $elements['#markup']);
+ $elements['#markup'] = SafeString::create(str_replace($placeholder, $markup, $elements['#markup']));
$elements = $this->mergeBubbleableMetadata($elements, $placeholder_elements);
// Remove the placeholder that we've just rendered.
diff --git a/core/modules/block/src/Tests/BlockFormInBlockTest.php b/core/modules/block/src/Tests/BlockFormInBlockTest.php
new file mode 100644
index 0000000..32eddfa
--- /dev/null
+++ b/core/modules/block/src/Tests/BlockFormInBlockTest.php
@@ -0,0 +1,76 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\block\Tests\BlockFormInBlockTest.
+ */
+
+namespace Drupal\block\Tests;
+
+use Drupal\simpletest\WebTestBase;
+
+/**
+ * Tests form in block caching.
+ *
+ * @group block
+ */
+class BlockFormInBlockTest extends WebTestBase {
+
+ /**
+ * Modules to install.
+ *
+ * @var array
+ */
+ public static $modules = ['block', 'block_test', 'test_page_test'];
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function setUp() {
+ parent::setUp();
+
+ // Enable our test block.
+ $this->drupalPlaceBlock('test_form_in_block');
+ }
+
+ /**
+ * Test to see if form in block's redirect isn't cached.
+ */
+ function testCachePerPage() {
+ $form_values = ['email' => 'test@example.com'];
+
+ // Go to "test-page" and test if the block is enabled.
+ $this->drupalGet('test-page');
+ $this->assertResponse(200);
+ $this->assertText('Your .com email address.', 'form found');
+
+ // Make sure that we're currently still on /test-page after submitting the
+ // form.
+ $this->drupalPostForm(NULL, $form_values, t('Submit'));
+ $this->assertUrl('test-page');
+ $this->assertText(t('Your email address is @email', ['@email' => 'test@example.com']));
+
+ // Go to a different page and see if the block is enabled there as well.
+ $this->drupalGet('test-render-title');
+ $this->assertResponse(200);
+ $this->assertText('Your .com email address.', 'form found');
+
+ // Make sure that submitting the form didn't redirect us to the first page
+ // we submitted the form from after submitting the form from
+ // /test-render-title.
+ $this->drupalPostForm(NULL, $form_values, t('Submit'));
+ $this->assertUrl('test-render-title');
+ $this->assertText(t('Your email address is @email', ['@email' => 'test@example.com']));
+ }
+
+ /**
+ * Test the actual placeholders
+ */
+ public function testPlaceholders() {
+ $this->drupalGet('test-multiple-forms');
+
+ $placeholder = 'form_action_' . hash('crc32b', 'Drupal\Core\Form\FormBuilder::prepareForm');
+ $this->assertText('Form action: ' . $placeholder, 'placeholder found.');
+ }
+
+}
diff --git a/core/modules/block/src/Tests/Views/DisplayBlockTest.php b/core/modules/block/src/Tests/Views/DisplayBlockTest.php
index 0beb539..9b85831 100644
--- a/core/modules/block/src/Tests/Views/DisplayBlockTest.php
+++ b/core/modules/block/src/Tests/Views/DisplayBlockTest.php
@@ -290,7 +290,7 @@ class DisplayBlockTest extends ViewTestBase {
// Ensure that the view cachability metadata is propagated even, for an
// empty block.
$this->assertCacheTags(array_merge($block->getCacheTags(), ['block_view', 'config:block_list', 'config:system.site', 'config:views.view.test_view_block' ,'rendered']));
- $this->assertCacheContexts(['url.query_args:_wrapper_format', 'user.roles:authenticated']);
+ $this->assertCacheContexts(['url.path', 'url.query_args', 'user.roles:authenticated']);
// Add a header displayed on empty result.
$display = &$view->getDisplay('block_1');
@@ -308,7 +308,7 @@ class DisplayBlockTest extends ViewTestBase {
$this->drupalGet('');
$this->assertEqual(1, count($this->xpath('//div[contains(@class, "block-views-blocktest-view-block-block-1")]')));
$this->assertCacheTags(array_merge($block->getCacheTags(), ['block_view', 'config:block_list', 'config:system.site', 'config:views.view.test_view_block' ,'rendered']));
- $this->assertCacheContexts(['url.query_args:_wrapper_format', 'user.roles:authenticated']);
+ $this->assertCacheContexts(['url.path', 'url.query_args', 'user.roles:authenticated']);
// Hide the header on empty results.
$display = &$view->getDisplay('block_1');
@@ -326,7 +326,7 @@ class DisplayBlockTest extends ViewTestBase {
$this->drupalGet('');
$this->assertEqual(0, count($this->xpath('//div[contains(@class, "block-views-blocktest-view-block-block-1")]')));
$this->assertCacheTags(array_merge($block->getCacheTags(), ['block_view', 'config:block_list', 'config:system.site', 'config:views.view.test_view_block' ,'rendered']));
- $this->assertCacheContexts(['url.query_args:_wrapper_format', 'user.roles:authenticated']);
+ $this->assertCacheContexts(['url.path', 'url.query_args', 'user.roles:authenticated']);
// Add an empty text.
$display = &$view->getDisplay('block_1');
@@ -343,7 +343,7 @@ class DisplayBlockTest extends ViewTestBase {
$this->drupalGet('');
$this->assertEqual(1, count($this->xpath('//div[contains(@class, "block-views-blocktest-view-block-block-1")]')));
$this->assertCacheTags(array_merge($block->getCacheTags(), ['block_view', 'config:block_list', 'config:system.site', 'config:views.view.test_view_block' ,'rendered']));
- $this->assertCacheContexts(['url.query_args:_wrapper_format', 'user.roles:authenticated']);
+ $this->assertCacheContexts(['url.path', 'url.query_args', 'user.roles:authenticated']);
}
/**
diff --git a/core/modules/block/tests/modules/block_test/block_test.routing.yml b/core/modules/block/tests/modules/block_test/block_test.routing.yml
new file mode 100644
index 0000000..58af6fc
--- /dev/null
+++ b/core/modules/block/tests/modules/block_test/block_test.routing.yml
@@ -0,0 +1,7 @@
+block_test.test_multipleforms:
+ path: '/test-multiple-forms'
+ defaults:
+ _controller: '\Drupal\block_test\Controller\TestMultipleFormController::testMultipleForms'
+ _title: 'Multiple forms'
+ requirements:
+ _access: 'TRUE'
diff --git a/core/modules/block/tests/modules/block_test/src/Controller/TestMultipleFormController.php b/core/modules/block/tests/modules/block_test/src/Controller/TestMultipleFormController.php
new file mode 100644
index 0000000..d786290
--- /dev/null
+++ b/core/modules/block/tests/modules/block_test/src/Controller/TestMultipleFormController.php
@@ -0,0 +1,43 @@
+<?php
+/**
+ * @file
+ * Contains \Drupal\block_test\Controller\TestMultipleFormController.
+ */
+
+namespace Drupal\block_test\Controller;
+
+use Drupal\Core\Controller\ControllerBase;
+use Drupal\Core\Form\FormState;
+
+/**
+ * Controller for block_test module
+ */
+class TestMultipleFormController extends ControllerBase {
+
+ public function testMultipleForms() {
+ $form_state = new FormState();
+ $build = [
+ 'form1' => $this->formBuilder()->buildForm('\Drupal\block_test\Form\TestForm', $form_state),
+ 'form2' => $this->formBuilder()->buildForm('\Drupal\block_test\Form\FavoriteAnimalTestForm', $form_state),
+ ];
+
+ // Output all attached placeholders trough drupal_set_message(), so we can
+ // see if there's only one in the tests.
+ $post_render_callable = function ($elements) {
+ $matches = [];
+ preg_match_all('<form\s(.*?)action="(.*?)"(.*)>', $elements, $matches);
+
+ $action_values = $matches[2];
+
+ foreach ($action_values as $action_value) {
+ drupal_set_message('Form action: ' . $action_value);
+ }
+ return $elements;
+ };
+
+ $build['#post_render'] = [$post_render_callable];
+
+ return $build;
+ }
+
+}
diff --git a/core/modules/block/tests/modules/block_test/src/Form/FavoriteAnimalTestForm.php b/core/modules/block/tests/modules/block_test/src/Form/FavoriteAnimalTestForm.php
new file mode 100644
index 0000000..2e40675
--- /dev/null
+++ b/core/modules/block/tests/modules/block_test/src/Form/FavoriteAnimalTestForm.php
@@ -0,0 +1,46 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\block_test\Form\FavoriteAnimalTestForm.
+ */
+
+namespace Drupal\block_test\Form;
+
+use Drupal\Core\Form\FormBase;
+use Drupal\Core\Form\FormStateInterface;
+
+class FavoriteAnimalTestForm extends FormBase {
+
+ /**
+ * {@inheritdoc}
+ */
+ public function getFormId() {
+ return 'block_test_form_favorite_animal_test';
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function buildForm(array $form, FormStateInterface $form_state) {
+ $form['favorite_animal'] = [
+ '#type' => 'textfield',
+ '#title' => $this->t('Your favorite animal.')
+ ];
+
+ $form['submit_animal'] = [
+ '#type' => 'submit',
+ '#value' => $this->t('Submit your chosen animal'),
+ ];
+
+ return $form;
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function submitForm(array &$form, FormStateInterface $form_state) {
+ drupal_set_message($this->t('Your favorite animal is: @favorite_animal', ['@favorite_animal' => $form['favorite_animal']['#value']]));
+ }
+
+}
diff --git a/core/modules/block/tests/modules/block_test/src/Form/TestForm.php b/core/modules/block/tests/modules/block_test/src/Form/TestForm.php
new file mode 100644
index 0000000..c0a0694
--- /dev/null
+++ b/core/modules/block/tests/modules/block_test/src/Form/TestForm.php
@@ -0,0 +1,55 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\block_test\Form\TestForm.
+ */
+
+namespace Drupal\block_test\Form;
+
+use Drupal\Core\Form\FormBase;
+use Drupal\Core\Form\FormStateInterface;
+
+class TestForm extends FormBase {
+
+ /**
+ * {@inheritdoc}
+ */
+ public function getFormId() {
+ return 'block_test_form_test';
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function buildForm(array $form, FormStateInterface $form_state) {
+ $form['email'] = [
+ '#type' => 'email',
+ '#title' => $this->t('Your .com email address.')
+ ];
+
+ $form['show'] = [
+ '#type' => 'submit',
+ '#value' => $this->t('Submit'),
+ ];
+
+ return $form;
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function validateForm(array &$form, FormStateInterface $form_state) {
+ if (strpos($form_state->getValue('email'), '.com') === FALSE) {
+ $form_state->setErrorByName('email', $this->t('This is not a .com email address.'));
+ }
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function submitForm(array &$form, FormStateInterface $form_state) {
+ drupal_set_message($this->t('Your email address is @email', ['@email' => $form['email']['#value']]));
+ }
+
+}
diff --git a/core/modules/block/tests/modules/block_test/src/Plugin/Block/TestFormBlock.php b/core/modules/block/tests/modules/block_test/src/Plugin/Block/TestFormBlock.php
new file mode 100644
index 0000000..feeb9df
--- /dev/null
+++ b/core/modules/block/tests/modules/block_test/src/Plugin/Block/TestFormBlock.php
@@ -0,0 +1,29 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\block_test\Plugin\Block\TestFormBlock.
+ */
+
+namespace Drupal\block_test\Plugin\Block;
+
+use Drupal\Core\Block\BlockBase;
+
+/**
+ * Provides a block to test caching.
+ *
+ * @Block(
+ * id = "test_form_in_block",
+ * admin_label = @Translation("Test form block caching")
+ * )
+ */
+class TestFormBlock extends BlockBase {
+
+ /**
+ * {@inheritdoc}
+ */
+ public function build() {
+ return \Drupal::formBuilder()->getForm('Drupal\block_test\Form\TestForm');
+ }
+
+}
diff --git a/core/modules/block_content/src/Tests/BlockContentTranslationUITest.php b/core/modules/block_content/src/Tests/BlockContentTranslationUITest.php
index 4ce1565..964f0f9 100644
--- a/core/modules/block_content/src/Tests/BlockContentTranslationUITest.php
+++ b/core/modules/block_content/src/Tests/BlockContentTranslationUITest.php
@@ -36,7 +36,8 @@ class BlockContentTranslationUITest extends ContentTranslationUITestBase {
protected $defaultCacheContexts = [
'languages:language_interface',
'theme',
- 'url.query_args:_wrapper_format',
+ 'url.path',
+ 'url.query_args',
'user.permissions',
'user.roles:authenticated',
];
diff --git a/core/modules/content_translation/src/Tests/ContentTestTranslationUITest.php b/core/modules/content_translation/src/Tests/ContentTestTranslationUITest.php
index 43fd779..08b1d4a 100644
--- a/core/modules/content_translation/src/Tests/ContentTestTranslationUITest.php
+++ b/core/modules/content_translation/src/Tests/ContentTestTranslationUITest.php
@@ -32,7 +32,8 @@ class ContentTestTranslationUITest extends ContentTranslationUITestBase {
protected $defaultCacheContexts = [
'languages:language_interface',
'theme',
- 'url.query_args:_wrapper_format',
+ 'url.path',
+ 'url.query_args',
'user.permissions',
'user.roles:authenticated',
];
diff --git a/core/modules/menu_link_content/src/Tests/MenuLinkContentTranslationUITest.php b/core/modules/menu_link_content/src/Tests/MenuLinkContentTranslationUITest.php
index f72cb29..1404384 100644
--- a/core/modules/menu_link_content/src/Tests/MenuLinkContentTranslationUITest.php
+++ b/core/modules/menu_link_content/src/Tests/MenuLinkContentTranslationUITest.php
@@ -20,7 +20,7 @@ class MenuLinkContentTranslationUITest extends ContentTranslationUITestBase {
/**
* {inheritdoc}
*/
- protected $defaultCacheContexts = ['languages:language_interface', 'theme', 'url.query_args:_wrapper_format', 'user.permissions', 'user.roles:authenticated'];
+ protected $defaultCacheContexts = ['languages:language_interface', 'theme', 'url.path', 'url.query_args', 'user.permissions', 'user.roles:authenticated'];
/**
* Modules to enable.
diff --git a/core/modules/node/src/Tests/NodeBlockFunctionalTest.php b/core/modules/node/src/Tests/NodeBlockFunctionalTest.php
index 6f319f5..b6855ba 100644
--- a/core/modules/node/src/Tests/NodeBlockFunctionalTest.php
+++ b/core/modules/node/src/Tests/NodeBlockFunctionalTest.php
@@ -148,7 +148,7 @@ class NodeBlockFunctionalTest extends NodeTestBase {
$this->assertCacheContexts(['languages:language_content', 'languages:language_interface', 'theme', 'url.query_args:' . MainContentViewSubscriber::WRAPPER_FORMAT, 'user', 'route']);
$this->drupalGet('node/add/article');
$this->assertText($label, 'Block was displayed on the node/add/article page.');
- $this->assertCacheContexts(['languages:language_content', 'languages:language_interface', 'theme', 'url.query_args:' . MainContentViewSubscriber::WRAPPER_FORMAT, 'user', 'route']);
+ $this->assertCacheContexts(['languages:language_content', 'languages:language_interface', 'theme', 'url.path', 'url.query_args', 'user', 'route']);
$this->drupalGet('node/' . $node1->id());
$this->assertText($label, 'Block was displayed on the node/N when node is of type article.');
$this->assertCacheContexts(['languages:language_content', 'languages:language_interface', 'theme', 'url.query_args:' . MainContentViewSubscriber::WRAPPER_FORMAT, 'user', 'route', 'timezone']);
diff --git a/core/modules/shortcut/src/Tests/ShortcutTranslationUITest.php b/core/modules/shortcut/src/Tests/ShortcutTranslationUITest.php
index 5fa63d8..e42262f 100644
--- a/core/modules/shortcut/src/Tests/ShortcutTranslationUITest.php
+++ b/core/modules/shortcut/src/Tests/ShortcutTranslationUITest.php
@@ -21,7 +21,7 @@ class ShortcutTranslationUITest extends ContentTranslationUITestBase {
/**
* {inheritdoc}
*/
- protected $defaultCacheContexts = ['languages:language_interface', 'theme', 'user', 'url.query_args:_wrapper_format', 'url.site'];
+ protected $defaultCacheContexts = ['languages:language_interface', 'theme', 'user', 'url.path', 'url.query_args', 'url.site'];
/**
* Modules to enable.
diff --git a/core/modules/system/src/Tests/Common/RenderWebTest.php b/core/modules/system/src/Tests/Common/RenderWebTest.php
index 5cfe3ab..f6252e9 100644
--- a/core/modules/system/src/Tests/Common/RenderWebTest.php
+++ b/core/modules/system/src/Tests/Common/RenderWebTest.php
@@ -30,17 +30,17 @@ class RenderWebTest extends WebTestBase {
* Asserts the cache context for the wrapper format is always present.
*/
function testWrapperFormatCacheContext() {
- $this->drupalGet('');
+ $this->drupalGet('common-test/type-link-active-class');
$this->assertIdentical(0, strpos($this->getRawContent(), "<!DOCTYPE html>\n<html"));
$this->assertIdentical('text/html; charset=UTF-8', $this->drupalGetHeader('Content-Type'));
- $this->assertTitle('Log in | Drupal');
+ $this->assertTitle('Test active link class | Drupal');
$this->assertCacheContext('url.query_args:' . MainContentViewSubscriber::WRAPPER_FORMAT);
- $this->drupalGet('', ['query' => [MainContentViewSubscriber::WRAPPER_FORMAT => 'json']]);
+ $this->drupalGet('common-test/type-link-active-class', ['query' => [MainContentViewSubscriber::WRAPPER_FORMAT => 'json']]);
$this->assertIdentical('application/json', $this->drupalGetHeader('Content-Type'));
$json = Json::decode($this->getRawContent());
$this->assertEqual(['content', 'title'], array_keys($json));
- $this->assertIdentical('Log in', $json['title']);
+ $this->assertIdentical('Test active link class', $json['title']);
$this->assertCacheContext('url.query_args:' . MainContentViewSubscriber::WRAPPER_FORMAT);
}
diff --git a/core/modules/system/tests/modules/common_test/common_test.routing.yml b/core/modules/system/tests/modules/common_test/common_test.routing.yml
index 92c80be..8062d06 100644
--- a/core/modules/system/tests/modules/common_test/common_test.routing.yml
+++ b/core/modules/system/tests/modules/common_test/common_test.routing.yml
@@ -1,6 +1,7 @@
common_test.l_active_class:
path: '/common-test/type-link-active-class'
defaults:
+ _title: 'Test active link class'
_controller: '\Drupal\common_test\Controller\CommonTestController::typeLinkActiveClass'
requirements:
_access: 'TRUE'