When #asd is entered on the UI into a link field's URL, it gets stored as internal:#asd. When the default "Link" field formatter is used, it gets rendered as <a href="">whatever title</a>. Expected result would be <a href="#asd">whatever title</a>.

CommentFileSizeAuthor
#6 drupal-link_with_in_page-2717401-6.patch8.9 KBboobaa
#6 drupal-link_with_in_page-2717401-tests_only-6.patch8.31 KBboobaa
#3 drupal-link_with_in_page-2717401-3.patch663 bytesboobaa

Comments

Boobaa created an issue. See original summary.

boobaa’s picture

boobaa
he/him
commented
Related issues: +#2054011: Implement built-in support for internal URLs

I found the offending line in \Drupal\link\Plugin\Field\FieldFormatter::buildUrl(): $options = $item->options; should be $options = $url->getOptions(); and this change seems to fix the problem (patch is coming). This line was introduced in #2054011: Implement built-in support for internal URLs.

boobaa’s picture

boobaa
he/him
commented
Status: Active » Needs review
StatusFileSize
new drupal-link_with_in_page-2717401-3.patch663 bytes

Let's see an initial testbot run. (Initial, because I don't think there are tests for this case yet.)

Status: Needs review » Needs work

The last submitted patch, 3: drupal-link_with_in_page-2717401-3.patch, failed testing.

boobaa’s picture

boobaa
he/him
commented
Assigned: Unassigned » boobaa

Okay, so neither the item nor the url options are enough by themselves. Working on a fixed patch with tests as well.

boobaa’s picture

boobaa
he/him
commented
Assigned: boobaa » Unassigned
Status: Needs work » Needs review
StatusFileSize
new drupal-link_with_in_page-2717401-tests_only-6.patch8.31 KB
new drupal-link_with_in_page-2717401-6.patch8.9 KB

A patch with tests only (failing where it should on my local) and a patch with test and the fix (passing on my local).

The last submitted patch, 6: drupal-link_with_in_page-2717401-tests_only-6.patch, failed testing.

jan.mashat’s picture

jan.mashat commented
Status: Needs review » Reviewed & tested by the community
dawehner’s picture

dawehner
Primary language German
commented

Looks properly for me! I'm glad we have test coverage for it now!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

@jan.mashat generally an rtbc requires some evidence that you've reviewed the patch and thought about any consequences. This rtbc makes me suspect you and @Boobaa work for the same company.

+++ b/core/modules/link/src/Plugin/Field/FieldFormatter/LinkFormatter.php
@@ -242,6 +242,7 @@ protected function buildUrl(LinkItemInterface $item) {
     $options = $item->options;
+    $options += $url->getOptions();

This looks fine. $item->options has to be an array and we're going to call $url->setOptions() later so this is kind of a merge. $url->getOptions() will always return an array too and we should use any options set in $item->options over the url options. So this looks good to go.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 6b947a6 and pushed to 8.1.x and 8.2.x. Thanks!

  • alexpott committed b953606 on 8.2.x 
    Issue #2717401 by Boobaa: Link with in-page fragment only is rendered...

  • alexpott committed 6b947a6 on 8.1.x 
    Issue #2717401 by Boobaa: Link with in-page fragment only is rendered...

Status: Fixed » Closed (fixed)

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