Linkit for Link field

For internal links I think you should go for an entitiy reference field. Makes no sense to have that as a link field anymore.

I would say that it does still make sense to support Linkit for field types using autocomplete like entity reference fields, link fields (if you enabled links to local content).

Finding content using the native autocomplete in Drupal core can be really hard for editors if you have a lot of content and have multiple nodes with the same title. And this is where LinkIt makes the editor experience much better, because the profiles can be configured to show more info about the entity than just title and id.

I'm personally maintaining
https://www.drupal.org/project/linkit_references and would like that moved to D8 for entity references, but not sure if that is possible at all without the field support in version 8.x-5.x of the Linkit core module.

I would agree on the need for support for the Link field.

I have a Link field that can have either links to internal or external content. I can't change it to a reference field as that would block use of external links.

+1 for this feature in D8

+1 for this feature in 8.x-5.x

I made my first Drupal 8 patch so it might not be very good, but it works for regular link fields and link fields in paragraphs.

I decided to make a new Linkit widget for link fields, so to use this widget you need to select it on the form display tab and select a profile in the widget settings.

I saw that my patch relies on file_entity. I'll try to fix it so that it doesn't rely on file_entity.

Update: I did not get my patch to work without file_entity.

I updated my patch a little because there was a problem.

When a user typed a url in the form of a internal url (like: 'google.com') and clicked the autocomplete result, the 'href' hidden field got set. The validation would fail, because this is an incorrect internal path. The form would be reloaded and when the user fixes his error by typing in 'https://google.com', but not click the autocomplete result, the hidden field would keep its previous value. I fixed this issue by not using the href hidden field, but instead use the value of the uri field.

I also fixed a wrongly indented array and removed the 'Selected link:' text, because it was redundant.

I suppose the correct status for this issue is 'Needs review' now that there is a patch?

Tested the patch and it doesn't apply to the latest dev version. May try and fix it but if @frank.schalkwijk wants to take a look as the patch author, that would be great.

+1 for this feature in 8.x-5.x

patch doesn't apply to the latest dev version

Ditto - this feature would be very useful. Entity reference is good for linking to only internal content - but does not work if you want to provide both internal and external links.

Attempted to amend the patch but it proved beyond my ability.

I have taken the patch above, which I could not get to work, and created a module linkit_widget.

It would make far better sense for this to be incorporated into the Linkit module, perhaps as a sub-module? But I created a separate module to avoid the un-merged patch state we have on this ticket. (@anon any chance of getting this into Linkit, the use-case is solid).

There are some issues on the linkit widget, like if you have more than one on a page :); if it was a patch or a sub-module we could fix that in autocomplete.js (I think).

This needs to be a thing! This module creates a better way for you to create links, so why shouldn't it work with the built in way to create links?

Rerolled the patch so that it applies on current dev again. The only changes that didn't apply were in the autocomplete.js.

With patch #16 if there are multiple link field items on the form, then all open up at the same time.
The attached patch ensure that the "click" event only operates on 1.

updated the element used to search upon the click event as well.

Added JS to auto-populate the link title if it has not been manually set.

The above patch works fine with populating the link title but breaks the links placed via CKEditor.

Not sure about the unset logic in linkit_form_editor_link_dialog_submit() but removing it fixes the issues.

As the substitution doesn't work for link fields, this patch aims to do so. It adds a new LinkitFormatter which overrides the URL and replaces it with the generated/substituted one.

Also, moved utility classes from the LinkitWidget into a new helper class.

Thoughts are very welcome.

Added schema for the linkit widget and linkit formatter.

When I tested the other day it looked like the show/hide link text option in field settings was not being respected, will need to check again though.

Just tried the patch in #23 and it seemed fine. I haven't tried the formatter yet though (was using Link's "Separate link text and URL" instead).

An improvement that adds a possibility to link multilingual content correctly.

Also, when touching LinkitHelper replaced $entity->GetEntityTypeId() with $entity->getEntityTypeId() call.

Missing comment from #27:

1. +++ b/js/autocomplete.js
   @@ -75,12 +75,11 @@
   -
   -      $('input[name="attributes[href]"], input[name$="[attributes][href]"]', $context).val(ui.item.path);
   -      $('input[name="attributes[data-entity-type]"], input[name$="[attributes][data-entity-type]"]', $context).val(ui.item.entity_type_id);
   -      $('input[name="attributes[data-entity-uuid]"], input[name$="[attributes][data-entity-uuid]"]', $context).val(ui.item.entity_uuid);
   -      $('input[name="attributes[data-entity-substitution]"], input[name$="[attributes][data-entity-substitution]"]', $context).val(ui.item.substitution_id);
        }
   +    $('input[name="attributes[href]"], input[name$="[attributes][href]"]', $context).val(ui.item.path);
   +    $('input[name="attributes[data-entity-type]"], input[name$="[attributes][data-entity-type]"]', $context).val(ui.item.entity_type_id);
   +    $('input[name="attributes[data-entity-uuid]"], input[name$="[attributes][data-entity-uuid]"]', $context).val(ui.item.entity_uuid);
   +    $('input[name="attributes[data-entity-substitution]"], input[name$="[attributes][data-entity-substitution]"]', $context).val(ui.item.substitution_id);
   

   Having this inside if condition prevents updating a link from internal to external.

