Link shown after the autocomplete selection is the bare node/xxx link, not the alias

Rerolled #41 against 6.0.x https://git.drupalcode.org/project/linkit/-/merge_requests/16. Everything passes. Patch applies cleanly locally against both 6.0.x-dev and 6.0.0-beta4. Tested locally and now I'm getting aliases instead of node URLs! Marking as "Needs Review" – thank you!

The merge request in patch format.

The linkit-n2877535-50.patch is successfully changed the URL Link field from "node/xxx" to URL Alias, but it still needs to work, as the saved URL is still "node/xxx" rather than saved as its path alias, 'canonical' is not active.

It's ok that the saved value is "node/xxx" because it's converted to the correct URL when the content is rendered; it's a feature.

Thank you Damien, but I think once it has the path alias also it appears in the Link field, it should save as path alias with canonical active.

That would need to be a separate discussion.

I'm encountering an issue after applying patch #50 to Linkit 6.0.0-rc1 (as well as Linkit 5.0-beta 13), the data-entity-substitution, data-entity-type, data-entity-uuid attributes are no longer being added to the link, so the LinkitFilter is no longer working. I can't see why this would be the case after applying this patch, removing the patch resolves this and the attributes are added but I need the functionality provided by this patch. Could anyone suggest why this may be the case?

Sounds like this is right status.

In my instance, that check is preventing the values from being unset, not unsetting them. Changing status after my review.

Testing this in 9.5, #50 breaks the a tag extra attributes. The tag gets put in as just the bare '/node/x' without any data attributes.

Addressing the comments in #51, #53, and #59, all of which are suggesting that the URL alias is what should be saved to the database, rather than the internal route, I agree with Damien McKenna's statement in #52:

It's ok that the saved value is "node/xxx" because it's converted to the correct URL when the content is rendered; it's a feature.

It's ok that the saved value is "node/xxx" because it's converted to the correct URL when the content is rendered

After testing to confirm, I take back my comment. Using the latest patch, the comments in #51, #52, and #59 are effectively pointing out a problem -- not that the internal URL is what is saved (that's fine), but that when the resulting page is rendering, the internal URL is what is rendered to the end-user, rather than the URL alias.

Changing status to "Needs work" to address this. I'm also surprised that there is apparently no test coverage to catch this. I'd like to add test coverage for this going forward: the URL alias should be rendered on the page after save.

This patch no longer applies to the most recent release (Sep 30) :(

This patch no longer applies to the most recent release (Sep 30)

Here's a revised patch that replicates verbatim what was in the patch in #50, and which will apply to the current development release for 6.0.x and 6.1.x.

Noting that the comments from #51, #52, and #59 still need to be addressed: when the resulting page is rendered, the internal URL should not be rendered to the end-user. The URL alias should. Test coverage needs to be added for this, too. Leaving status as "Needs work."

Ive removed this part from the patch everything seems to be working fine for me.
In the code below $input variable comes with an Alias and I don't understand why we should get the $path from it.
In My case the $path variable is converted back to the node/[nid] which is then being set to be the new input.

+    if (!empty($input) && \Drupal::moduleHandler()->moduleExists('path_alias')) {
+      /** @var \Drupal\path_alias\AliasManagerInterface $aliasManager */
+      $aliasManager = \Drupal::service('path_alias.manager');
+      $path = $aliasManager->getPathByAlias($input);
+      if ($path !== $input) {
+        $input = $path;
+      }
+    }

Patch from #64 applied to 6.1.2 for me and solved the generic node path on D10.1.5

The LinkitFilter doesn't look to work with the patch. I'm not seeing the additional attributes getting added to the tag.

I tried to get something working, but did not succeed.

My goal was to use the alias during editing and store the node-url in the database. The link would then show the node alias in the editor, but in the database the node/ was stored. By adding another data-property (data-entity-path) I tried adding a editorDowncast and dataDowncast in the plugin, but that didn't work correctly: it would create a new a-tag wrapping the one that was being made by the base link plugin.

By adding a general upcast conversion, the href is changed to the alias:

editor.conversion.for('upcast')
      .elementToAttribute( {
        view: {
          name: 'a',
          attributes: {
            href: true,
            ['data-entity-alias']: true
          }
        },
        model: {
          key: 'linkHref',
          value: viewElement => viewElement.getAttribute('data-entity-alias'),
        },
        converterPriority: 'high'
      } );

But I failed trying to move the other way around. I tried adding a dataDowncast (which would set the path back to the href), and an editorDowncast (which would use the alias as href). This didn't work, because when creating an element with the same attribute, would add another element to the html instead of merging them (effectively overwriting the href-attribute)
I also tried using a data downcastDispatcher, but that doesn't seem to be able to get the correct viewElement (or rather, it couldn't find any element at all).

ps. there are some other code changes required as well to not only pass but also use the alias and path in the links.

For anybody who needs a quick and dirty fix for this, I'm handling it in hook_preprocess_field() with:

