Link field should accept protocol-relative URLs

I really would like to see "example.com" as a valid external URL, yet the implementation with the validation we have today for internal URLs would not be so easy. We need to discuss the validation strategy for that.

Patch to accept URLs starting with //. I still think we should accept "example.com".
I did not change the help messages, as in the original issue some people said it could be confusing for end users.
Add credit for dawehner, lluvigne.

This works for me. Do you think we need something on top of that?

@Wim Leers I thought that being more verbose would be better for code readability. I also improved the code at file LinkExternalProtocolsConstraintValidator.php to be easier to understand at the first look.

@dawehner what I think that needs to be done on top of hat is accept example.com as a valid URL. The problem on that is about the autocomplete and validation. Right now it would be validated as a content title. We need to discuss how we would handle this.

For example, I do not think it would be a good idea to ping an external host in order to validate it. On the other hand, if it is not validated as a content title are we going to just validate it with something like this UrlHelper::isValid() and check if it uses an allowed protocol? How about the scenario of a content title being a valid external URL?

Sorry, I sent the wrong patch file while doing some tests and it was missing the strict operator.

1. +++ b/core/modules/link/src/Plugin/Field/FieldFormatter/LinkFormatter.php
   @@ -179,6 +179,11 @@ public function viewElements(FieldItemListInterface $items, $langcode) {
   +      if ($url->isExternal() && (strpos($url->getUri(), '//'))) {
   +        $link_title = ltrim($link_title, '//');
   +      }
   

   In fact, we do not need this 'if', as the ltrim() would work fine anyway and I cannot think about another case which a URL could come with leading '//'. Shall we keep it for readability?

2. +++ b/core/modules/link/src/Plugin/Validation/Constraint/LinkExternalProtocolsConstraintValidator.php
   @@ -39,9 +39,19 @@ public function validate($value, Constraint $constraint) {
   +        // Protocol-relative URLs start with '//'.
   +        $not_protocol_relative = strpos($url->getUri(), '//');
   

   It was interesting to see the tests passing even when it returns 'FALSE' here, but I think we should not worry about it.

Attached patch with the strict operator.

@Mac_Weber I'm just an interested bystander, but it seems to me that given that issues you raised with example.com, that might be worth being a separate parallel or follow-up issue to this one. Given that supporting protocol-relative URLs is semi-controversial (based on the previous issue) it might be worth splitting these two ideas to see if one generates more consensus than the other.

I get the interest in addressing both of these at once given we're talking about updating the same functions here, so this is just a suggestion.

@dawehner what I think that needs to be done on top of hat is accept example.com as a valid URL. The problem on that is about the autocomplete and validation. Right now it would be validated as a content title. We need to discuss how we would handle this.

This is something we should explore in an additinional issue as @rootwork said.

Rerolled.

Made a minor change of short array syntax,for the #15 patch.

index b0928b0..34d20cc 100644
--- a/core/modules/link/src/Plugin/Field/FieldFormatter/LinkFormatter.php

--- a/core/modules/link/src/Plugin/Field/FieldFormatter/LinkFormatter.php
+++ b/core/modules/link/src/Plugin/Field/FieldFormatter/LinkFormatter.php

+++ b/core/modules/link/src/Plugin/Field/FieldFormatter/LinkFormatter.php
@@ -179,6 +179,11 @@ public function viewElements(FieldItemListInterface $items, $langcode) {

@@ -179,6 +179,11 @@ public function viewElements(FieldItemListInterface $items, $langcode) {
       $url = $this->buildUrl($item);
       $link_title = $url->toString();
 
+      // If the external URL starts with '//', trim it.
+      if ($url->isExternal() && (strpos($url->getUri(), '//') === 0)) {
+        $link_title = ltrim($link_title, '//');
+      }
+

We don't have tests for this change.

@dawehner test was in the patch file. I'm sending it separated.

I had forgotten to change the status.

Setting the correct status, as the patch on #16 passed the tests.

Setting the correct status, as the patch on #16 passed the tests.

The test is just for the validator, but not for the link formatter.

Needs tests as said in #22 and also depending of the field config it displays a form error or it is saved but never displayed in the node view.

@dawehner @Wim Leers just a side note: if we really want this feature we should change or get rid of the HTML5 validation.

Related issue about HTML5 validation modifier: #2861316: Display meaningful HTML5 validation error message

I just tried today and found that "//example.com" can be saved now.
Did we keep this issue open because we still need to discuss accepting "example.com" without double slash as a valid external URL?

yes, also don't see mention of where accepting example.com was forked to a new issue - so is it still being handled under this one?

Can someone please point me to the issue discussing the fix that allows example.com to be entered?

For those stumbling upon this trying to figure out why Drupal doesn't have a proper Link field; I posted a fix here: https://drupal.stackexchange.com/questions/300471/how-to-automatically-a...

@liquidcms This has been discussed here: https://www.drupal.org/project/drupal/issues/1473220 with no solution to handle HTML5 validation.

The HTML5 url validation is not allowing to enter a url that has a pattern "//example.com" and unless it passes the html5 validation the patch won't work here.

my solution would be to change the type of field from "url" to "text" and add some pattern validation in case the url in external.

thanks @vakulrai.. yes, my solution (on existing link fields) likely only works as we have applied the patch which disabled html5 form validation (as it doesn't really work very well).

Setting to NW because a test is needed.

Is #2195983: Support scheme-relative URLs a duplicate issue?

Edit: Nevermind, it appears that tackles the base component. Hopefully it is still helpful to link here.

Yes, let's add #2195983: Support scheme-relative URLs to the related list.

this is another related issue: #2031149: Add support for additional protocols in Link field definition

Re-rolling patch for 10.1.x.

There's another duplicate that was filed as a bug report due to protocol-relative URLs being accepted input that were validated as internal URLs, but resulting in malformed links when rendered: #2889658: Handle protocol-relative URLs in LinkWidget.

I'm closing this in favor of #2889658: Handle protocol-relative URLs in LinkWidget since this issue evolved into being a bug. The changes in that issue are nearly identical in function. Credit has been granted for this issue.