2. +++ b/src/Plugin/Field/FieldFormatter/LinkitFormatter.php
   @@ -96,9 +96,10 @@ class LinkitFormatter extends LinkFormatter implements ContainerFactoryPluginInt
   +      $substituted_url = $this->getSubstitutedUrl($link_item);
   +      // Convert generated URL into a URL object.
   +      if ($substituted_url && ($url = \Drupal::pathValidator()->getUrlIfValid($substituted_url->getGeneratedUrl()))) {
   +        $item['#url'] = $url;
   

   Without conversion (generated URL to URL object) the formatted doesn't work for entities.
   However, a substitution plugin should return a URL object instead IMO. Not sure if that is going to work for files though...

One more thing I spotted, the linkit widget fails to select a file via autocomplete as Url::fromEntityUri() can't create a URL for file entities (There is no canonical link template for files).

We could think about writing test coverage for the current code.

+1 for this FR, for link fields allowing both internal and external links we can't use entity references, and LinkIt is a huge improvement in UX

Made a bug report in #2916023: Deleted linkit entity produce fatal exception and when i started to create a patch i figured that we are using patched version of module from this issue.

We have a quite a big problem with a next scenario. We have a node with paragraphs which can have linkit field to other nodes. If linkit node is deleted then when you view parent node next PHP fatal error is generated:

The website encountered an unexpected error. Please try again later.
TypeError: Argument 1 passed to Drupal\Core\Entity\EntityRepository::getTranslationFromContext() must implement interface Drupal\Core\Entity\EntityInterface, null given, called in.../web/modules/contrib/linkit/src/Utility/LinkitHelper.php on line 21 in Drupal\Core\Entity\EntityRepository->getTranslationFromContext() (line 82 of core/lib/Drupal/Core/Entity/EntityRepository.php).
Drupal\Core\Entity\EntityRepository->getTranslationFromContext(NULL) (Line: 21)
Drupal\linkit\Utility\LinkitHelper::getEntityFromUri('entity:node/108') (Line: 121)
Drupal\linkit\Plugin\Field\FieldFormatter\LinkitFormatter->getSubstitutedUrl(Object) (Line: 99)
Drupal\linkit\Plugin\Field\FieldFormatter\LinkitFormatter->viewElements(Object, 'de') (Line: 80)
Drupal\Core\Field\FormatterBase->view(Object, 'de') (Line: 259)
Drupal\Core\Entity\Entity\EntityViewDisplay->buildMultiple(Array) (Line: 320)
Drupal\Core\Entity\EntityViewBuilder->buildComponents(Array, Array, Array, 'primer_teaser_grid') (Line: 263)
Drupal\Core\Entity\EntityViewBuilder->buildMultiple(Array) (Line: 18)
Drupal\paragraphs\ParagraphViewBuilder->buildMultiple(Array) (Line: 220)
Drupal\Core\Entity\EntityViewBuilder->build(Array)
call_user_func(Array, Array) (Line: 376)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 195)
...

The problem is in LinkitHelper::getEntityFromUri() method which will try to load entity that not exist and then pass it to the getTranslationFromContext() method, and this produce fatal error.

Here is a quick change for now that will fix this quickly until we figure better approach.

Adding another use case in hope to get some momentum on this issue: Linking to media entities.

The core link field autocomplete does not work with multiple different entity types, and the single entity-type is hardcoded to node.

The link field of Drupal supports providing a fragment to link targets. This linkit implementation always drops the fragment.
However, with the linkit UI, we also need to figure out how we would display / edit that fragment part. That's related to: #2902875: Make the selected item not look like text

From tests i discovered that if the link target title contains a special char like "&" it leads to double encoding in the link label.

Working on this.

After lots of research found the escaping problem. It basically escapes the label of the node and when building the suggestions it retrieves it in the same way, escaping. So when clicking the suggestion it just copies the raw value of the retrieved suggestion for the label.

The change should work for labels, keeping the URL as it was.

I don't see how this change might affect the current functionality. Using a better var name.

Are you sure that this issue specifically only affects the link field and not also the link button in WYSIWYG? If yes, then it should be a separate issue. I guess it will conflict though because the changes will also overlap.

Yeah sorry, tested in WYSIWYG and it's the same.
Created dedicated issue.

For those interested, I've created an add-on for this feature at https://www.drupal.org/project/link_attributes/issues/2939514 which adds a new widget based on the one provided by this patch. The new widget combines the attributes provided by link_attributes module with the linkit selector for link fields.

The work there is based on the patch at comment #30.

Here's an updated version of #30 to address recent updates to dev.

I think this should fix the tests.

Also removing a problem we had with inline form errors module:

Adding patch against old dev same as #30 and against current dev.

Not really sure why it fails, it basically says the 'data-entity-type' is added to the link but manual test says that it isn't.

Working on this.

This should fix the existing tests I think. Also changed some CS violations along the way.

Will be working on writting tests for the new widget/formatter next.

This adds a test that should cover the most basic scenarios for the widget and formatter.

Anything else we should test?

