If I go to http://www.example.com/node?foo=bar, the [current-page:url] token should probably return the same.

CommentFileSizeAuthor
#58 1198032-58-interdiff.txt729 bytesjcisio
#58 1198032-58.patch7.13 KBjcisio
#54 1198032-token-paginated-54.patch6.69 KBjcisio
#53 1198032-token-paginated-53.patch5.19 KBjcisio
#45 token-current_page_token_include_query-1198032-45.patch3.47 KBkeopx
#44 token-current_page_token_include_query-1198032-44.patch3.46 KBckng
#43 token-current_page_token_include_query-1198032-43.patch3.63 KBckng
#38 token-current_page_token_include_query-1198032-38.patch3.42 KBAlex Bukach
#36 token-current_page_token_include_query-1198032-36.patch3.46 KBcgmonroe
#33 after.png7.06 KBvakulrai
#33 before.png7.33 KBvakulrai
#30 1198032-30.patch3.41 KBdpagini
#30 1198032-30-testsonly.patch2.33 KBdpagini
#26 1198032-26.patch2.91 KBdpagini
#26 1198032-26-testsonly.patch2.21 KBdpagini
#24 token-query-1198032-24-D8.patch2.71 KBAlex Bukach
#22 token-query-1198032-22-D8.patch2.33 KBAlex Bukach
#20 token-query-1198032-20-D8.patch617 bytesAlex Bukach
#17 token-query_string-1198032-17.patch2.24 KBNWOM
#7 token-query_string-1198032-7.patch1.3 KBtedavis
#4 query_string-1198032-3.patch1.33 KBded
#2 query_string-1198032-2.patch534 bytesded
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ded’s picture

Version: 7.x-1.x-dev » 7.x-1.1

This appears to still be an issue in current version. Has anyone solved this? If not I will look at a patch.

ded’s picture

Version: 7.x-1.1 » 7.x-1.x-dev
Status: Active » Needs review
FileSize
534 bytes

Here is a patch which fixes this issue by adding the query parameters to the $url_options array before the url() call.

Status: Needs review » Needs work

The last submitted patch, query_string-1198032-2.patch, failed testing.

ded’s picture

Here is a revised patch with the tests updated so that it checks the existence of the parameters.

ded’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, query_string-1198032-3.patch, failed testing.

Anonymous’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
1.3 KB

Slight error in the url() structure of the last patch for the query. Should be sorted now.

coderintherye’s picture

What are module maintainers thoughts on this?

The patch looks fine, but I could see how it could be considered a breaking change by some. I did check and if one knows the query parameters that are expected for the page then one can build the url out of existing tokens: [current-page:url]?placement_id=[current-page:query:placement_id]#information

That said, it'd be really nice if the current url included the query parameters by default or if we added a token which included them, without having to build it out like the above.

coderintherye’s picture

Status: Needs review » Reviewed & tested by the community

Actually, I take it back, the syntax of [current-page:url]?query=[current-page:query:query]#anchor does not work, because if query is blank then token just spits out the token text itself.

The patch does indeed work though, so marking this as RTBC. The question remains if it should change the functionality of the existing token or become a new token, but if we go with the form then this patch should go in as is.

m4olivei’s picture

Also needed this, tried the patch, and it's working well as far as I can tell. Being used in a metatag token.

Dave Reid’s picture

My concern about including the current query string with the token is that we should probably ignore the 'page' query string? This token is used extensively with Metatag as the default for the 'Canonical' URL which should always be the same regardless of paged content. This would cause a regression there. We'd need to have a plan to resolve that.

D2ev’s picture

we had the same issue that 'Canonical' URL is ignoring the page query string. Patch #7 fixed it. +1 to RTBC

moxojo’s picture

Stumbled across this while trying to find a fix for canonicalizing paged content. Paged content should receive it's own self referential canonical tag. Google has written about this and said that pointing all pages to the first page is an incorrect use of the canonical tag. rel="next" and rel="prev" is how pagination should be handled. If you are going to canonicalize all paginated pages to a single page then it should be a view all page, not the first page in the series.