function cc_gin_preprocess_field__node__body__article(&$variables) {
  // Assuming $text is the HTML content you're preprocessing
  $text = $variables["items"][0]["content"]["#text"];

  // Create a new DOMDocument and load the HTML content
  $dom = new DOMDocument();
  @$dom->loadHTML(mb_convert_encoding($text, 'HTML-ENTITIES', 'UTF-8'), LIBXML_HTML_NOIMPLIED | LIBXML_HTML_NODEFDTD);

  $links = $dom->getElementsByTagName('a');

  foreach ($links as $link) {
    // Check if the link is an internal node link
    if ($link->hasAttribute('href') && str_starts_with($link->getAttribute('href'), '/node/')) {
      // Extract node ID
      $path = $link->getAttribute('href');

      // Fetch the alias for the node
      $alias = \Drupal::service('path_alias.manager')->getAliasByPath($path);

      // Replace the href attribute with the alias
      $link->setAttribute('href', $alias);
    }
  }

  // Save the updated HTML
  $variables["items"][0]["content"]["#text"] = $dom->saveHTML();
}

Note that this was generated by chatgpt, so I'm not sure that all of the args in $dom->loadHTML() are needed, but it seems to do the trick for me.

I updated this issue with some code that will use the entity title (already obtained via Linkit) as the default link text for a brand new inserted link without a selection range: ✨ Display node title (a text) in by default when creating link in ckeditor5 Closed: duplicate

Patch from #64 applied to 7.0.0-alpha1 seems to be working well for me ; here is the generated link code :
<a class="test-css" href="/fr/mentions-legales" target="_blank" aria-label="test-aria" id="test-idd" rel="test-rel" data-entity-type="node" data-entity-uuid="7a012594-8e2f-446f-8559-ac377b1ec0d5" data-entity-substitution="canonical" tabindex="-1" data-title="Test title">Test link</a>

Sorry for the noise;

First I rebased the existing MR branch on the latest 6.1.x branch but apparently you can't change the destination of MRs. I then reverted that existing MR.

I then applied the patch from #64 to both the 6.1.x and the 7.x branch; apparently the 6.1.x branch has more (recent) changes than the 7.x branch. I've created MRs for both branches.

Snapshots of latest state of MRs for both 6.1.x and 7.x for safe usage with composer-patches

Only attaching a patch for 6.1.x because that's what I'm using.

It should return alias on selection, downcast to internal path for the source, and upcast to alias when editing.

I did see some errors thrown related to tooltip or something but it seem to affect functionality so I didn't bother fixing it.

Moved some things around so there's fewer changes and easier to understand.

The buildPath() signature change can break extending classes (eg. media_link_enhancements) ... but it seems silly to repeat that code as a new function with just the path processing change.

I'm also aware that the synchronous call is maybe not ideal and throws a warning.

If someone were inclined to do this without the sync call:

The problem is on the editor initial load with existing links.

Link upcast/downcast occurs and there's no alias info in the source data where upcast/downcast usually get its values from.
And the source data shouldn't have alias info because we don't want to save alias data into markup.
And, at least for me, I want to use LinkIt matchers but not the LinkIt url converter because it's not compatible with links on drupal-media images; extra data attributes like those required for url conversion are consumed off the link.

Possible solution is to find where to generate alias info from the source data and make it available to the upcast/downcast and before the editor is available for users to start clicking around. Maybe it can be done in the field widget initialization.

Okay, last one: this version doesn't use synch xhr, adds data-entity-alias attributes on form load, and removes them on downcast.

There's a minor issue with switching text formats between no-ckeditor5 and ckeditor5.
The alias addition on form load is set to happen for ckeditor5.
If switching from ckeditor5 to no ckeditor5, the data-entity-alias will show up.
Conversely, if switching from no ckeditor5 to ckeditor5, the aliases will not be populated in the link balloon.

One method to address this is the same way the editor js calls xss filtering on text format changes.
In fact, the data-entity-alias form preload could probably all be handled in js anyways like that.

Loading alias info into global drupalSettings allows it to be available to the ckeditor5 linkit plugin on init so the alias can be upcast into the model. Storing in drupalSettings also allows it to be available to all text formats and persistent between switching text formats.

I think this no longer applies on 7.0.5. The patch from #77 applies, but appears to be causing the error reported on Can't Insert or Update Link after Upgrading to Drupal Core 10.5 🐛 Can't Insert or Update Link after Upgrading to Drupal Core 10.5 Active

Ah! My patented "wait a while and see if things 'magically solve themselves'" technique seems to have worked!

Thanks to all the wizards out there!

Patch #64 worked fine for me. Running 10.5.x version of core.

I tested the last patch, but it didn't work for Drupal 10.5.x and Linkit 7.0.8. If anyone has any idea how to solve this, I would greatly appreciate it.

Weighing in as a maintainer of the Linkit module, I would point out that there are others in the community who want the entity title to show: ✨ Show entity title after autocomplete selection instead of internal route (e.g., /node/123) Needs work . This, plus the fact that it is possible to configure the Linkit metadata to render the entity title in the matcher information, makes me disinclined to plan to include this in a release of Linkit. Community members are free to use the patch here, of course, but I'd really recommend going the route of adjusting the autocomplete metadata in the Linkit settings.

MR 138: for 7.x Continue to use internal route so links don’t get broken, but display url alias in linkit widget and CKeditor when present.

Apologies for the commit noise.. Now that all tests are passing for my MR138....

Drupal Link Handling Changes

Apologies for the commit noise.. Now that all tests are passing for my MR138....

Drupal Link Handling Changes

Some screengrabs: