Problem/Motivation

In FieldWebTest we have:

      // @fixme: The actual result is node/123?foo#bar so views has a bug here.
      // $this->assertSubStringExists(decode_entities($result), decode_entities($expected_result));

So basically we are expecting node/123?foo#bar but we currently get node/123?foo=#bar. The problem occurs are we parse the URL when then makes an empty query string parameter into 'param' => ''. So an empty string. in UrlHelper::buildQuery() anything other than NULL is added to the query string with $key . '=' . $value.

It would be nice to uncomment out this code once again.

Proposed resolution

In FieldPluginBase::renderAsLink():

Use the parsed url path, and deal with just the array of parsed query parameters. We currently try to deal with array and the original string by using the raw path value in $path. So overwrite the parsed value from url parsing to the $path variable too.

Uncomment failing test assertion and all *should* be well.

Remaining tasks

Patch

User interface changes

None

API changes

None

CommentFileSizeAuthor
vdc.url-parsing-FieldPluginBase.patch3.41 KBdamiankloip
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented

vdc.url-parsing-FieldPluginBase.patch queued for re-testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

Nice!

+++ b/core/modules/views/src/Plugin/views/field/FieldPluginBase.php
@@ -1364,23 +1368,28 @@ protected function renderAsLink($alter, $text, $tokens) {
-          unset($query[$param]);
+          unset($url['query'][$param]);
+        }
+        // Replace any empty query params from URL parsing with NULL. So the
+        // query will get built correctly with only the param key.
+        // @see \Drupal\Component\Utility\UrlHelper::buildQuery().
+        if ($val === '') {
+          $url['query'][$param] = NULL;
         }

Did we considerd to use array_filter with a callback instead?

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented

Hmm, I thought that originally but sadly we need the key and the value to check the token replacement:

if ($val == '%' . $param) {
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

meh, but well, this is already an improvement. Thank you damian!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed 973fd26 and pushed to 8.0.x. Thanks!

  • alexpott committed 973fd26 on 8.0.x 
    Issue #2278327 by damiankloip: Fixed Url parsing of empty query string...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.