I am looking for a solution that will preserve pagination. Bonus if it allows us to select the url parameters to keep or exclude. For instance, if we have example.com/term?page=2&foo=bar&gclid=xxxxxx I would want to preserve the canonical for "page" and possibly for "foo" while excluding gclid which is a tracking param. This would allow us to use the canonical as intended for designating the authoritative url for duplicate content. As pagination changes the content and as other filtering params can drastically change the content we need to have the ability to canonicalize to those params while excluding params that create true duplicate pages.

Cheers!

DamienMcKenna’s picture

Maybe there should be a separate token that includes the full query string?

alexmoreno’s picture

+1 with @damienMcKenna I am in a project where we need the url clean of query strings

NWOM’s picture

Status: Reviewed & tested by the community » Needs work

#7 worked great for the standard [current-page:url] token, however did not work for the [current-page:url:relative] and [current-page:url:absolute] tokens.

When I get a chance, I'll setup a new environment with a dev version of Token to provide a patch. I'm currently using the Token version that is provided by the Panopoly distribution. In the meantime, I added the following manually:

        case 'absolute':
+          $url_options['query'] = drupal_get_query_parameters();
           $replacements[$original] = url($path, $url_options);
          break;
        case 'relative':
+          $url_options['query'] = drupal_get_query_parameters();
          $replacements[$original] = url($path, array('absolute' => FALSE) + $url_options);
          break;
NWOM’s picture

NWOM’s picture

Status: Needs work » Needs review
Alex Bukach’s picture

Version: 7.x-1.x-dev » 8.x-1.x-dev
Assigned: Unassigned » Alex Bukach
Status: Needs review » Active

This is not done in the 8.x as well.

Alex Bukach’s picture

Assigned: Alex Bukach » Unassigned
Status: Active » Needs review
FileSize
617 bytes

Status: Needs review » Needs work

The last submitted patch, 20: token-query-1198032-20-D8.patch, failed testing.

Alex Bukach’s picture

Status: Needs review » Needs work

The last submitted patch, 22: token-query-1198032-22-D8.patch, failed testing.

Alex Bukach’s picture

gaurav.kapoor’s picture

Thanks Alex Bukach. Last patch helped me a lot. Its fit to be RTBC if it is re rolled for latest dev branch.

dpagini’s picture

Attempt to re-roll against current dev branch. Adding a tests-only patch which I expect to fail, and the patch with the expected fix.

I'm not sure I know how to create an interdiff here (was going to try) since the current patch doesn't apply. Please advise if needed.

The last submitted patch, 26: 1198032-26-testsonly.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

dpagini’s picture

@gaurav.kapoor given your last comment, not sure if this should be changed to RTBC? I'll wait and let anyone chime in on this one. I took a stab at it b/c I'm having the problem and will need to pull in this patch. Thanks!

dpagini’s picture

Hmm... I'm not sure this fully encapsulates the description. For me, [current-page:url] still does not contain the query string. Other similar tokens, like the ones updated in the tests ([current-page:url:relative], [current-page:url:absolute]) do contain the querystring, but not simply [current-page:url].

dpagini’s picture

Here's another shot... new tests file, adding the test for [current-page:url] and putting in code to hopefully pass the test.

The last submitted patch, 30: 1198032-30-testsonly.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

jennypanighetti’s picture

How does this token, or patch, work with i18n/translated views? I need a current-page:url tag with pagination query to use for the hreflang metatag. Is that supported here? For example, would the tag be [current-page:en-gb:url] ?

vakulrai’s picture

FileSize
7.33 KB
7.06 KB

@ dpagini I have applied the patch given at #30 and it gets applied successfully and I am able to get the query string by token.
adding image url's for reference.

thank's.

dpagini’s picture

@jenstechs - I'm not too familiar with translation, so I know I can't speak to that, sorry!

@vakulrai - thanks for checking out the patch!

Given the feedback, would anyone mind if this goes to RTBC and hopefully get a maintainers eyes on this one?

cgmonroe’s picture

Status: Needs review » Needs work

This patch is a subset of the one as in this issue:

#1614456: [current-page:query:?] should fall back to empty string

So, it will not work correctly with a URL that has multiple query parameters: E.g. /blog?tag=security&page=4

The & will be encoded and become & which causes problems with SEO canonical links.

The encoding is happening at ~line 204 in Token.php.

