HTTP response render should preserve query parameters from the original request [#2709455]

I have an http_response variant that redirects the taxonomy term view page to a view, such that

taxonomy/term/1 => videos/tags/kittens

I need to be able to pass query parameters through to my views display, such that

taxonomy/term/1?keywords=whiskers => videos/tags/kittens?keywords=whiskers

Right now the query string handling in page_manager_http_response_render() respects any query parameters that are specified in $handler['conf'], but it drops query parameters from the current request. To me it would make sense to pass along any query parameters from the current request as part of the redirect.

Comment	File	Size	Author
#5	ctools-2709455-5.patch	1.01 KB	Snehal Brahmbhatt
#2	2709455-2.http_response_query_params.patch	997 bytes	ksenzee

Comments

Comment #1

19 April 2016 at 18:23

ksenzee created an issue. See original summary.

Comment #2

ksenzee

she/her

commented 19 April 2016 at 18:27

Status:

Active

» Needs review

Status	File	Size
new	2709455-2.http_response_query_params.patch	997 bytes

Comment #3

merlinofchaos commented 19 April 2016 at 18:59

IMO, on a redirect, the preservation of query string should be optional and not enabled by default. However, the use case for preserving the query string makes sense.

Comment #4

chris matthews commented 22 January 2019 at 02:04

Status:	Needs review	» Needs work
Issue tags:		+Needs reroll

The 3 year old patch to http_response.inc does not apply to the latest ctools 7.x-1.x-dev.

Checking patch page_manager/plugins/task_handlers/http_response.inc...
error: while searching for:
  if ($info['response code'] == 301) {
    $path = ctools_context_keyword_substitute($handler->conf['destination'], array(), $contexts);
    $url = parse_url($path);
    if (isset($url['query'])) {
      $path = strtr($path, array('?' . $url['query'] => ''));
      $info['query'] = drupal_get_query_array($url['query']);
    }
    if (isset($url['fragment'])) {
      $path = strtr($path, array('#' . $url['fragment'] => ''));

error: patch failed: page_manager/plugins/task_handlers/http_response.inc:316
error: page_manager/plugins/task_handlers/http_response.inc: patch does not apply

Comment #5

Snehal Brahmbhatt commented 25 January 2019 at 10:26

Status:

Needs work

» Needs review

Status	File	Size
new	ctools-2709455-5.patch	1.01 KB

@Chris2, Find the attached that rerolled for ctools 7.x-1.x-dev as per the added requirement in #4.

Thanks!..

Comment #6

rivimey

she/her

commented 12 February 2019 at 00:11

Review-wise, the thought seems good but:

I'm with #3 - given this is a change to existing behaviour, it should be disabled by default and enable-able somehow.
Use of the array += operator always causes concern. Are you 100% sure it should not be array_merge() in this case

Finally, some tests should be added to check that both the query is, and is not, passed through.