Problem/motivation

The LinkitFormatter plugin implementation contains logic of using a "substitute URL" when the link points to an existing entity. In such a case, the formatter plugin will substitute any given URL from the field item by the URL of the entity.

This is mainly happening within the viewElements method of the LinkitFormatter class as shown in the following excerpt:

public function viewElements(FieldItemListInterface $items, $langcode) {
    $elements = parent::viewElements($items, $langcode);
    $settings = $this->getSettings();

    // Loop over the elements and substitute the URL.
    foreach ($elements as $delta => &$item) {
      /** @var \Drupal\link\LinkItemInterface $link_item */
      $link_item = $items->get($delta);
      $item_url = $this->buildUrl($link_item);
      $item_url_attributes = $item_url->getOption('attributes');
      if ($url = $this->getSubstitutedUrl($link_item)) {
        if ($url instanceof CacheableDependencyInterface) {
          $cacheable_url = $url;
        }
        // Keep query and fragment.
        $parsed_url = parse_url($link_item->uri);
        if (!empty($parsed_url['query'])) {
          $parsed_query = [];
// ...

Link to the relevant code part:
https://git.drupalcode.org/project/linkit/-/blob/7.x/src/Plugin/Field/Fi...

I have an (admittedly edgy but valid) case where I store custom "query" values as options at the link field itself. The link field item allows to store such options as a serialized array besides the link uri.

Due to the currently implemented logic highlighted above, such previously stores "query" values may get lost, since the part that is trying to keep the "query" option is only looking at the uri itself, but not at the stored field item's options.

Steps to reproduce

This is coming from a custom code implementation where it fell into this trap, so it's hard to provide easy steps to reproduce here. I hope the problem description is enough for understanding the problem.

Proposed resolution

The logic that is trying to preserve the query arguments should additionally check for any stores "query" options at the field item. Or even better, load all options from the field item and put it back into the newly create Url object.

Issue fork linkit-3613627

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

mxh created an issue. See original summary.

mxh opened merge request !177

mxh’s picture

mxh commented
Status: Active » Needs review

Created MR177 which suggests to add an additional merge of already existing query arguments from the "original" item url.

csakiistvan’s picture

csakiistvan
Primary language Hungarian
Location Budapest, Hungary
commented
Assigned: Unassigned » csakiistvan
csakiistvan’s picture

csakiistvan
Primary language Hungarian
Location Budapest, Hungary
commented
Assigned: csakiistvan » Unassigned
Status: Needs review » Reviewed & tested by the community

Environment

  • Drupal: 11.4.4
  • PHP: 8.5.5
  • Database: MariaDB 10.11.16
  • DDEV: v1.25.2
  • Linkit: 7.x-dev (0a84c12)
  • Browser: Chrome

Prerequisites

  • An article node used as the link target, with a URL alias, for example "Linkit target ALPHA" at /alpha-target.
  • A link field on the article content type, with the Linkit widget and the Linkit formatter, using a Linkit profile whose node matcher uses canonical substitution.
  • A second article carrying that link field. Query options stored on the field item cannot be entered through the widget, so the field value has to be set programmatically:
ddev drush php:eval '
$n = \Drupal::entityTypeManager()->getStorage("node")->load(<NID>);
$n->set("field_linkit_link", [[
  "uri" => "entity:node/<TARGET NID>",
  "title" => "ALPHA with tracking query",
  "options" => ["query" => ["utm_source" => "newsletter", "page" => "2"]],
]]);
$n->save();
'

Steps

  1. Apply the fix from MR !177: the formatter now merges the query arguments of the original item URL into the substituted URL, instead of only preserving the query string parsed out of the item's uri.
  2. Rebuild caches: ddev drush cr
  3. Visit the node carrying the link field and inspect the rendered link's href in the Linkit link field.
  4. Repeat with the query supplied in the uri instead of the options, for example entity:node/<TARGET NID>?from=uri with empty options, and confirm that case still works.
  5. Repeat with a query in both places plus a fragment, for example entity:node/<TARGET NID>?from=uri#sec with options holding utm_source=newsletter and from=options.

Expected results

  • Query arguments stored in the field item's options survive the URL substitution and appear on the rendered link.
  • Query arguments supplied in the uri keep working, as does the fragment.
  • When the same key is present in both places, the value from the uri wins.

Actual results

As expected. Before the fix, the stored options were dropped: the link rendered as <a href="/alpha-target" hreflang="en">, with no trace of utm_source or page. A query supplied in the uri was kept (/alpha-target?from=uri), and with both sources present only the uri one survived (/alpha-target?from=uri#sec).

After applying MR !177, the same three cases render as /alpha-target?utm_source=newsletter&page=2, /alpha-target?from=uri and /alpha-target?utm_source=newsletter&from=uri#sec. The stored options are preserved, the previous behaviour for URI queries and fragments is unchanged, and on the colliding from key the uri value takes precedence.

Testing produced with the assistance of an LLM.

idebr’s picture

idebr commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

Let's add some automated test coverage showcasing where the current logic fails