Problem/Motivation

LinkItem::getUrl doesn't respect any options it may hold.
This prevents us from being able to write an enhanced widget for setting attributes etc.
See https://www.drupal.org/project/link_attributes for one such widget.

Proposed resolution

Do so

Remaining tasks

Review

User interface changes

None

API changes

None

Data model changes

None

CommentFileSizeAuthor
#19 link-options-null-2760557.patch540 bytesBerdir
#7 2760557-link-options.2.patch1.33 KBlarowlan
#3 2760557-link-options.pass_.patch1.42 KBlarowlan
#3 2760557-link-options.fail_.patch842 byteslarowlan
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

larowlan created an issue. See original summary.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Related issues: +#2174435: Port Menu attributes to Drupal 8
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext for amaysim Australia Ltd. commented
Status: Active » Needs review
Issue tags: +Quickfix
FileSize
2760557-link-options.fail_.patch842 bytes
2760557-link-options.pass_.patch1.42 KB

Simple bug

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented 
+++ b/core/modules/link/src/Plugin/Field/FieldType/LinkItem.php
@@ -169,7 +169,11 @@ public static function mainPropertyName() {
+    $url = Url::fromUri($this->uri);

Why not just pass the options as 2nd param?

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext for amaysim Australia Ltd. commented
Issue tags: +needs backport to 8.1.x

The last submitted patch, 3: 2760557-link-options.fail_.patch, failed testing.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext for amaysim Australia Ltd. commented
FileSize
2760557-link-options.2.patch1.33 KB

jibran++

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext for amaysim Australia Ltd. commented
Issue summary: View changes
Issue tags: +Contributed project blocker

Blocks https://www.drupal.org/project/link_attributes

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented
Version: 8.2.x-dev » 8.1.x-dev
Status: Needs review » Reviewed & tested by the community
Issue tags: -needs backport to 8.1.x

Nice and easy thanks @larowlan. For D8 it's forwardport policy.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented

Just a shameless ad about some other issue :) #2656534: Provide a way to extract a menu plugin from a menu form

This looks perfect for me.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented
Related issues: +#2477155: Update LinkWidget to accommodate the attribute., +#2656534: Provide a way to extract a menu plugin from a menu form

I have another ad as well #2477155: Update LinkWidget to accommodate the attribute.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 7: 2760557-link-options.2.patch, failed testing.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented
Status: Needs work » Reviewed & tested by the community

Everything is green.

alexpott’s picture

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

Committed and pushed f94a4c2e13a6d77d757a1d396268c0764725c093 to 8.2.x and 8a7eab2 to 8.1.x. Thanks!

  • alexpott committed f94a4c2 on 8.2.x 
    Issue #2760557 by larowlan, jibran: LinkItem::getUrl ignores the value...

  • alexpott committed 8a7eab2 on 8.1.x 
    Issue #2760557 by larowlan, jibran: LinkItem::getUrl ignores the value...

Status: Fixed » Closed (fixed)

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

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

Interesting, this breaks an older D8 site that I have pretty badly. For some strange reason, I have N; (serialized NULL) in the options column for my shortcuts. passing NULL along results in a recoverable fatal error.

Looking a bit more, I can see that in at least 4 older D8 sites, some are empty arrays, some NULL. Expecting them all to choke on this.

Quick patch to prevent that, need one for composer, will open a new issue later.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
FileSize
link-options-null-2760557.patch540 bytes
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

#2787183: LinkItem::getUrl() fails if options are NULL