Problem/Motivation

In a Link field's settings configuration page, it is impossible to set a default value for the "Link text" subfield while leaving the URL subfield with no default.

One use case for doing this is to have preset link text for consistency while expecting the URL to change per entity. Consider the example of a website about books. Each book is a node with a link field. The default text of these links might be "Search for this book at nearby libraries" while the URL must be customized for each individual book. This would save editing time and give some guarantee of consistency across nodes.

Site builders requiring default link text functionality are advised to use the Fixed text link formatter module while this issue is in progress or as a permanent solution.

Steps to reproduce

  1. Add a link field to a content type.
  2. The link field is configured with "Allow link text" set to be required.
  3. Under Default Value, enter text for the "link text".
  4. Leave the "default URL" empty.
  5. Save the field configuration.

Edit the link field and see that the default link text has not been saved.

Proposed resolution

The problem here is that LinkItem::isEmpty() returns true if the item does not have an URI, then WidgetBase::extractFormValues() calls filterEmptyItems() and that removes the items that only have a text but not an URI.
This behavior is wanted when a user is filling the form normally but not when setting the default value.
However, we lack the context in LinkItem::isEmpty() to know if we are in the default value form or not.

The proposed solution involves overriding extractFormValues() in LinkWidget and not to call filterEmptyItems() if we are in the default value form. This allow settings the default value in the config but then this value is still not used in the real form.

Remaining tasks

Usability review, focusing on the fact that now a field not marked required is required.
Review
Commit

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#157 2708101-157.patch6.77 KBmichaelsoetaert
#156 2708101-156.patch7.6 KBmichaelsoetaert
#143 Screenshot 2024-12-06 at 17-43-39 Link settings for Basic page dev0-web.png22.56 KBquietone
#141 After link.mp41.17 MBsagarmohite0031
#141 Before Link.mp41.26 MBsagarmohite0031
#138 2708101-nr-bot.txt2.58 KBneeds-review-queue-bot
#130 default_value_for_link-2708101-130.patch7.88 KBemerham
#127 2708101-nr-bot.txt1.87 KBneeds-review-queue-bot
#120 default_value_for_link-2708101-120.patch7.78 KBrenukakulkarni
#112 default_value_for_link-2708101-112.patch8.06 KBshivansunfire
#101 After Patch.png355.09 KBroshanibhangale
#101 Before Patch.png348.45 KBroshanibhangale
#98 default_value_for_link-2708101-98.patch16.83 KBshivansunfire
#94 After Patch 2708101 up.png311.56 KBchetanbharambe
#93 after-patch-2708101--2.png84.82 KBvikashsoni
#93 after-patch-2708101-1.png31.44 KBvikashsoni
#89 2708101-89.patch7.5 KBranjith_kumar_k_u
#88 2708101-88.patch7.38 KBomrmankar
#87 default_value_for_link-2708101-86.patch7.65 KBshivansunfire
#85 default_value_for_link-2708101-85.patch7.61 KBshivansunfire
#79 After Patch 2708101.png365.68 KBchetanbharambe
#79 Before Patch 2708101.png345.05 KBchetanbharambe
#78 interdiff_73-78.txt629 bytesneslee canil pinto
#78 2708101-78.patch7.38 KBneslee canil pinto
#73 default_value_for_link-2708101-73.patch7.31 KBvakulrai
#72 interdiff-2708101-68-72.txt5.71 KBvakulrai
#72 default_value_for_link-2708101-72.patch7.31 KBvakulrai
#70 Screenshot 2021-03-26 at 16.30.54.png37.86 KBtarik.cipix
#70 Screenshot 2021-03-26 at 16.28.46.png19.71 KBtarik.cipix
#68 drupal-2708101-68.patch2.79 KBprudloff
#48 Screenshot-Link-text-2708101.png149.62 KBdani3lr0se
#28 1251x938_26_05_2016_img-02.png69.57 KBpifagor
#28 1079x856_26_05_2016_img-01.png48.65 KBpifagor
#27 drupal-fix_bug_link_module-2708101-27.patch883 bytespifagor
#21 Screenshot from 2016-05-23 17_52_43.png36.84 KBamit.drupal
#21 interdiff-20-13-20.txt1.17 KBamit.drupal
#21 drupal-fix_bug_link_module-2708101-20.patch1.52 KBamit.drupal
#13 drupal-fix_bug_link_module-2708101-13.patch877 bytespifagor
#6 drupal-fix_bug_link_module-2708101-6.patch665 bytespifagor
#5 drupal-fix_bug_link_module-2708101-5.patch608 bytespifagor

Issue fork drupal-2708101

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

JasonSafro created an issue. See original summary.

clairedesbois@gmail.com’s picture

I confirm the problem.

If I add a default URL and a default link text, values are save.
If I add only a default URL without default link text, it works too.
But If I add only a default link test without the URL, the default link text isn't save.