cgmonroe’s picture

Status: Needs work » Needs review
FileSize
3.46 KB

Current patch does not apply with latest dev code because of the coding standard changes. Re-rolled so that it applies now.

NOTE:

Use case for this is to generate SEO correct canonical values on paginated output. E.g. [current-page:url] should create
/blog?page=4&tag=security and not just /blog

dpagini’s picture

So when I use the latest patch, and use [current-page:url] in the metatag module, I am getting the ampersand encoded as suggested in #35. I was thinking that #36 would address that, but it doesn't seem to do so for me.

Having said that... I think this all makes sense, but I was trying to really consider when I would use this. I think I first became interested in this issue b/c I wanted something like pagination in my canonical links. But I don't REALLY want the whole URL. What if someone comes in with with utm_campaign tags (or something like that)? All of a sudden Google is indexing my campaign parameters as my preferred page URL. It seems like other users are having similar thoughts... wouldn't a token like [current-page:url:query:page&tag] be a more useful token? Maybe there's already a way to do that...?

Alex Bukach’s picture

Regenerated against current dev.

surfgatinho’s picture

This is an SEO disaster! Having the canonical page for all paginated pages pointing to the first (non querystring) page is preventing any of the following pages from being indexed.

Google have recently admitted they never even looked at the 'prev' / 'next' tags so those further pages may as well not exist.

In view of this I'd suggest this is quite should be quite an urgent fix...

surfgatinho’s picture

Priority: Normal » Major

Given the impact this has on SEO via the canonical tag I feel it is fairly serious.

robpowell’s picture

I applied this patch to token 8.x-1.5 and it applied cleanly. The view page I was testing had the canonical_url set to [current-page:url:relative] and before this patch, url args would not be passed. After applying the patch, I see the args in the header,

<link rel="canonical" href="/events?title=&amp;event_type%5B0%5D=Industry%20Event" />.

I am marking this RTBC.

robpowell’s picture

Status: Needs review » Reviewed & tested by the community
ckng’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
3.63 KB

Reroll for latest dev.

ckng’s picture

keopx’s picture

ngkoutsaik’s picture

Status: Needs review » Reviewed & tested by the community

Hi,

the latest patch works for me. I run it against the latest branch and the tests pass.

Anybody’s picture

Confirming RTBC!

orlando.thoeny’s picture

Patch #45 works for me as well, using Drupal 9.0.2.

Berdir’s picture

Status: Reviewed & tested by the community » Needs work

Torn on this one, it is a minor behavior change, but according to some comments here, sites with multiple pages should have their own canonical URL. Going to be a bit awkward as content moves further back on pages, as it won't be on the page that google thinks it is, but oh well, the same already happens now.

Will try to remember mentioning this in the release notes.

What about offering a token that does *not* contain the query though? Something like current-page:url:no-query? I can imagine cases where this would be a problem, then those people at least have an option to use a different token.

foreveryo’s picture

As @berdir points out having the no-query should also exist. I've used the patch to solve the issue in paginated views, but now I have issues on regular nodes.

It seems Google add known parameters to other nodes (like articles) and it now returns as canonical the URL with the parameter...

So, it would be great to have 2 tokens. One with parameters and another one without them. In my opinion would be better to add a new one for the URL with parameters and keep the other as it was to prevent regressions.

foreveryo’s picture

I ended up with a custom module solution. I leave it here as it might be useful for someone else. Can be used also as a base for extending the tokens with a query attached.

/**
 * Implements hook_token_info() to add page parameter token
 */
function mymodule_token_info() {
  $info = [];
  // Attach token in "current-page" tokens group
  $info['tokens']['current-page']['url-paginated'] = [
    'name' => t('Url with page parameter'), 
    'description' => t('Absolute url with the page parameter if exists')
  ];

  return $info;
}

/**
 * Implements hook_tokens().
 */