A small improvement that preserves the fragment (#anchor) from entity URIs when used in the field widget.

Looks pretty good, didn't really review the JS though.

1. +++ b/js/autocomplete.js
   @@ -172,7 +187,26 @@
            $autocomplete.click(function () {
   -          $autocomplete.autocomplete('search', $autocomplete.val());
   +          var $t = $(this);
   +          $t.autocomplete('search', $t.val());
   +        });
   

   maybe we can use a better variable name for this, looks like it is related to translation but it isn't.

2. +++ b/src/Plugin/Field/FieldWidget/LinkitWidget.php
   @@ -0,0 +1,211 @@
   +      '#description' => t('Start typing to find content or paste a URL.'),
   

   $this->t()

3. +++ b/src/Utility/LinkitHelper.php
   @@ -0,0 +1,82 @@
   +  public static function getEntityFromUri($uri) {
   +    // Stripe out potential query and fragment from the uri.
   +    $uri = strtok(strtok($uri, "?"), "#");
   +    list($entity_type, $entity_id) = explode('/', substr($uri, 7), 2);
   +    $entity_manager = \Drupal::entityTypeManager();
   +    if ($entity_manager->getDefinition($entity_type, FALSE)) {
   +      if ($entity = $entity_manager->getStorage($entity_type)->load($entity_id)) {
   

   This helper uses quite a few services, seems like it should possibly be a service too that is injected into the widget (and formatter, if used there)?

4. +++ b/tests/src/FunctionalJavascript/LinkFieldTest.php
   @@ -0,0 +1,162 @@
   +    // In order to prevent false failures in enviornments with url prefixes,
   +    // use a more robust way of checking if the href is what we expect.
   +    $this->assertTrue(strpos($href_value, '/entity_test_mul/') !== FALSE);
   

   I think phpunit has an assertcontains(), maybe also include the ID here in the check?

@Berdir thanks for reviewing!

This addresses #51 except for #51.3:

This helper uses quite a few services, seems like it should possibly be a service too that is injected into the widget (and formatter, if used there)?

Both helper functions are declared as static, and one of them (getEntityFromUri()) is called in a static context from \Drupal\linkit\Plugin\Field\FieldWidget\LinkitWidget::getUriAsDisplayableString(), which is overriding core's \Drupal\link\Plugin\Field\FieldWidget\LinkWidget::getUriAsDisplayableString().

The other helper (::getUriFromSubmittedValue() is apparently not being called in a static context, not sure why it was initially created as static on this patch. Do you think it's worth making this a service and having only this method non-static?

+++ b/tests/src/FunctionalJavascript/LinkFieldTest.php
@@ -0,0 +1,165 @@
+
+    // Create a test entity to be used as target.
+    /** @var \Drupal\Core\Entity\EntityInterface $entity */
+    $entity = EntityTestMul::create(['name' => 'Foo']);
+    $entity->save();

You are creating the entity that you are using for tests here and have it in $entity, shouldn't be necessary to load it again with a query?

Ok, that one usage of the helper is static is a bit unfortunate, but we are using several services, so if we ever want to write unit tests for some logic in there, it would be a lot easier to do so when it is a service. The none call needs to use \Drupal::service() but that's not a big deal.

oops, that is indeed not necessary :)

Thanks!

This restores the behavior from patch 30 (that was lost at some point along the way) of showing the selected entity's label in the URL field, instead of /node/23 or something like that.

Also, improved the test coverage to cover external URLs and anchor fragments as well.

OK, trying not to break it elsewhere now.

I've tested the patch in #59 against 8.x-5.0-beta7 and it worked for me. Thank you!

+++ b/tests/src/FunctionalJavascript/LinkitDialogTest.php
@@ -190,8 +190,8 @@ class LinkitDialogTest extends JavascriptTestBase {
 
-    // Make sure the linkit field field is populated with the node url.
-    $this->assertEquals($entity->toUrl()->toString(), $href_field->getValue(), 'The href field is populated with the node url.');
+    // Make sure the linkit field field is populated with the node label.
+    $this->assertEquals($entity->label(), $href_field->getValue(), 'The href field was not populated with the node label.');
 

the description seems backward, we are making that it is equal?

Could be fixed on commit but maybe @marcoscano has a minute to confirm and update the patch.

We also use this and it would be great if we could finally commit this. This issue has 53 followers, so lots of people are interested in this, would be amazing if we could finally get switch to a non-patched version of linkit. We've been using different patches from this issue for a year or so and have been testing it extensively.

+    $this->assertEquals($entity->label(), $href_field->getValue(), 'The href field was not populated with the node label.');

Yeah the assert* message parameter is always confusing... lately I've just stopped using them, they are IMHO hardly needed to understand a given failure anyways...

This message is actually shown when the test fails ( https://github.com/sebastianbergmann/phpunit/blob/6.5.7/src/Framework/Co... ), so in this case I believe the message should indicate that the label is *not* what it is expected, isn't it?

Maybe I'm missing something, let me know if that's the case!

New bug (or arguably unexpected behavior) found here.

Steps to reproduce:
1) Apply this patch and configure a node link field to use this widget
2) Create a node with an internal route on the link field
3) Open the node edit form. Type an external route, such as "https://google.com"
4) Do not click on the dropdown suggestions. Instead click "Save" to save the node directly.

You would expect that "https://google.com" gets saved, but instead the internal route that was previously stored in the element attributes overrides that. If you click on the dropdown suggestion that says "Linkit could not find any suggestions. This URL will be used as is", however, everything works as expected.

I'm not very fond of my javascript skills, but I don't see any other option apart from resetting the attributes everytime a new key is pressed, once the user can write / select / write something different / etc several times, so we can never ensure the attributes are well defined, except when they click on the dropdown (after which no keyup should happen anymore).

Thoughts?

Tests++ :)

I had forgotten the anchor-feature... (too many features...)

So I guess we may have a conflict of expectations here...

In a node where a previous route to an internal link (for example "Related product Foo") exists:

- User A wants to be able to write "http://google.com#some-fancy-anchor" and then directly click save. He/she expects that this external URL will override the internal route previously saved.

- User B wants to be able to just add #some-anchor-text to the existing internal route title, so the link points to the correct fragment/anchor when saved. After writing the anchor in the text field (which would become then "Related product Foo#some-anchor-text"), he/she doesn't click on anything, only on "Save" to save the node. His/her expectation here is that the internal route is preserved and the anchor added.

Who should win?

I understand your predicament... You might consider expanding the "Link attributes" module to include a fragment field and use the patch I created in #41 to integrate "Link attributes" with "Linkit for link fields."

Good luck!

OK, so after thinking about this for a while, we decided to prioritize the "current" functionality, and maybe just try to improve some descriptions.

This patch tries to make it a little bit clearer that you need to effectively click on the dropdown, even if there are no matches.

The interdiff is against #59 because we should then disregard patch #63.

I get an issue with using the patch and trying to link to an article that has an ampersand "&" in the title. Using version 8.x-5.0-beta7 when I link to an article using linkit in the wysiwyg everything works as expected. After applying the patch and linking to an article that has an ampersand in its title e.g. "Fees & Services". Then the title is used as the href for the link.

Before patch source code:

<p><a data-entity-substitution="canonical" data-entity-type="node" data-entity-uuid="8826c7a2-d761-4205-b565-fe2daad14944" href="/node/1">link</a></p>

After patch source code:

<p><a href="Fees &amp;amp; Services">link</a></p>

I can confirm problem mentioned in #68.

I think that I have found another issue. I initially feature request since I would like to reference files from the link field (not part of a media). With the patch the files are shown in the autocomplete as expected but I can't save the node as it raise an error: "1 error has been found for: Links (value 3)".

The hidden inputs (entity-type, …) look correct.

edit: I disabled the "inline form errors" module and the error is more helpful I think: The path 'my_file.pdf' is invalid.
So the error come from LinkTypeConstraint.php from the core link field.

edit2: The error is in fact in LinkNotExistingInternalConstraintValidator.php, it raise a `RouteNotFoundException`. The Drupal\Core\Url object has a routeName "entity.file.canonical" and as a routeParameters `file` => `78` which looks right.

edit3: The "entity.file.canonical" is defined in the file_entity module but I have it disabled so that's why it doesn't work. I think it should not be a requirement for this module to work with files or I may have a strange config here.

edit4: The issue https://www.drupal.org/node/2423093 may be related to this.

edit5: So I compared the uri given by getUriFromSubmittedValue with a file loaded by id and using the "getFileUri" method on the entity.
The getUriFromSubmittedValue() return this: 'entity:file/86'
The getFileUri give me something like 'public://2018-06/myfile.pdf'
So the issue is in getUriFromSubmittedValue from LinkitHelper.php where if there is an entity the uri is "hardcoded" as this: '$entity_uri = 'entity:' . $entity->getEntityTypeId() . '/' . $entity->id();'

edit6: I tried to add this code to getUriFromSubmittedValue so that the uri returned for files look like the "public://" but I now get this LinkExternalProtocolsConstraintValidator from the link module.

$moduleHandler = \Drupal::service('module_handler');

$entity_uri = 'entity:' . $entity->getEntityTypeId() . '/' . $entity->id();
// File do not have the entity:file uri unless file_entity is enabled.
if ($entity->getEntityTypeId() == 'file' && !$moduleHandler->moduleExists('file_entity')){
  $entity_uri = $entity->getFileUri();
}

I have installed the latest patch and when I try and link to a media file, the link points to the file media entity rather than a direct link to file.

I have double checked and my LinkIt profile has the "Direct URL to Media File Entity" selected.

Are others having the same issue or I am missing something?

I tried to enable the file_entity module and now it is working as expected : ) It would be great if this was not a requirement or if it is display a warning when we enable the "file" matcher about this.

@danjordan maybe check that you are using the "linkit" formatter on the manage display tab, by default it still use the standard link formatter.

Thanks @idflood. That got it.

@danjordan I ran into the same issue where

I have installed the latest patch and when I try and link to a media file, the link points to the file media entity rather than a direct link to file.

I have also double checked and my LinkIt profile has the "Direct URL to Media File Entity" selected. And that I am using the "linkit" formatter on the manage display tab.
Did you have to do anything else to get the Linkit Link to generate a direct link to file.

@Thread
What's strange is that the WYSWYG editors have an optional LinkIt filter that Runs if checked which Converts entity:node/1 into a "real" URL when rendering
the text but I'm not sure how that optional Filter is handled outside of the Content Editors; for instance, when the Link is just a field on a content type.

@freddya21 that was all. I am using version 5.0-beta7 of Linkit module. Installed latest patch, cleared all caches and that got it to work for me.

The problem was that I was using a Custom LinkIt Profile that I had added; and apparently the Custom LinkIt Profiles are Ignored when It comes to Leveraging LinkIt to publish Fields-based Links. I even made sure to select my custom profile under the Form Display Formatter. So in order to get it to work I ended up having to Make my desired selection within the Default LinkIt Profile.

Also Noticed that there is an issue where Field-based LinkIt Links that point directly to the underlying Media file; they require the cache to be flushed before it will show the new URL. This isn't the case with WYSWYG LinkIt Links, they show the latest media files without flushing the cache.

Issue Summary:

• Custom LinkIt Profiles Ignored by LinkIt Fields
• Newly Swapped Media Files Require Editors to Flush The Cache before showing up for LinkIt Fields

Another quirk I noticed when using this in conjunction with simple (non-entity) matchers (i.e., Email), at least in the context of a wysiwyg: the returned href value is the autocomplete label, not the path. The attached interdiff against #67 sets the href to the raw path if entity matching is *not* used; I'm not at all confident this is the right fix or a good idea but it seems to fix the immediate problem.

I found another issue. Our website is translated in 2 languages, german and french, with french as the default language.

Eventually when we edit a german page and try to link to a translated page in german the link is invalid (only the title in href, no data-* attributes).

The problem comes from this check in `linkit_form_editor_link_dialog_submit`:

if ($entity && strpos($href, $entity->label()) !== FALSE) {

The '$href' is correct as this correspond to the translated page title. But the $entity we compare against is not translated so the condition doesn't work here.

In the updated patch I added `$translation = \Drupal::service('entity.repository')->getTranslationFromContext($entity);`, but it might not be right in all situations I think. Ideally we should load the translation according to the selected item and not from context. For example the current approach might cause an issue when trying to link to a french page when editing a german page.

This condition has been added in #57 by @marcoscano, maybe you can help here?

Hi,

Everyone can explain to me why don't using this :

$form_state->setValue(['attributes', 'href'], $href_dirty_check);

instead of this :

$form_state->setValue(['attributes', 'href'], $translation->toUrl()->toString());

in linkit_form_editor_link_dialog_submit handler from linkit.module.

By using the $href_dirty_check variable (ie node/1234), we are sure that the link will work in any language, even if the alias change after some modifications on the target entity. In this case, the multilingual support is ok and the target will remain because we map the internal link instead of an alias.
By using the alias, we can't assure that the link will become dead after modifications.

But maybe you have a good reason to do it like this ?

Thanks in advance for any explanation.

Hi,

New 8.x-5.x LinkitFilter makes sure new alias is rendered after alias changes. But it works only if links is added using new 8.x-5.x LinkitDrupalLink CKEditor plugin.
So, I ended up creating additional filter to process internal paths that are not created using new Linkit approach with data-entity attributes.

I agree with @titouille regarding setting href value as internal path, for consistency. LinkIt filter should be used in any case, let's generate url from internal path.

Adding modified patch with $href_dirty_check.

Should hopefully fix the null $entity testbot error in #81 and also deal with html entity encoding quirks as mentioned above a few times.

Thanks for working on this! Is it possible to include the functionality of indicating the Link Text as required with an asterisk when there is a URL populated like the core link field widget does? I appreciate adding the referenced entity title automatically, but in the event that an author wants to overwrite it and they delete the title, it would be nice to have an indication that something is required. I can confirm that the post-submit validation does work, just looking to improve the UX in this respect.

Have folks been able to implement this successfully with multiple instances of the linkit widget on a node edit form?

I'm getting an ajax error (whose symptom is the progress wheel spinning, but no dropdown of entities) with the following field setup on a node bundle:
* Link field on the node (linkit widget, functions as expected)
* Paragraphs field > Paragraphs field > Link field on paragraph (linkit widget, does not function)

To rule out Paragraphs ajax conflicts, I've also added an additional link field to the node directly and have gotten the same results:
* Link field on the node (linkit widget, functions as expected)
* Second link field on the node (linkit widget, does not function)

In both cases, I get the following ajax error in my console:

An AJAX HTTP error occurred.
HTTP Result Code: 500
Debugging information follows.
Path: /node/add/landing_page?ajax_form=1
StatusText: 500 Service unavailable (with message)
ResponseText: The website encountered an unexpected error. Please try again later.Drupal\Core\Entity\EntityMalformedException: The "node" entity cannot have a URI as it does not have an ID in Drupal\Core\Entity\Entity->toUrl() (line 190 of core/lib/Drupal/Core/Entity/Entity.php). Drupal\value\Normalizer\ContentEntityNormalizer->normalize(Object, 'value', Array) (Line: 143)
Symfony\Component\Serializer\Serializer->normalize(Object, 'value') (Line: 38)
Drupal\value\ThemeManager->buildValues('node', Array) (Line: 22)
Drupal\value\ThemeManager->render('node', Array) (Line: 437)
Drupal\Core\Render\Renderer->doRender(Array, 1) (Line: 195)
Drupal\Core\Render\Renderer->render(Array, 1) (Line: 139)
Drupal\Core\Render\Renderer->Drupal\Core\Render\{closure}() (Line: 582)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 140)
Drupal\Core\Render\Renderer->renderRoot(Array) (Line: 133)
Drupal\yoast_seo\EntityAnalyser->renderEntity(Object) (Line: 69)
Drupal\yoast_seo\EntityAnalyser->createEntityPreview(Object) (Line: 78)
Drupal\yoast_seo\Form\AnalysisFormHandler->analysisSubmitAjax(Array, Object, Object)
call_user_func_array(Array, Array) (Line: 69)
Drupal\Core\Form\FormAjaxResponseBuilder->buildResponse(Object, Array, Object, Array) (Line: 98)
Drupal\Core\Form\EventSubscriber\FormAjaxSubscriber->onException(Object, 'kernel.exception', Object)
call_user_func(Array, Object, 'kernel.exception', Object) (Line: 111)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('kernel.exception', Object) (Line: 228)
Symfony\Component\HttpKernel\HttpKernel->handleException(Object, Object, 1) (Line: 79)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 99)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 78)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 52)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 666)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

I've applied this patch to the 8.x-5.x-dev version. Please let me know if I can provide any other context.

I don't think that has anything to do with this issue. It looks like yoast_seo is interfering there:

....
Drupal\yoast_seo\EntityAnalyser->renderEntity(Object) (Line: 69)
Drupal\yoast_seo\EntityAnalyser->createEntityPreview(Object) (Line: 78)
Drupal\yoast_seo\Form\AnalysisFormHandler->analysisSubmitAjax(Array, Object, Object)
....

Combined with this:

Drupal\value\ThemeManager->buildValues('node', Array) (Line: 22)

I don't know what that module is but it seems to be replacing the core theme manager and for whatever reason serialize it and you can't do that with unsaved/new entities...

Thank you @Berdir I definitely missed that in the stacktrace. Sorry for crowding the thread with that noise.

I think I determined what was happening for me. When I switched the widget for the field on the node, I also checked the settings for the field widget which resulted in the linkit profile name being saved:

And as a result the ajax request is made to the proper path for that linkit profile:

When I switched the widget for a link field on my paragraph, I didn't need to see the widget settings so I never opened them and as a result, no linkit profile was saved:

And as a result the ajax request is made to a path which returns nothing:

I only have 1 linkit profile, so it'd be great to have that set by default without needing to interact with the field widget settings - but at least now I have a workaround.

Hope this helps.
Thanks

The recent changes fixed various problems, but for example the html encoding issue should instead be done by decoding in javascript (with https://api.jquery.com/jquery.parsehtml/) so that things like " also correctly show in the editor. I also think the dirty check is now broken, if you for example manually change to a path after selecting an entity, it will ignore that.

I'm wondering if we should remove the label stuff completely from this patch and leave that to issues like #2966320: Show entity title after autocomplete selection instead of internal route (e.g., /node/123). This patch is already doing too many things.

Adding in this combination of 84 and 77, with some additional checks to try to handle edits after the initial link is created.

In the submission callback:

  if (<linking to entity>)
    <snip>
  }
  elseif (empty($link_element) || isset($link_element['data-entity-type'])
          || (isset($link_element['href']) && ($link_element['href'] != $href_dirty_check))) {
    // No entity but also no manual change? Or maybe we're changing from an
    // entity to a non-entity?
    // Grab the raw path (presumably obtained by a simple matcher, like email),
    // and reset any entity attributes.
    $form_state->setValue(['attributes', 'href'], $href_dirty_check);
    $reset_attributes = TRUE;
  }

This is a mess... but seems to work so far.

Made some changes with the email matching:

1. Removed the "E-mail" text from the suggestions label, to make matching easier and I think that text wasn't that useful anyway.
2. For the link field, added a hidden href field to save the path of the email link there. Since it needs to have the mailto: prefix, but in the label we won't have that.
3. Made some changes in the tests to make them work, according to the ones with the matcher.

Fixed the failing test, forgot to enable the email matcher.