pifagor’s picture

Version: 8.0.6 » 8.1.0
pifagor’s picture

Assigned: Unassigned » pifagor
pifagor’s picture

StatusFileSize
new608 bytes

Patch

pifagor’s picture

StatusFileSize
new665 bytes

Or

pifagor’s picture

Status: Active » Needs review
pifagor’s picture

Assigned: pifagor » Unassigned

The last submitted patch, 5: drupal-fix_bug_link_module-2708101-5.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 6: drupal-fix_bug_link_module-2708101-6.patch, failed testing.

pifagor’s picture

Assigned: Unassigned » pifagor
andypost’s picture

+++ b/core/modules/link/src/Plugin/Field/FieldType/LinkItem.php
@@ -147,7 +147,7 @@ public static function generateSampleValue(FieldDefinitionInterface $field_defin
-    $value = $this->get('uri')->getValue();
+    $value = !empty($this->get('uri')->getValue()) ? $this->get('uri')->getValue() : $this->get('title')->getValue();

main property of the field is *uri* so title should be validated additionally

pifagor’s picture

StatusFileSize
new877 bytes

Add patch for validate

pifagor’s picture

Status: Needs work » Needs review
pifagor’s picture

Assigned: pifagor » Unassigned
alexpott’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

@pifagor there's no need to contact me through my d.o contact form to ask for patch review. As this is fixing a bug it needs tests.

pifagor’s picture

Status: Needs work » Needs review
pifagor’s picture

Status: Needs review » Needs work
pifagor’s picture

Status: Needs work » Needs review
amit.drupal’s picture

#13 patch is not working.

amit.drupal’s picture

Add patch for validate.

Status: Needs review » Needs work

The last submitted patch, 21: drupal-fix_bug_link_module-2708101-20.patch, failed testing.

pifagor’s picture

@amit.drupal
#13 patch is not working.
Please argue.
After https://www.drupal.org/files/issues/Screenshot%20from%202016-05-23%2017_... a screenshot from my patch. And it does not keep the title, without url. What is logical and correct.

pifagor’s picture

Status: Needs work » Needs review
sushantpaste’s picture

Issue summary: View changes

#13 is not working.
@pifagor I think that patch will only work for you as may be your link field name is "field_link", it will not work for others as they can have different field name.

You need to change below line in your patch

 if (isset($element['#parents'][2]) && !empty($values['field_link'][$element['#parents'][2]]['title'])) {

with

 if (isset($element['#parents'][1]) && !empty($values[$element['#parents'][1]][0]['title'])) {

hope this helps.

sushantpaste’s picture

Status: Needs review » Needs work
pifagor’s picture

Version: 8.1.0 » 8.1.1
Status: Needs work » Needs review
StatusFileSize
new883 bytes

@sushantpaste thank you for your comments
URL field is required for the title. For functional module link title field will not be saved, because is a must. Your code is valid only for a clear field. If the field is meaningful, your code is incorrect.
Fixed patch it should work

pifagor’s picture

StatusFileSize
new48.65 KB
new69.57 KB

Added screenshots, evidence that the patch works

pifagor’s picture

Issue summary: View changes
pifagor’s picture

pifagor’s picture

pifagor’s picture

Issue summary: View changes
amit.drupal’s picture

#27 Is Working Fine for me .

sushantpaste’s picture

Status: Needs review » Reviewed & tested by the community

#27 Works fine. Marking it as RTBC.

pifagor’s picture

Issue tags: -Needs tests +requires transfer
eric_a’s picture

Version: 8.1.1 » 8.1.x-dev
Status: Reviewed & tested by the community » Needs work
Issue tags: -requires transfer +Needs tests

@pifagor, @sushantpaste, the tag "Needs tests" refers to tests in code. For manual testing we have "Needs manual testing". So somebody needs to add a test to the patch and then both the test and the plain fix must be reviewed.

pifagor’s picture

Status: Needs work » Needs review
mac_weber’s picture

Status: Needs review » Needs work

@pifagor please read #36. Change the issue issue status only after adding tests in code.

minnoce’s picture

Version: 8.1.x-dev » 8.1.3
mac_weber’s picture

@minnoce please keep issues on development version and ask before doing changes if you are not sure.
Read the guidelines at https://www.drupal.org/node/314328#version

Choose the current development version of the module, theme, or Drupal itself. Don't just pick the version you noticed the problem in. For more information, see Drupal's backport policy.

mac_weber’s picture

Version: 8.1.3 » 8.1.x-dev
minnoce’s picture

Sorry, I haven't read carefully the guidelines.
Clearly, I noticed this bug in 8.1.3 (vanilla freshly installed on a Lamp).

Thanks

Ilesyt’s picture

Hello, I am creating a website where I put in a link field in a content type.
I want authors to be able to put in a link but not add link text as it should be a default text (such as "Check it out").
The links are outgoing.
So when working with the link field, I stumbled upon this issue.
If I disable the link text (because I don't want others messing with it) and still input a value (on field creation), it will disappear.
If I enable link text (although I do not want to), if I don't put in an URL value, the link text disappears.
Only if I put in a default URL value, does the default link text stay.

I would like an option where could I input a link text in the 'Manage display' when editing the field display.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

pifagor’s picture

Version: 8.4.x-dev » 8.5.x-dev
Status: Needs work » Needs review
dani3lr0se’s picture

Status: Needs review » Needs work
StatusFileSize
new149.62 KB

I'm sorry, but I'm a little confused. Is this patch supposed to allow adding default link text without a URL? If so, it's not working for me. I tested #27 on 8.5.x and it still requires a URL if I want default link text. See attached screenshot. I mean, I understand that it won't work unless there is a URL because why would we want link text to display without a URL? But, if this makes it so the link text only displays if there is a URL, then that's understandable. Sorry if I sound snarky or anything. I don't mean to be. I think this is an interesting issue, especially for use cases such as #43 above. Thanks for working on this. :)

pifagor’s picture

Status: Needs work » Needs review

The link cannot be without a URL. Can be a link without text.
See please - https://www.drupal.org/project/drupal/issues/2708101#comment-11233567

dani3lr0se’s picture

This is still a little confusing. The issue summary describes not being able to save default link text without a URL. The comment #27 is stating that the URL is required for the title. Is the patch addressing the issue or should the issue summary be updated?

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

mangy.fox’s picture

@daniel_rose I agree that this issue has become confused, because the given patch does not address the original issue.

I also do not think that the original issue can be easily resolved. When you set the default value of a field, you are setting the default value for the entire field. As such, that default value needs to be a valid field value. As @pifagor points out, a link without a URL is not valid, and so it makes sense that we should not be able to set a default value that does not contain one.

I propose that we change the issue to something like "Default value for link text does not trigger validation".

michelle’s picture

I just ran into this problem. I am trying to set the default value of the text and also the check box "Link to this page (Leave URL blank)". I'm not 100% sure that checkbox is part of core as I haven't seen it before and I'm new to this project. Even so, it is a use case for setting the link text without a URL so this issue is needed.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

dani3lr0se’s picture

Just wanted to make a note here that I've run into this issue on a project. Client would like to have default link text on a link field, but it doesn't save when I add it. This is definitely a use case and I agree with others that it's a necessary issue still.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

asak’s picture

Status: Needs review » Needs work

Same here - The client is requesting to have a link field, with a required title, with a default title set and an empty URL field, with the option to override the title when saving the entity. This is a legitimate request that matches the original description of the issue, but the patch and approach above do not address this at all.

The suggested module (https://www.drupal.org/project/fixed_text_link_formatter) also doesn't solve the issue, as it uses a fixed title for all links in all entities instead of a custom title per entity (if so desired).

Marking as "needs work" and I'll give this some more thought.

volkswagenchick’s picture

Issue tags: +drupalnorth2019

Tagging for DrupalNorth 2019

yago elias’s picture

Hi! Have you noticed this problem with a specific role? I am having this problem when it come to a editor role, but if I try the same scenario with administrator I don't have this problem

volkswagenchick’s picture

Issue tags: +dcco2019

Tagging for DrupalCamp Colorado Contrib day (Sunday August 4)

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

tarik.cipix’s picture

This problem still persists in 8.9.13, any status updates?

martijn de wit’s picture

Guess the patch from #27 needs a reroll, and there are still tests missing according the issue tags.

tarik.cipix’s picture

Like mentioned in #57 (https://www.drupal.org/project/drupal/issues/2708101#comment-13100404)

The patch in #27 does not really address the issue layed out, it just shows a message that reminds admins about this issue.

martijn de wit’s picture

Issue summary: View changes
Issue tags: -Needs tests, -Needs reroll

my bad.
patch #27 only inlcudes:

<?php
+    elseif(empty($element['#value'])) {
+      $values = $form_state->getValue('default_value_input');
+      if (isset($element['#parents'][2]) && isset($element['#parents'][1]) && !empty($values[$element['#parents'][1]][$element['#parents'][2]]['title'])) {
+        $form_state->setError($element, t('URL is required.'));
+      }
+    }
?>

That is no solution to the real problem indeed.

  • Hiding the current patch because it creates the expectation that a solution is also available.
  • Removing the issue tags because they don't apply to the ticket at this moment.
  • Updating issue summary
prudloff’s picture

StatusFileSize
new2.79 KB

The problem here is that LinkItem::isEmpty() returns true if the item does not have an URI, then WidgetBase::extractFormValues() calls filterEmptyItems() and that removes the items that only have a text but not an URI.
This behaviour is wanted when a user is filling the form normally but not when setting the default value.
However, we lack the context in LinkItem::isEmpty() to know if we are in the default value form or not.

I tried duplicating extractFormValues() in LinkWidget and not call filterEmptyItems() if we are in the default value form. This allow settings the default value in the config but then this value is still not used in the real form.

martijn de wit’s picture

Status: Needs work » Needs review
tarik.cipix’s picture

Status: Needs review » Needs work
StatusFileSize
new19.71 KB
new37.86 KB

Thank you for picking up this thread, I have tested your patch #68.

This does allow admins to save a default value in the link field config, but when adding a new node, the value is not pre-filled.

Tested on Drupal 8.9.13.

heikkiy’s picture

Tested also patch from #68 and verified that it does allow saving the value now but the value is not filled either in content add or edit page.

Tested on Drupal core 8.9.13.

vakulrai’s picture

Status: Needs work » Needs review
StatusFileSize
new7.31 KB
new5.71 KB

I have worked on the solution given by #68, as it is actually saving the default value for "title" field but the same is not coming up on the node form.This patch will take care of the value on node form.

I have also tweaked the unlimited value type , which was also behaving the same manner.

Added a test for the same.

Kindly review.

vakulrai’s picture

StatusFileSize
new7.31 KB

Fixing the CSpell issue in patch #72.

Status: Needs review » Needs work

The last submitted patch, 73: default_value_for_link-2708101-73.patch, failed testing. View results

tarik.cipix’s picture

Status: Needs work » Reviewed & tested by the community

#73 applies on 8.9.14 and seems to work great. Thank you for this patch.

martijn de wit’s picture

Status: Reviewed & tested by the community » Needs work

Seems there are still some test failures.
Patch is for 9.2.x-dev and an user test for 8.9.14.

This is not enough to change status.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

neslee canil pinto’s picture

Status: Needs work » Needs review
StatusFileSize
new7.38 KB
new629 bytes

$url variable seems to be not accessible here

if (isset($url) && $url !== NULL && !$uri_is_valid) {

added a patch to pass the tests.

chetanbharambe’s picture

StatusFileSize
new345.05 KB
new365.68 KB

Verified and tested patch #78.
Patch applied successfully but functionality not working

Testing Steps:
# Goto: admin/structure/types/manage/article/fields
# Click on Add field
# Select a Link from "Add a new field"
# Mention the Label name
# Click on save
# Go: node/add/article
# Leave the URL field is empty as it is not required
# Click on Save
# User is not able to Save and able to see the error "The URL field is required when the Link text field is specified."

Expected Results:
# User should be able to Save the node without mentioning the URL field (as URL field is non required)

Actual Results:
# User is not able to Save and able to see the error "The URL field is required when the Link text field is specified."

Not working as expected.
Moving to Needs Work.

chetanbharambe’s picture

Status: Needs review » Needs work

bnjmnm made their first commit to this issue’s fork.

bnjmnm’s picture

Status: Needs work » Needs review

The review in #79. is based on an incorrect understanding of the requirements.

Expected Results:
# User should be able to Save the node without mentioning the URL field (as URL field is non required)

That is incorrect. It's actually a good thing that this is happening. This issue is to allow a default link text value without including a default URL value for a field. When the field is in use, it should still enforce the presence of a URL value if the link text is not empty.

bnjmnm’s picture

Status: Needs review » Needs work

Review on the Merge Request that is mainly nits.

Just to see if there was a way to avoid duplicating code, I tried to see if there was a way to not have to override extractFormValues() so fully. I didn't find it, and with a little bit of changes to the docs the duplication shouldn't be that big a deal. Hopefully the test coverage will spot any issues that may arise if the parent extractFormValues() is modified and LinkWidget is out of sync with it. Maybe another reviewer will find a slick way around it, but the benefits seem to outweigh the drawbacks of a little duplication.

I manually tested and adding a default value for Link text without URL works great, and when the field is in use, URL is still required if Link Text has a value.

shivansunfire’s picture

Status: Needs work » Needs review
StatusFileSize
new7.61 KB

I've addressed all the suggestions in the merge request https://git.drupalcode.org/project/drupal/-/merge_requests/877, but I haven't been able to push the code back, I receive the message "You are not allowed to push code to this project", despite I clicked the "Get push access" button here in the issue, and it says now "✓ You have push access".

So I'm attaching the patch with the changes.

Status: Needs review » Needs work

The last submitted patch, 85: default_value_for_link-2708101-85.patch, failed testing. View results

shivansunfire’s picture

Status: Needs work » Needs review
StatusFileSize
new7.65 KB

Reverted line 239 of LinkWidget.php to make the tests pass. Attaching new patch.

omrmankar’s picture

StatusFileSize
new7.38 KB

#79 as comment please review the patch.

ranjith_kumar_k_u’s picture

StatusFileSize
new7.5 KB

Re-rolled #88

Status: Needs review » Needs work

The last submitted patch, 89: 2708101-89.patch, failed testing. View results

shivansunfire’s picture

@omrmankar I don't understand why have you uploaded a patch when I already addressed the changes and suggestions from patch 79's review in merge request. That patch is not taking into account all the feedback from the merge request, as I did. And it's not applying.

So @ranjith_kumar_k_u we shouldn't re-roll patch 88, which is failing again.

The issue was in needs review from patch 86, it's the patch that works and addresses all changes suggested by @bnjmnm and @douggreen review. Hidding the last two attempts and putting this back in needs review.

shivansunfire’s picture

Status: Needs work » Needs review
vikashsoni’s picture

StatusFileSize
new31.44 KB
new84.82 KB

applied #87 patch after patch we can set Default Value in text field but we can not save node

for reference sharing screenshot ..

chetanbharambe’s picture

StatusFileSize
new311.56 KB

Verified and tested patch #87.
Patch applied successfully and looks good to me

Testing Steps:
# Goto: admin/structure/types/manage/article/fields
# Click on Add field
# Select a Link from "Add a new field"
# Mention the Label name
# Click on save
# Go: node/add/article
# Leave the URL field is empty as it is not required
# Click on Save
# User can set Default Value in a text field but the user can not save the node

Working as expected.
Please refer attached screenshot
Can be a move to +1RTBC

paulocs’s picture

Assigned: Unassigned » paulocs

I'll organize this issue to keep the MR approach as @juanolalla was not able to push his changes.
@juanolalla maybe your ssh key is not configured in drupal.org. See: https://www.drupal.org/drupalorg/docs/user-accounts/git-authentication-for-drupalorg-projects#s-authenticating-with-ssh-keys

paulocs’s picture

Assigned: paulocs » Unassigned

I update the MR based on the patch #87. I also merged 9.3.x into 2708101-default-value-for branch.

The patches #88 and #89 are wrong so I didn't consider them when I updated the MR.

paulocs’s picture

Issue tags: +Bug Smash Initiative

Tagging the Bug Smash Initiative.

shivansunfire’s picture

StatusFileSize
new16.83 KB

I could solve the issue with the default value overriding the current value for that field in the merge request. Uploading here also an updated patch.

volkswagenchick’s picture

Issue tags: +Europe2021

Adding tag for DrupalCon Europe2021. Thanks

douggreen’s picture

I still think for core that the duplication is unacceptable, for contrib maybe ok, but not for core. This is only a minor problem for my client, so I'm not going to be able to put more time into this. But I think we need a totally different solution. Would extending FieldItemList to LinkFieldItemList give us a place to do this?

In LinkTypeConstraintValidator::validate() $url will always be set, this change is unnecessary, tests pass without it.

roshanibhangale’s picture

Assigned: Unassigned » roshanibhangale
StatusFileSize
new348.45 KB
new355.09 KB

Verified and tested patch #98.
Patch applied successfully but functionality not working

Testing Steps:

  1. Go to admin/structure/types/manage/article/fields
  2. Click on Add field
  3. Select a Link from "Add a new field"
  4. Add the Label name
  5. Click on save
  6. Go to node/add/article
  7. Leave the URL field is empty as it is not required
  8. Click on Save
  9. User can set Default Value in a text field but the user can not save the node.

Expected Result

  1. User should be able to Save the node without mentioning the URL field (as URL field is non required)

Actual Result

  1. User is not able to Save and able to see the error "The URL field is required when the Link text field is specified."

Not Working as expected.
Please refer attached screenshot.
Hence moving to Needs Work

roshanibhangale’s picture

Status: Needs review » Needs work
roshanibhangale’s picture

Assigned: roshanibhangale » Unassigned
shivansunfire’s picture

Status: Needs work » Needs review

@RoshaniBhangale Thank you so much for your work and your effort!

However, what you assumed as expected result, is not correct. See explanation in #82. When the field is in use in a node, it should still enforce the presence of a URL value if the link text is not empty. So this is working as expected.

thatguy’s picture

Patch at #98 applies correctly for Drupal 9.2.7. Fixes the issue of setting default value for link text without setting the default url.

quietone’s picture

Status: Needs review » Needs work
Issue tags: +Needs issue summary update

Nice to see an older issue getting near completion. Thanks everyone.

I came to review but the Issue Summary is incomplete. There are steps to reproduce which is great but there is no proposed resolution or remaining tasks. Tagging for update.

There is also an MR and a patch here - which one is to be reviewed?

@RoshaniBhangale, thanks for the screenshots. They need to be places in the Issue Summary where it is easy for reviewers and committers to find them. I also think it helps to add a comment in the remaining tasks that you tested and the patch works or does not with a link to your comment for more details.

shivansunfire’s picture

Issue summary: View changes
Status: Needs work » Reviewed & tested by the community
shivansunfire’s picture

> There is also an MR and a patch here - which one is to be reviewed?

Both are up to date.

This has already been reviewed and approved by the community in 105#.

shivansunfire’s picture

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 98: default_value_for_link-2708101-98.patch, failed testing. View results

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

shivansunfire’s picture

StatusFileSize
new8.06 KB

The merge request needed to resolve merge conflicts. I've resolved that and updating here the patch.

shivansunfire’s picture

Status: Needs work » Needs review
paulocs’s picture

@juanolalla i know you are putting a big effort in this issue and thanks for that, but you don't need to upload the patch of the MR all the time.
The issue page and the gitlab page provides a link to download the patch of the MR.
Next to the issue fork information you have an option "plain diff" where you can download it. :)

shivansunfire’s picture

Thanks @paulocs, I already know that, but I'm am concerned that if I used that merge request diff patch and someone changed it in a way that something wouldn't work as expected, my site could be broken. That's why I'm also uploading the patch here, so I can use that specific patch and have that control. I don't know the way to use a partial patch from a merge request at a specific point, if there's any.

quietone’s picture

Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Needs usability review

@juanolalla, I think good practice is to have the patch local and use https://github.com/cweagans/composer-patches.

I tested on Drupal 9.4.x, standard install.

Without the patch I confirmed that the default text is not saved. What makes that a problem is that the user is not informed.

With the patch the problem in the IS is resolved. When I then went to add a node and did not enter a link url, I got an error message. "The URL field is required when the Link text field is specified." That makes sense, given this change but it makes we wonder if this change should be made. At least, for me, It seems unusual to have default text for different URLs.

This issue has not had a usability review, so adding tag. The usability team prefers the IS to be up to date and to have screen shots in the IS. It would also help to include reasons why this change is needed in the IS. Setting to NW for that.

Removing credit for unnecessary screenshots.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

murz’s picture

At least, for me, It seems unusual to have default text for different URLs.

My use case with default text for different URLs is the "Download" button with link to a file on CDN, so the content manager should have the "Download" button text by default and type different URLs, but also should have ability to sometimes change it to custom text like "Click here to get the file".

So I was pretty surprised when was trying to save the default "Link text" with the empty URL on default Drupal install, because it always disappears after save. And this patch works pretty well to solve that simple misapprehension!

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

renukakulkarni’s picture

StatusFileSize
new7.78 KB

Re-rolled the patch to work with 9.5.x.

csakiistvan’s picture

@renukakulkarni for me this patch is is very nice, but the link field will mandatory on the form display (but my field is not mandatory on the field settings.)

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

emerham’s picture

At least, for me, It seems unusual to have default text for different URLs.

Also using field tokens you can have a mostly static title that replaces part of the text on render.
For instance we have some PAC classes and we want the link to be:
View the OSU course catalog page for [node:field_pac_class_name].
As long as you make the token field required then it should work. Tested on a site that I'm migrating and it works with this patch.

damienmckenna’s picture

damienmckenna’s picture

The patch mostly works. However, if a default title is provided, when trying to save the entity form the URL field will be required, there's no way of saving the field without an URL. Which is what @csakiistvan said in #121.

pguillard’s picture

Status: Needs work » Needs review

Nice patch, I agree with #125.
I guess it's worth a Needs review, if not RTBC yet.

needs-review-queue-bot’s picture

Status: Needs review » Needs work
StatusFileSize
new1.87 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

douggreen’s picture

I rebase and force pushed the branch. It needs review by someone who is familiar with what this does.

emerham’s picture

StatusFileSize
new7.88 KB

I rerolled the patch from 120 that was working

shalini_jha made their first commit to this issue’s fork.

shalini_jha’s picture

Status: Needs work » Needs review

I have replicate this issues and after applying the changes in the MR 8552, the link text default value save is working properly.
I have observed the pipeline failure, and i have fixed the pipeline failure and now all the test passes in MR 8552 against 11.x . same branch have MR 877 against 9.x , need to hide this MR as we are using MR 8552.

Apart from that i am also agree with #125 . but according to #126 i have fixed the pipeline failure and moving for NR.
Kindly Review.

smustgrave’s picture

Status: Needs review » Needs work

Issue summary still has a TBD section.

shivansunfire’s picture

From #125:

The patch mostly works. However, if a default title is provided, when trying to save the entity form the URL field will be required, there's no way of saving the field without an URL. Which is what @csakiistvan said in #121.

From #121:

For me this patch is is very nice, but the link field will mandatory on the form display (but my field is not mandatory on the field settings.)

I see there is still some confusion with the natural core's behavior of the form display, forcing an URL to be set if there is a link text set, which is correct, and the issue we are fixing here, just allowing a default link text without a default URL in the field settings form.

We had that explained in #82:

The review in #79. is based on an incorrect understanding of the requirements.

Expected Results:
# User should be able to Save the node without mentioning the URL field (as URL field is non required)

That is incorrect. It's actually a good thing that this is happening. This issue is to allow a default link text value without including a default URL value for a field. When the field is in use, it should still enforce the presence of a URL value if the link text is not empty.

So please, let's not bring again the form display behavior that is in core, which is correct, and it is not part of the issue we are dealing with here.

shivansunfire’s picture

Issue summary: View changes

There are no interface changes with this issue, it's just a bug: the link text was not saved, and now with the fix it is saved.

So this doesn't need usability review.

No remaining tasks and nothing else, so moving this to RTBC.

shivansunfire’s picture

Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs usability review
needs-review-queue-bot’s picture

Status: Reviewed & tested by the community » Needs work
StatusFileSize
new2.58 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

mrinalini9 made their first commit to this issue’s fork.

mrinalini9’s picture

Status: Needs work » Needs review

Fixed phpstan issues reported by the bot and have updated the MR, please review it.

Thanks!

sagarmohite0031’s picture

StatusFileSize
new1.26 MB
new1.17 MB

Hello,
Verified and tested on Drupal 11.
The MR is applied successfully.

1.Add a link field to a content type.
2.The link field is configured with "Allow link text" set to be required.
3.Under Default Value, enter text for the "Test data for link".
4.Leave the "default URL" empty.
5.Save the field configuration.
6.Edit the link field and see that the default link text should saved.

Working as expected -
Default link text is saved successfully.

Check attachments-

shivansunfire’s picture

Status: Needs review » Reviewed & tested by the community
quietone’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs usability review
StatusFileSize
new22.56 KB

With the settings shown in the screenshot, before this change the link field is not required and after this change it is, even though the 'Required field' setting is not selected. I think that will cause confusion for admins and content editors, as it has done for people working on this issue. I think we find a way to resolve that here. For that getting an Usability review will help, so I am tagging and changing status.

shivansunfire’s picture

Status: Needs work » Reviewed & tested by the community

With the settings shown in the screenshot, before this change the link field is not required and after this change it is

That is not true, please see #135 for the explanation. This fix is not changing at all the current's Drupal core behavior, that requires an URL if you would like to set a link just with a link text.

This is just a simple fix for the existing bug on the field settings form: the default link text is not being saved.

This fix has nothing to do and touches nothing of the display form, there is no UI changes at all. Please open a new ticket to discuss current Drupal's behavior from an usability point of view, and let this fix solving the bug be committed.

shivansunfire’s picture

Issue tags: -Needs usability review
quietone’s picture

Yes, I read #135 and the other comments as well. Yes, this is a bug with a straightforward fix. That fix, however, has caused unexpected behavior as noted in #79, #101, #116, #121, #125 and #143.

And this fix does cause changes the UI, on a node/add form. The URL part of link field will be shown to the user as required. That is the change and yes it make sense. However, there is no information provided to let the use that the form can be submitted without completing the required field. That is why I asked for a usability review here 3 years ago and I still think one should be done.

amateescu’s picture

Status: Reviewed & tested by the community » Needs work

I think it would be easier to introduce a field item list class for the link field type and override \Drupal\Core\Field\FieldItemList::defaultValuesFormSubmit() to manually return a default value if the title setting is set to required and there's a value for the link title.

shivansunfire’s picture

And this fix does cause changes the UI, on a node/add form.

No, this fix doesn't cause changes in the UI. What you are referring is not part of this fix, is Drupal core current's confusing UI.

Right now, if you want to save a default value for the link text, that's broken. This patch is allowing many sites to fix that, for years. It should be committed.

The URL part of link field will be shown to the user as required.

This is the current core behavior, not the result of this fix. If you try to save a link field with text and no URL. Please open a new ticket to improve that.

shivansunfire’s picture

I think it would be easier to introduce a field item list class for the link field type and override \Drupal\Core\Field\FieldItemList::defaultValuesFormSubmit() to manually return a default value if the title setting is set to required and there's a value for the link title.

I don't understand how that would solve the current problem of the UI's bug, failing to save the default value in the field settings form. And why are you changing the status of this bug fix to "needs work" again.

alexpott’s picture

I'm not sure about the fix here. I think saving an invalid default value might result in more problems as all other default values are valid and not empty. I wonder if we should consider a different fix where we save the default title value somewhere else in the field config if the link is not present. That way we wouldn't have to override extractFormValues which feels a bit risky.

alexpott’s picture

@juanolalla this does result in a behaviour change and it will be confusing for users because once you have a default link title it's completely not obvious that you could remove the link title to save the content without a link. This suggests a potential fix - only allow setting a default link title if the field is required.

alexpott’s picture

On posting the comment I do think we could consider a completely different fix. We could only allow a default link title to be set if the default link is also set. I've read the issue summary and I've not seen a use-case for providing a default link title without the link url. The link title and URL have to be related right - so the user needs to make the title fit the link.

Also as far as I can see the use of titles on links is largely discouraged - see https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/titl..., https://www.tpgi.com/using-the-html-title-attribute-updated/ and https://www.24a11y.com/2017/the-trials-and-tribulations-of-the-title-att... I don't think providing a default title with no related link is going to make this situation better.

shivansunfire’s picture

Also as far as I can see the use of titles on links is largely discouraged

@alexpott This issue is about the default link text, not about link title attribute.

I've read the issue summary and I've not seen a use-case for providing a default link title without the link url.

@alexpott We are using this patch in many sites. The client needs to set a default link text so it is always the same text and the URL is changing: let's say for example "See the full source for more details", where the link pointing to the resource is different in every node.

this does result in a behaviour change and it will be confusing for users because once you have a default link title it's completely not obvious that you could remove the link title to save the content without a link.

The way I see this, more than "this does result in a behavior change", is that this actually allows something expected, the ability to save a default link text (instead of the frustration of doing it and see that it is not saved), and as you say, allowing the expected functionality allows the situation of a new node with a link text already filled and the user could be kind of forced to fill either the URL or delete the link text for that specific node.

However having to delete the link text to leave it empty so it passes validation without a URL in a non required field scenario, is something core is doing right now, not this fix. This fix is just removing a veil over an existing problem in core, hidden by the bug of the default text not actually being saved, despite the UI's supposedly allows it as with any other field. That's why I am encouraging to open a new issue about this, tackling what core does in the form display with links and URLs not filled on non required fields.

This suggests a potential fix - only allow setting a default link title if the field is required.

While it could be the most common scenario, we would not be satisfying all possible desired functionalities. I, as client, could perfectly desire to have a default link text for this field in all nodes that have an URL, which maybe is the 80% of the nodes for that content type, but not 100% so I don't want it to be required, but the default link text is a must, for consistency and time saving.

The real problem here is that, right now, core forces you to remove the link text if you write something on it and click save without filling the URL. This is the issue we are talking about now, the one unveiled by fixing this bug, +8 years open issue.

A possible solution to current's core not very good behavior, could be maybe to let the user have clear that the link text is ultimately dependent on the URL on a specific node edit/create form. So for example, if the link text input would be hidden and only be shown when the URL is actually filled, that would make things clearer, and possibly we wouldn't have to make the user delete the link text input if she/he finally saves the node without a value for the URL, as core currently enforces. As @quitetone says, that would require an usability look.

My proposal is to open a new issue from here to improve current's core confusing behavior, instead of leaving this bug unfixed forever so we not have to look at what is actually an incomplete implementation. If we decided not to fix this bug until we have a better display form implementation we should at least alert the user than if she/he takes the time to fill a default text link value, she/he should also fill an URL default value, or directly remove the option of filling a default text link. But I would love not to do these things, so many people as myself could at least be patching (and maintaining, rerolling) this to allow this necessary feature to our clientes. Many people are actually needing to have this failing feature solved.

dcam’s picture

Issue summary: View changes

I've been following this issue for years, since long before I became the Link module maintainer. I've also wanted to set a default text value for links while letting the URLs be customized per node. Now that I think about it, I still manage that application where I wanted that ability.

That said, I understand the potential usability issues at play here. I started thinking about other possible solutions and came to a realization that ironically was suggested back in 2016 in comment #43 by @Ilesyt:

I would like an option where could I input a link text in the 'Manage display' when editing the field display.

The need that this issue is looking to solve would be really easy implement with a field formatter that has configurable default text. And wouldn't you know it, there's a module that does it: Fixed text link formatter. So if you need default link text functionality, then I advise looking into it.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

michaelsoetaert’s picture

StatusFileSize
new7.6 KB

Created a patch based on the current state of MR 8552, which applies on 11.3.x.

michaelsoetaert’s picture

StatusFileSize
new6.77 KB

The previous patch for 11.3.x did apply (wrongly) on 11.4.x, but caused a crash due to isNoLinkRoute-function already being part of drupal/core:

Fatal error: Cannot redeclare Drupal\link\Plugin\Field\FieldWidget\LinkWidget::isNoLinkRoute() in /var/www/html/docroot/core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php on line 595

I've updated the patch to not add the isNoLinkRoute-function anymore.

shivansunfire’s picture

The need that this issue is looking to solve would be really easy implement with a field formatter that has configurable default text. And wouldn't you know it, there's a module that does it: Fixed text link formatter. So if you need default link text functionality, then I advise looking into it.

The need that this issue is looking to solve is not that content appears with a fixed link not decided by editors but in configuration.

The need this issue is looking to solve is to allow editors to create links having already a suggested default configuration value in the node add form, a value they can use or change there. That module doesn't help with this at all.

shivansunfire’s picture

Here is a module to provide this missing functionality: Default Link Text Widget

Since this is 10 years old and we've been most of the time supporting a patch, I decided to contribute a simple module extending Link widget to allow to set this default value for link text in the widget through Form Display, so we don't depend on core solving this bug with all its implications.