Add support for additional protocols in Link field definition

Postponing feature requests as of the allowed beta changes https://www.drupal.org/contribute/core/beta-changes

Drupal 8-beta 1 has been released.

Drupal 8-beta 1 has been released.

Actually we are on Beta 15 now. Please read the policy at https://www.drupal.org/contribute/core/beta-changes

Yup.. I was just going on why this was postponed initially. Totally good to move this to 8.1 and postpone it as something to tackle there.

I'm currently working on a solution to allow for a custom protocol whitelist, specified in the field settings. There won't be any custom validation just yet. I'd suggest handling that in a follow-up issue.

I updated the title as per my previous comment.

Even better would be a option in the field configuration to set which scheme names are valid for this field.

This patch adds a new configuration option that takes a comma-separated list of additional protocols, which are added to the default whitelist.

Looks like I forgot to add the new configurable to the schema.

Updated IS (while waiting for test results).

Ensured that the whitelist is actually an array if unset.

Hopefully this does the trick. I updated some expected test results, and also moved some new code to its own method to make things cleaner.

This adds the extra blank line I missed.

I can't fix the build failure, but assuming that clears up on its own, we should now be left with the Drupal\Tests\link\Functional\LinkFieldUITest::testFieldUI error:

The string "Saved link 1 16 0 1 configuration." was not found anywhere in the HTML response of the current page.

Anyone have any ideas?

Through my own testing, the above patches won't work because the storage is assuming a string, when it should actually be an array.

1. whitelist -> allowlist
2. May need a reroll for D9 if the retest fails.

Re-rolling for 9.1.x and handling #32.1

Triggering the tests

Missed a few instances

Here is the patch from #35 rebased onto 8.9.x and 9.0.x (no interdiff because no changes).

I have an issue with #35, it does not apply to Drupal 9.1.5. Added fixes to the #35 patch for support Drupal 9.1.5.

A re-rolled patch against 9.2.x-dev.

The patch in #39 works as expected and allows me to add additional protocols as part of the field configuration.

Marking as RTBC, as this successfully applies and works as requested in the original ticket description.

Sorry, this is still work to do here.

The Issue Summary and the tags state that this needs tests and I do not see a test of the new schema property, 'protocol_allowlist'. The patch in #39 is for 9.2.x instead of 9.3.x and it is not passing the commit code check.

The new property name 'protocol_allowlist' is a unusual, all the other instances in core of something that is allowed begins with the word allow. Therefor I think the name should be 'allowed_protocols'. And this change will require an Issue Summary update.

1. +++ b/core/lib/Drupal/Component/Utility/UrlHelper.php
   @@ -206,11 +206,13 @@ public static function encodePath($path) {
   +   *   A list of extra protocols (strings) that have been explicitly allowed.
   

   s/array/string[]/

2. +++ b/core/lib/Drupal/Component/Utility/UrlHelper.php
   @@ -206,11 +206,13 @@ public static function encodePath($path) {
   +   * @param array $protocol_allowlist
   

   s/array $protocol_allowlist /string[] $allowed_protocols/

   There are many, many places in the patch where $protocol_allowlist should be changed.

3. +++ b/core/lib/Drupal/Component/Utility/UrlHelper.php
   @@ -206,11 +206,13 @@ public static function encodePath($path) {
   +   *   A list of extra protocols (strings) that have been explicitly allowed.
   

   I think custom is better than extra and there is no need to add strings, the hint is above.

   'The list of custom allowed protocols'.

4. +++ b/core/lib/Drupal/Component/Utility/UrlHelper.php
   @@ -344,13 +346,15 @@ public static function setAllowedProtocols(array $protocols = []) {
       * @see \Drupal\Component\Utility\Html::escape()
   

   As above

5. +++ b/core/modules/link/src/Plugin/Field/FieldType/LinkItem.php
   @@ -112,6 +122,25 @@ public function fieldSettingsForm(array $form, FormStateInterface $form_state) {
   +   * Element validate; ensure that the Protocol Allowlist setting is valid.
   

   This feels clumsy. How about, "Validate the format of the allowed_protocol list.'

   It would probably be a good idea to add a sentence that this is not validating the protocols. The user can enter meaningless strings here.

6. +++ b/core/modules/link/src/Plugin/Field/FieldType/LinkItem.php
   @@ -112,6 +122,25 @@ public function fieldSettingsForm(array $form, FormStateInterface $form_state) {
   +  public static function validateSettingsFormElementProtocolAllowlist($element, FormStateInterface $form_state, $form) {
   

   Not convinced the comment is needed here and the code can be simpler. One option is

       $allowed_protocols = array_map('trim', explode(',', $element['#value']));
       $allowed_protocols = $allowed_protocols == [''] ? [] : $allowed_protocols;
       $form_state->setValueForElement($element, $allowed_protocols);

This also changes the user interface for the link field settings so a screenshot before and after is needed. The screenshots needs to be added to the Issue Summary to help reviewers and committers, thanks.

Updated the patch in #39 to address all the changes provided in #43. Also fixed the custom command failure errors shown in #39. Kindly review the changes made in this patch.

Thank you!

Fixed custom command failed for unknown word. patch added and interdiff attached. Please review

The CSpell check failed in core/modules/link/src/Plugin/Validation/Constraint/LinkExternalProtocolsConstraintValidator.php file, so updated the patch accordingly.

Thank you!

Patch from #47 doesn't apply to D9.4 (due to the smallest change in Umani demo config files).

This patch works for 9.4. Haven't tested with 9.5 but suspect it works there as well.

Seems to have a test failure

Left some comments on MR.

Previously tagged for issue summary and screenshots.

Leaving tests tag as they will be needed for post_update hook.