This adds a minor tweak to 93 to do quote htmlentity conversion (the real fix is probably berdir's suggestion above for front-end processing of these characters).

(I don't know what the actual magic entity decode that matches the translation label, but here's another version that uses the top answer on http://php.net/manual/en/function.html-entity-decode.php)

Please @ahebrank we need the interdiff agains the previous patch so we know what has been changed. Could you upload that please?

Won't Gitlab be nice, someday?

It's just this line in the .module file. Originally:

if (strpos(html_entity_decode($href), $translation->label()) !== FALSE)

vs. in 94:

if (strpos(html_entity_decode($href, ENT_QUOTES), $translation->label()) !== FALSE)

or in 95:

if (strpos(html_entity_decode($href, ENT_QUOTES | ENT_XML1, 'UTF-8'), $translation->label()) !== FALSE)

+++ b/src/Utility/LinkitHelper.php
@@ -65,6 +65,11 @@ class LinkitHelper {
 
+    // For now only using this for email links.
+    if (!empty($value['attributes']['href']) && strpos($value['attributes']['href'], 'mailto:') !== FALSE) {
+      return $value['attributes']['href'];
+    }

I think we should always use the path field and drop the logic that resolves the label back to the entity.

Fixing yet another problem, not related to the labels though. Basically what happened was that the linkit autocomplete conflicts with the core autocomplete when it initializing the field. This happened, with fields in paragraphs, but either way this could be a problem with fields directly on nodes as well.
I have a paragraph with a linkit field and another with a field that uses the core's autocomplete widget. Add the paragraph with the linkit first on a node form then the other one. There will be a javascript error in the console, which doesn't really break things, but in the second field with the autocomplete widget, the returned results will be rendered as in the linkit field, which is a problem, because the two should have different designs.
Hopefully this description is clear enough to see what my changes aim to fix.

As suggested dropped out a lot of code which corresponds to showing the labels. So we are back again at showing the paths in the url field.

I tried this patch and I'm having trouble getting it to use the Media Substitution. In LinkitFormatter::getSubstitutedUrl() the PHP URL scheme for a media link is internal, not entity. I am using the default Linkit profile both for the form display and the link field formatter. This is with Linkit 8.x-5.0-beta8.

Looking deeper I see LinkitHelper::getUriFromSubmittedValue() is setting everything except to "internal:".

I modified the previous patch by adding code to getUriFromSubmittedValue() from the patch in comment 99.

Added query parsing. The patches so far only preserved the fragment and the query part wasn't saved.

Added query and fragment support when the user clicks on the popup.

Fixed a regression introduced in #104.

Ok, it turns out users are really confused with the input change, so here's a patch that shows the linked entity in a separate item and always shows the input as a url. It looks like this:

1. Moved the uri processing methods to LinkitHelper.
2. Added a way to alter the url that will be presented in the textfield.
3. Added uri normalization for absolute links that are really local.

@blazey Thank you for the improvements.

IMHO we should keep the scope of this issue limited so that a commitable state is reached. Most importantly, all critical functionality added here was test covered while your additional extensibility points 2. / 3. seem not to be covered. For instance as a regression, the link does not open in a new tab, easily resulting in data loss if the user clicks it... We're now back to WIP.

Thank your for the review, sir! Good point about the link opening in the same tab. It's been addressed in the attached patch. Moreover:

1. A few coding standards problems have been addressed.
2. Some dead code has been removed.
3. A regression in massageFormValues has been fixed. There was a problem when changing an existing value without clicking the autocomplete bubble.
4. A new problem has been identified and fixed. Links to entities entered as aliases in languages other than the current one weren't identified correctly. Now they are and the entity association is saved.

So, to sum up, all of these inputs are now valid

/node/1
alias-to-node-1
/alias-to-node-1
/en/alias-to-node-1
/en/alias-to-node-1?with-a=query#and-fragment
/de/alias-to-node-1-in-a-different-language
http://same-domain.com/de/alias-to-node-1-in-a-different-language?with-a=query#and-fragment

and will be connected to the underlying entity. The decision whether to transform an absolute link to a relative version can be changed with hook_linkit_host_is_internal_alter().

Unfortunately, we don't have any additional resources to add test coverage at this point. We're sharing what we can, though.

Let's trigger testbot.

Fixed the coding standards and the path alias matching logic.

Prevent an exception that was thrown for certain uris in a specific situation (when editing values that were created with the core link widget).

First off, thank you all for the work and patiens on this issue.

I have read all the comments and there is many different things going on here, which is good, but at the same time I'm having a hard time wrapping my head around all of them.

I have tested the latest patch and it seems ok.

Some small things:

• However I'm not sure about the changes in #108. That should be a separate issue I think, do you all agree?

• Most importantly, all critical functionality added here was test covered while your additional extensibility points 2. / 3. seem not to be covered.

  This needs to be fixed.

Also the patch fails as the JS functional test class is extends JavascriptTestBase, but should now extend WebDriverTestBase.

A test is also failing testLinkFieldWidgetAndFormatter

When I tried to enter mailto: link
it prevents me from entering it and it said the path is not valid
is there anyway to fix that ?

I modified the validation so that the email link (mailto:) can be added.

Enable URL directing to add and edit form to be valid

I can confirm #123 addresses validation issues with internal content creation paths.

Select first available linkit profile if there is any configured. (Instead of default)

I'm currently using #118, but it has a major issue with optional link texts:
Given a link field with an optional link text, when you select a node, then the link text field is prefilled with the node label.
A user can override the link text with a custom text, the custom text is stored correctly. But a user can not remove the link text and store an empty value, instead the link text is always filled with the node label on entity save.

Before #118 we have been using patch #59, which didn't have the optional link text issue, but we had to move forward due #78's multi-linguality issue.

I think this problem was introduced in #108 in LinkWidget::massageFormValues

      if (empty($value['title']) && $entity) {
        // Set the title here, otherwise it's set further down the chain with
        // a wrong (default) language.
        $value['title'] = $entity->label();
      }

I believe those lines prevent an optional, empty link text.

+++ b/src/Utility/LinkitHelper.php
@@ -0,0 +1,193 @@
+    if ($is_external) {
+      $host_is_internal = \Drupal::request()->getHost() === $host;
+      \Drupal::moduleHandler()->alter('linkit_host_is_internal', $host_is_internal, $host, $input);
+      if ($host_is_internal) {

I'm not sure why we have to add so much low-level logic and add (undocumented) hooks.

There is \Drupal\Component\Utility\UrlHelper::isExternal() and \Drupal\Component\Utility\UrlHelper::externalIsLocal(), can't we use that?

Again, as before when we've split off features from earlier patches, the more things we add here, the harder it is to get it committed.

I know it's hard because this introduces so many changes that it's very hard to combine it with other patches.

So lets try to keep not strictly related changes/improvements to a minimum and get this committed and then continue in smaller, more focused issues.

As commented in #119, all important functionality here needs to be covered with tests, and the more things we add, the harder that is as well.

As discussed before I removed that rendered 'Linked entity' feature introduced in #108 as it should be a separate feature and also it might conflict with the idea to show labels again in the url field, but this should remain outside of this patch.

Also removed the ability to alter the display url in the linkit widget and the alter to override if the url should be handled as internal or not. As discussed with @berdir, this was probably added for specific use case, but doesn't make much sense to use it as it could introduce some issues if the urls are not properly altered in the right format.

Refactored the changes from #122 as it add a new variable to store the host to check if the url is a mailto link, but there was already a host variable to use.

In the LinkitFormatter I added the linkit profile as a configurable setting, to be able to use profiles other than the default. Also added a description that says that the profile must be the same as on the widget, because different profiles can cause dissimilarities when editing or viewing a content. There was an attempt for this in #125, but the changes there weren't working properly, since it would always select the first available profile, which is mostly the default one.

As pointed out in #126 the link title shouldn't be set even if it's empty.

Set the title here, otherwise it's set further down the chain with a wrong (default) language.

@blazey added this, but I didn't experience such problems and it doesn't make sense to set the title after saving the widget.

The LinkitHelper is more functional now so I didn't change it too much. As it was suggested in #127 I changed the way we detect if the url is external or internal in LinkitHelper::uriFromUserInput().

Changed the FrontPageMatcher a little bit to match the front page by path as well.

Added new tests to test the url converting from an external url using the current website's host to a relative url. And also a test for the front page.

One, sort of new bug that I found is in the file matcher, but it might also make sense to leave this for a separate issue and I think there are already some issues on improving the file matcher. Anyway the problem is matching the relative path to the file, after users choose from the returned list. Searching is done by typing in the filename e.g. new-logo.png, which would return a path like /sites/default/files/new-logo.png. After having this path in the field, the file matcher won't give us any results, because it only searches by the filename.

Tried the patch in comment 128 and it works very well for me. I still haven't put it on prod, but at first glance, it seems to be working correctly.

Setting back to needs work since the tests are failing.

Reroll for latest beta. Only change from #128 is changing js/autocomplete.js to js/linkit.autocomplete.js.

Oops

@KarlShea Interdiff is missing. Setting to Needs Review to trigger testbot.

This patch appears to mostly work, but I noticed that if you have multiple query params of the same name that even though its saved in the database exactly as you entered it, when it is output on the page, you only get one of those parameters.

For example:

/search?facet=foo:bar&facet=baz:foobar&page=10&resultsPerPage=15

Displays on the page as:

/search?facet=baz:foobar&page=10&resultsPerPage=15

How can I change the behavior to not do that?

Fixed the failing test, which was caused by the test case not finding page titles even though the title is present. So I removed those checks completely, since we have other checks to make sure we are on the right page.

Fixed a PHP warning in the FrontPageMatcher, when the url doesn't have a query or a fragment.

@kevinquillen What you discovered above is not an issue with this patch or module it is a core issue as of my testing. I think this is the issue for the bug: #2885351: Query string duplications

For the title test fail, see: https://www.drupal.org/project/drupal/issues/2870453#comment-12102425

If you change titleEquals to

  /**
   * Pass if the page title is the given string.
   *
   * @param string $expected_title
   *   The string the page title should be.
   *
   * @throws \Behat\Mink\Exception\ExpectationException
   *   Thrown when element doesn't exist, or the title is a different one.
   */
  public function titleEquals($expected_title) {
    $title_element = $this->session->getPage()->find('css', 'title');
    if (!$title_element) {
      throw new ExpectationException('No title element found on the page', $this->session->getDriver());
    }
    $actual_title = $title_element->getHtml();
    $this->assert($expected_title === $actual_title, 'Title found');
  }

(so ->getText() changed to ->getHtml()) the test is green. Not using that assertion in a JS test seems best I guess, or open a core issue to update titleEquals on \Drupal\FunctionalJavascriptTests\JSWebAssert

Fixed the last test fail due to the test bot running in a subdirectory and the front page matcher test expected to see just /.

Not using that assertion in a JS test seems best I guess...

Regarding the title tests, I'm going with this option from @lendude's comment.

+++ b/tests/src/FunctionalJavascript/LinkFieldTest.php
@@ -314,7 +314,7 @@ class LinkFieldTest extends WebDriverTestBase {
 
-    $this->assertEquals('/', $url_input->getValue());
+    $this->assertEquals('/subdirectory/', $url_input->getValue());
     // Check that the title was populated automatically.

We can't hardcode it, because now it fails when you don't have a subdirectly.

I'm also not sure that the fix needs to be in the test, it's more likely that the actual code is broken. The link field should always be relative to the drupal root, so the parsing logic need to make sure to not store the directly.

You are right about hardcoding, now I'm loading that URL from the route. The parsing logic doesn't have any problem with this though, because the drupal root will contain the subdirectory, if your server is setup like that.

Tested it locally and got the same results. Put drupal into a subdirectory e.g. /var/www/html/subdirectoryand setup apache virtualhost to have the doc root one directry above drupal's root i.e. /var/www/html and then Drupal's root url will be localhost/subdirectory/.

Had a discussion with @berdir and the problem is using Url::fromRoute('<front>')->toString() which is good for display, but shouldn't be used for storing data. So my previous comment is irrelevant, changed back the test to check for / and in the FrontPageMatcher also saving that directly instead of using Url::fromRoute('<front>')->toString().

Hi

When I try and add a tel: link to a link field using linkit, I receive the following error

InvalidArgumentException: A path was passed when a fully qualified domain was expected. in Drupal\Component\Utility\UrlHelper::externalIsLocal() (line 265 of /app/web/core/lib/Drupal/Component/Utility/UrlHelper.php).

I am experiencing the issue on several sites. Is anyone else experiencing a similar issue with the latest patch?

Thanks
Dan

Im really not sure what the below code is trying to do, I have removed it in the attached patch because it is causing problems for me. Looking back it seems it was introduced in #108 without too much explanation.

If you add a page view with a contextual filter and a example path of "example-page/%taxonomy_term" where %taxonomy_term is a named route parameter, and then you try and link to this view using a link field with the linkit widget and an example link "/example-page/term1"

The below code finds the route correctly as a view and then it starts looking at the routes parameters which returns taxonomy_term and this results in an error whilst trying to save. Ideally it would just leave this manual link alone entirely.

I'm struggling to see why it is looking at what entity type a route's parameters might be. Does anyone have an example of when this might be useful?

+    try {
+      $route_name = Url::fromUri($input)->getRouteName();
+      if ($route_name != "entity.node.edit_form" && $route_name != "node.add") {
+        $params = Url::fromUri($input)->getRouteParameters();
+        $possibly_an_entity_type = key($params);
+        $entity = \Drupal::entityTypeManager()
+          ->getStorage($possibly_an_entity_type)
+          ->load($params[$possibly_an_entity_type]);
+        return \Drupal::service('entity.repository')
+          ->getTranslationFromContext($entity);
+      }
+    }
+    catch (\Exception $e) {
+      // Or not.
+    }
+
+    return NULL;
+  }

@kevinquillen that is a bug in PHP's parse_str function. See #3038774: Url only outputs the last value of a query parameter for details.

Small d9 compatibility related change. Patches from 143 or 144 did not apply and wasn't quite sure about the reroll, so this patch builds atop of #140.

Overlooked the form - view difference.

We noticed that submitting invalid input as link can lead to fatals on save, ie: tel:0123456. Adding that to patch.

Note: The deprecation updates here require 8.8, so that means this is basically blocked on #3042631: Drupal 9 Deprecated Code Report for LinkIt module, which in turn is blocked on the maintainer deciding to drop 8.7 support.

Ignore this comment, this was an issue in my local

I am trying to apply the patch #148 to linkit-8.x-5.x-dev, The files are getting added but final status is failed. Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2020-01-30/linkit_for_link_field-271.... I am using composer to apply patch. Adding patch entry in composer.json and then executing composer install

After changing the link field to linkit the Max Length & Count down messages were missing from widget settings. After exporting the config it got removed from YML file also. Seems linkit not inheriting properties like Max length & Count down messages from Link field.

Noticed the tel: case is not saved correctly. Not sure though if we should make this more general.