function mymodule_tokens($type, $tokens, array $data, array $options, BubbleableMetadata $bubbleable_metadata) : array {
  $replacements = [];
  $url_options = ['absolute' => TRUE];

  if ($type == 'current-page') {
    $request = \Drupal::request();
    foreach ($tokens as $name => $original) {
      switch ($name) {
        case 'url-paginated':
          $bubbleable_metadata->addCacheContexts(['url.path']);
          try {
            // Checks if URL has the query parameter
            if ($page = $request->query->get('page')) {
              $url = Url::createFromRequest($request)
                ->setOptions($url_options)
                ->setOption('query', ['page' => $page]);
            } else {
              $url = Url::createFromRequest($request)
                ->setOptions($url_options);
            }
          }
          catch (\Exception $e) {
            // Url::createFromRequest() can fail, e.g. on 404 pages.
            // Fall back and try again with Url::fromUserInput().
            try {
              $url = Url::fromUserInput($request->getPathInfo(), $url_options);
            }
            catch (\Exception $e) {
              // Instantiation would fail again on malformed urls.
            }
          }
          if (isset($url)) {
            $replacements[$original] = $url->toString();
          }
          break;
        }
      }
  }


  return $replacements;
}
Summit’s picture

Hi, Foreveryo, what a great name!
It would be great to have this in this module! +1!

Greetings and great christmas!

jcisio’s picture

Status: Needs work » Needs review
FileSize
5.19 KB

So based on #51 and previous patches, a new one with paginated url tokens and leaving the old one intact. According to previous comments, adding query string to current tokens is a behavior change and I think it could cause more headache then what it is trying to solve. Query could be used for sorting etc. and should not change canonical url.

I'm wondering should the url-with-query version be added too.

jcisio’s picture

Status: Needs review » Needs work

The last submitted patch, 54: 1198032-token-paginated-54.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

jcisio’s picture

There was 1 failure:

1) Drupal\Tests\token\Functional\TokenCurrentPageTest::testCurrentPageTokens
Token value for [current-page:url-with-query] was 'http://php-apache-jenkins-drupal8-contrib-patches-77696/subdirectory/node-alias?foo=bar&amp;amp;page=2', expected value 'http://php-apache-jenkins-drupal8-contrib-patches-77696/subdirectory/node-alias?foo=bar&amp;page=2'.
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'http://php-apache-jenkins-drupal8-contrib-patches-77696/subdirectory/node-alias?foo=bar&amp;page=2'
+'http://php-apache-jenkins-drupal8-contrib-patches-77696/subdirectory/node-alias?foo=bar&page=2'

/var/www/html/modules/contrib/token/tests/src/Functional/TokenTestTrait.php:103
/var/www/html/modules/contrib/token/tests/src/Functional/TokenCurrentPageTest.php:76

Something is wrong in the assert.

jcisio’s picture

Ok the last fail is probably due to a core bug #2968558: Canonical (and other) links with multiple query parameters have ampersand encoded. Either to fix core or to fix how token compare urls.

jcisio’s picture

Status: Needs work » Needs review
FileSize
7.13 KB
729 bytes

Just to test CI.

Status: Needs review » Needs work

The last submitted patch, 58: 1198032-58.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

jcisio’s picture

I'm not sure if it is a core bug. I'm trying to reproduce the bug locally but it is not successfully. I can't get the ampersand.

parisek’s picture

sashken2’s picture

I'm use patch #21 from https://www.drupal.org/project/token/issues/1614456
And it works good for me.

generalredneck’s picture

Given that this issue is 5 years older than my oldest child and it can watch PG-13 without supervision, I created a module that handles the scenario and a few like it.

I present Token Url Plus.

This module (as the time of this writing) provides the tokens:

[current-page:url-with-query]

The URL of the current page with query string included, if it exists. This is the functionality we were looking for here.

[current-page:url-with-query:without-some-parameters:param1,param2,…]

The URL of the current page with query string included but filtered by removing the provided query parameters. e.g. 'without-some-parameters:utm_campaign,utm_medium,utm_source' would be replaced with the current url, with the three utm parameters filtered out if they existed.

This is advanced usage for what you are looking for here

[current-page:url-with-query:with-some-parameters:param1,param2,…]

The URL of the current page with query string included but filtered to only include the provided query parameters. e.g. 'with-some-parameters:page,category_id' would return a url containing only query parameters, '?page=1&category_id=2' if they existed.

You can use this one with metatags on views to support JUST the query parameters that the view provides.

This will allow Token the module to remain unchanged and take the burden off of the decision for backward compatibility as expressed in #11.