From b6b8e0f4c8c153171a74c5919723e0f9770ce509 Mon Sep 17 00:00:00 2001 From: Lee Rowlands Date: Thu, 15 Aug 2019 10:58:29 +1000 Subject: [PATCH] Issue #2820347 by acbramley, rsmylski, Lendude, jibran, Dinesh18, xaqrox, xjm, fenstrat, esolitos, maximpodorov, tameeshb, plach, catch, alexpott, gambry, larowlan: Exposed filter reset redirects user to 404 page on AJAX view when placed as a block --- .../src/Controller/ViewAjaxController.php | 2 +- .../views/src/Form/ViewsExposedForm.php | 39 +++++++- .../views.view.test_block_exposed_ajax.yml | 80 ++++++++++++++++ ...view.test_block_exposed_ajax_with_page.yml | 96 +++++++++++++++++++ .../BlockExposedFilterAJAXTest.php | 91 ++++++++++++++++++ core/modules/views/views.module | 2 +- .../FunctionalJavascriptTests/JSWebAssert.php | 26 +++++ 7 files changed, 331 insertions(+), 5 deletions(-) create mode 100644 core/modules/views/tests/modules/views_test_config/test_views/views.view.test_block_exposed_ajax.yml create mode 100644 core/modules/views/tests/modules/views_test_config/test_views/views.view.test_block_exposed_ajax_with_page.yml create mode 100644 core/modules/views/tests/src/FunctionalJavascript/BlockExposedFilterAJAXTest.php diff --git a/core/modules/views/src/Controller/ViewAjaxController.php b/core/modules/views/src/Controller/ViewAjaxController.php index 1bc6e3e367c..8b7d115fa5d 100644 --- a/core/modules/views/src/Controller/ViewAjaxController.php +++ b/core/modules/views/src/Controller/ViewAjaxController.php @@ -160,7 +160,7 @@ class ViewAjaxController implements ContainerInjectionInterface { $response->setView($view); // Fix the current path for paging. if (!empty($path)) { - $this->currentPath->setPath('/' . $path, $request); + $this->currentPath->setPath('/' . ltrim($path, '/'), $request); } // Add all POST data, because AJAX is always a post and many things, diff --git a/core/modules/views/src/Form/ViewsExposedForm.php b/core/modules/views/src/Form/ViewsExposedForm.php index e9d5fa4f410..7e92378ae3f 100644 --- a/core/modules/views/src/Form/ViewsExposedForm.php +++ b/core/modules/views/src/Form/ViewsExposedForm.php @@ -5,6 +5,7 @@ namespace Drupal\views\Form; use Drupal\Component\Utility\Html; use Drupal\Core\Form\FormBase; use Drupal\Core\Form\FormStateInterface; +use Drupal\Core\Path\CurrentPathStack; use Drupal\Core\Render\Element\Checkboxes; use Drupal\Core\Url; use Drupal\views\ExposedFormCache; @@ -24,21 +25,39 @@ class ViewsExposedForm extends FormBase { */ protected $exposedFormCache; + + /** + * The current path stack. + * + * @var \Drupal\Core\Path\CurrentPathStack + */ + protected $currentPathStack; + /** * Constructs a new ViewsExposedForm * * @param \Drupal\views\ExposedFormCache $exposed_form_cache * The exposed form cache. + * @param \Drupal\Core\Path\CurrentPathStack $current_path_stack + * The current path stack. */ - public function __construct(ExposedFormCache $exposed_form_cache) { + public function __construct(ExposedFormCache $exposed_form_cache, CurrentPathStack $current_path_stack = NULL) { $this->exposedFormCache = $exposed_form_cache; + if ($current_path_stack === NULL) { + @trigger_error('The path.current service must be passed to ViewsExposedForm::__construct(), it is required before Drupal 9.0.0. See https://www.drupal.org/node/3066604', E_USER_DEPRECATED); + $current_path_stack = \Drupal::service('path.current'); + } + $this->currentPathStack = $current_path_stack; } /** * {@inheritdoc} */ public static function create(ContainerInterface $container) { - return new static($container->get('views.exposed_form_cache')); + return new static( + $container->get('views.exposed_form_cache'), + $container->get('path.current') + ); } /** @@ -113,7 +132,21 @@ class ViewsExposedForm extends FormBase { '#id' => Html::getUniqueId('edit-submit-' . $view->storage->id()), ]; - $form['#action'] = $view->hasUrl() ? $view->getUrl()->toString() : Url::fromRoute('')->toString(); + if (!$view->hasUrl()) { + // On any non views.ajax route, use the current route for the form action. + if ($this->getRouteMatch()->getRouteName() !== 'views.ajax') { + $form_action = Url::fromRoute('')->toString(); + } + else { + // On the views.ajax route, set the action to the page we were on. + $form_action = Url::fromUserInput($this->currentPathStack->getPath())->toString(); + } + } + else { + $form_action = $view->getUrl()->toString(); + } + + $form['#action'] = $form_action; $form['#theme'] = $view->buildThemeFunctions('views_exposed_form'); $form['#id'] = Html::cleanCssIdentifier('views_exposed_form-' . $view->storage->id() . '-' . $display['id']); diff --git a/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_block_exposed_ajax.yml b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_block_exposed_ajax.yml new file mode 100644 index 00000000000..0c3544621fd --- /dev/null +++ b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_block_exposed_ajax.yml @@ -0,0 +1,80 @@ +langcode: en +status: true +dependencies: + config: + - core.entity_view_mode.node.teaser + module: + - node +id: test_block_exposed_ajax +label: '' +module: views +description: '' +tag: '' +base_table: node_field_data +base_field: nid +core: '8' +display: + default: + display_options: + access: + type: none + cache: + type: tag + exposed_form: + options: + submit_button: Apply + reset_button: true + type: basic + filters: + type: + expose: + identifier: type + label: 'Content: Type' + operator_id: type_op + reduce: false + exposed: true + field: type + id: type + table: node_field_data + plugin_id: in_operator + entity_type: node + entity_field: type + pager: + type: full + query: + options: + query_comment: '' + type: views_query + style: + type: default + row: + type: 'entity:node' + display_extenders: { } + use_ajax: true + display_plugin: default + display_title: Master + id: default + position: 0 + cache_metadata: + max-age: -1 + contexts: + - 'languages:language_interface' + - url + - url.query_args + - 'user.node_grants:view' + tags: { } + block_1: + display_plugin: block + id: block_1 + display_title: Block + position: 2 + display_options: + display_extenders: { } + cache_metadata: + max-age: -1 + contexts: + - 'languages:language_interface' + - url + - url.query_args + - 'user.node_grants:view' + tags: { } diff --git a/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_block_exposed_ajax_with_page.yml b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_block_exposed_ajax_with_page.yml new file mode 100644 index 00000000000..3dff996b9ec --- /dev/null +++ b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_block_exposed_ajax_with_page.yml @@ -0,0 +1,96 @@ +langcode: en +status: true +dependencies: + config: + - core.entity_view_mode.node.teaser + module: + - node +id: test_block_exposed_ajax_with_page +label: '' +module: views +description: '' +tag: '' +base_table: node_field_data +base_field: nid +core: '8' +display: + default: + display_options: + access: + type: none + cache: + type: tag + exposed_form: + options: + submit_button: Apply + reset_button: true + type: basic + filters: + type: + expose: + identifier: type + label: 'Content: Type' + operator_id: type_op + reduce: false + exposed: true + field: type + id: type + table: node_field_data + plugin_id: in_operator + entity_type: node + entity_field: type + pager: + type: full + query: + options: + query_comment: '' + type: views_query + style: + type: default + row: + type: 'entity:node' + display_extenders: { } + use_ajax: true + display_plugin: default + display_title: Master + id: default + position: 0 + cache_metadata: + max-age: -1 + contexts: + - 'languages:language_interface' + - url + - url.query_args + - 'user.node_grants:view' + tags: { } + block_1: + display_plugin: block + id: block_1 + display_title: Block + position: 2 + display_options: + display_extenders: { } + cache_metadata: + max-age: -1 + contexts: + - 'languages:language_interface' + - url + - url.query_args + - 'user.node_grants:view' + tags: { } + page_1: + display_plugin: page + id: page_1 + display_title: Page + position: 2 + display_options: + display_extenders: { } + path: some-path + cache_metadata: + max-age: -1 + contexts: + - 'languages:language_interface' + - url + - url.query_args + - 'user.node_grants:view' + tags: { } diff --git a/core/modules/views/tests/src/FunctionalJavascript/BlockExposedFilterAJAXTest.php b/core/modules/views/tests/src/FunctionalJavascript/BlockExposedFilterAJAXTest.php new file mode 100644 index 00000000000..c483ba0331a --- /dev/null +++ b/core/modules/views/tests/src/FunctionalJavascript/BlockExposedFilterAJAXTest.php @@ -0,0 +1,91 @@ +createContentType(['type' => 'page']); + $this->createContentType(['type' => 'article']); + $this->createNode(['title' => 'Page A']); + $this->createNode(['title' => 'Page B']); + $this->createNode(['title' => 'Article A', 'type' => 'article']); + + $this->drupalLogin($this->drupalCreateUser([ + 'access content', + ])); + } + + /** + * Tests if exposed filtering and reset works with a views block and ajax. + */ + public function testExposedFilteringAndReset() { + $node = $this->createNode(); + $block = $this->drupalPlaceBlock('views_block:test_block_exposed_ajax-block_1'); + $this->drupalGet($node->toUrl()); + + $page = $this->getSession()->getPage(); + + // Ensure that the Content we're testing for is present. + $html = $page->getHtml(); + $this->assertContains('Page A', $html); + $this->assertContains('Page B', $html); + $this->assertContains('Article A', $html); + + // Filter by page type. + $this->submitForm(['type' => 'page'], t('Apply')); + $this->assertSession()->waitForElementRemoved('xpath', '//*[text()="Article A"]'); + + // Verify that only the page nodes are present. + $html = $page->getHtml(); + $this->assertContains('Page A', $html); + $this->assertContains('Page B', $html); + $this->assertNotContains('Article A', $html); + + // Reset the form. + $this->submitForm([], t('Reset')); + // Assert we are still on the node page. + $html = $page->getHtml(); + // Repeat the original tests. + $this->assertContains('Page A', $html); + $this->assertContains('Page B', $html); + $this->assertContains('Article A', $html); + $this->assertSession()->addressEquals('node/' . $node->id()); + + $block->delete(); + // Do the same test with a block that has a page display to test the user + // is redirected to the page display. + $this->drupalPlaceBlock('views_block:test_block_exposed_ajax_with_page-block_1'); + $this->drupalGet($node->toUrl()); + $this->submitForm(['type' => 'page'], t('Apply')); + $this->assertSession()->waitForElementRemoved('xpath', '//*[text()="Article A"]'); + $this->submitForm([], t('Reset')); + $this->assertSession()->addressEquals('some-path'); + } + +} diff --git a/core/modules/views/views.module b/core/modules/views/views.module index f46a2604fbd..744c966525c 100644 --- a/core/modules/views/views.module +++ b/core/modules/views/views.module @@ -59,7 +59,7 @@ function views_views_pre_render($view) { 'view_name' => $view->storage->id(), 'view_display_id' => $view->current_display, 'view_args' => Html::escape(implode('/', $view->args)), - 'view_path' => Html::escape(Url::fromRoute('')->toString()), + 'view_path' => Html::escape(\Drupal::service('path.current')->getPath()), 'view_base_path' => $view->getPath(), 'view_dom_id' => $view->dom_id, // To fit multiple views on a page, the programmer may have diff --git a/core/tests/Drupal/FunctionalJavascriptTests/JSWebAssert.php b/core/tests/Drupal/FunctionalJavascriptTests/JSWebAssert.php index e72bda06e26..d52cf431811 100644 --- a/core/tests/Drupal/FunctionalJavascriptTests/JSWebAssert.php +++ b/core/tests/Drupal/FunctionalJavascriptTests/JSWebAssert.php @@ -71,6 +71,32 @@ JS; return $result; } + /** + * Looks for the specified selector and returns TRUE when it is unavailable. + * + * @param string $selector + * The selector engine name. See ElementInterface::findAll() for the + * supported selectors. + * @param string|array $locator + * The selector locator. + * @param int $timeout + * (Optional) Timeout in milliseconds, defaults to 10000. + * + * @return bool + * TRUE if not found, FALSE if found. + * + * @see \Behat\Mink\Element\ElementInterface::findAll() + */ + public function waitForElementRemoved($selector, $locator, $timeout = 10000) { + $page = $this->session->getPage(); + + $result = $page->waitFor($timeout / 1000, function () use ($page, $selector, $locator) { + return !$page->find($selector, $locator); + }); + + return $result; + } + /** * Waits for the specified selector and returns it when available and visible. *