Problem/Motivation

The link module does not allow to insert aliases. The expected behavior is as with menu module in D7 where the alias is matched to the system path, which is then stored and the user is notified it has stored the system path instead of the alias.

Proposed resolution

In the massageFormValues() use alias manager to match the path prior to creating the url object.

Remaining tasks

-

User interface changes

Identify the case when user input is alias and display message on the save action.

API changes

-

Blocking

CommentFileSizeAuthor
#7 link-2231413-7.patch1.88 KBblueminds
#7 link-2231413-7.interdiff.txt1.76 KBblueminds
#2 link-2231413-2-PASS.patch1.87 KBtim.plunkett
#2 link-2231413-2-FAIL.patch833 bytestim.plunkett

Comments

blueminds’s picture

blueminds commented
Related issues: +#2054011: Implement built-in support for internal URLs
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Active » Needs review
StatusFileSize
new link-2231413-2-FAIL.patch833 bytes
new link-2231413-2-PASS.patch1.87 KB

Seems the widget isn't injecting anything yet, so sticking with a service call for now.

The last submitted patch, 2: link-2231413-2-FAIL.patch, failed testing.

blueminds’s picture

blueminds commented

Tested, works for me. Tests are fine.

We just miss the message (as in D7) saying that the provided path was stored as a system path. Not sure if we need it here as well, moreover the only place for it seems to be massageFormValues() that is probably not the right place for it.

For me RTBC.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented 
  1. +++ b/core/modules/link/lib/Drupal/link/Plugin/Field/FieldWidget/LinkWidget.php
@@ -207,6 +207,12 @@ public function massageFormValues(array $values, array $form, array &$form_state
+          // If the widget supports internal links, ensure that if this is a
+          // path alias that the system path is stored.

    "If internal links are supported, look up whether the given value is a path alias and store the system path instead." 

  2. +++ b/core/modules/link/lib/Drupal/link/Plugin/Field/FieldWidget/LinkWidget.php
@@ -207,6 +207,12 @@ public function massageFormValues(array $values, array $form, array &$form_state
+          if ($this->supportsInternalLinks() && !UrlHelper::isExternal($parsed_url['path'])) {

    I'm confused - how can the parsed 'path' be external?

    Shouldn't this check whether the given $value['url'] is external?

  3. +++ b/core/modules/link/lib/Drupal/link/Tests/LinkFieldTest.php
@@ -103,12 +103,15 @@ function testURLValidation() {
+    \Drupal::service('path.crud')->save('admin', 'a/path/alias');

    Just in case it is committed first:

    Note that #2208631: Rename \Drupal\Core\Path\Path to \Drupal\Core\Path\AliasStorage renames this service to path.alias_storage

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs review » Needs work

NW as per #5

blueminds’s picture

blueminds commented
Status: Needs work » Needs review
StatusFileSize
new link-2231413-7.interdiff.txt1.76 KB
new link-2231413-7.patch1.88 KB

Implemented comments from #5.

amateescu’s picture

amateescu commented
Status: Needs review » Reviewed & tested by the community

Oops, quite an oversight from original issue that added internal url support :/ The review from #5 is correct and the last patch looks good to go.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 7: link-2231413-7.patch, failed testing.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Status: Needs work » Reviewed & tested by the community

Patch at #7 still applies, and testbot its green. Looks like random failure in HEAD?

yesct’s picture

yesct commented
Issue summary: View changes
Related issues: +#2235457: Use link field for shortcut entity

this is blocking #2235457: Use link field for shortcut entity

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Category: Task » Bug report
Priority: Normal » Major

#2235457: Use link field for shortcut entity is blocked on this and is major, so this should be major, too.

Also, this is more of a bug than a task.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

  • Commit 1efd2ad on 8.x by webchick: 
    Issue #2231413 by blueminds, tim.plunkett: Link field does not accept a...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.