Default value for link text is not saved

Created on 18 April 2016, almost 9 years ago
Updated 28 March 2023, almost 2 years ago

Problem/Motivation

The default value for "link text" is not being saved.

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 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.

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

Update IS for usability review
Usability review

User interface changes

Yes

TBA

API changes

-

Data model changes

-

🐛 Bug report
Status

Needs work

Version

10.1

Component
Link 

Last updated 5 days ago

Created by

🇺🇸United States JasonSafro

Live updates comments and jobs are added and updated live.
  • Needs usability review

    Used to alert the usability topic maintainer(s) that an issue significantly affects (or has the potential to affect) the usability of Drupal, and their signoff is needed. When adding this tag, make it easy to review the issue. Make sure the issue summary describes the problem and the proposed solution. Screenshots usually help a lot! To get sign-off on issues with the "Needs usability review" tag, post about them in the #ux channel on Drupal Slack, and/or attend a UX meeting to demo the patch and get direct feedback from designers/UX folks/product management on next steps. If an issue represents a significant new feature, UI change, or change to the general "user experience" of Drupal, use Needs product manager review instead. See the scope of responsibilities for product managers.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇭🇺Hungary csakiistvan

    @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.)

  • 🇺🇸United States emerham

    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.

  • 🇺🇸United States DamienMcKenna NH, USA

    FYI there's a related feature request for allowing a static link title: Link field: add the ability to have a static anchor text and link title Active

  • 🇺🇸United States DamienMcKenna NH, USA

    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.

  • Status changed to Needs review about 1 year ago
  • 🇫🇷France pguillard

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

  • Status changed to Needs work about 1 year ago
  • 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.

  • 🇺🇸United States douggreen Winchester, VA

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

  • Pipeline finished with Failed
    7 months ago
    Total: 349s
    #209134
  • Merge request !8552Resolve #2708101 "Default value for" → (Open) created by douggreen
  • Pipeline finished with Failed
    7 months ago
    Total: 203s
    #209138
  • 🇺🇸United States emerham

    I rerolled the patch from 120 that was working

  • First commit to issue fork.
  • Pipeline finished with Failed
    4 months ago
    Total: 571s
    #305076
  • Pipeline finished with Failed
    4 months ago
    Total: 631s
    #305075
  • Pipeline finished with Failed
    4 months ago
    Total: 458s
    #305093
  • Pipeline finished with Canceled
    4 months ago
    Total: 514s
    #305092
  • Pipeline finished with Success
    4 months ago
    Total: 354s
    #305104
  • 🇮🇳India shalini_jha

    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.

  • 🇺🇸United States smustgrave

    Issue summary still has a TBD section.

  • 🇪🇸Spain juanolalla

    From #125 🐛 Default value for link text is not saved Needs work :

    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 🐛 Default value for link text is not saved Needs work :

    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 title set, which is correct, and the issue we are fixing here, just allowing a default title text without a default URL in the field settings form.

    We had that explained in #82 🐛 Default value for link text is not saved Needs work :

    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.

  • 🇪🇸Spain juanolalla

    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.

  • 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.

  • First commit to issue fork.
  • 🇮🇳India mrinalini9 New Delhi

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

    Thanks!

  • 🇮🇳India sagarmohite0031

    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-

  • 🇳🇿New Zealand quietone

    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.

  • 🇪🇸Spain juanolalla

    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 test it is not being saved.

    This fix has nothing to do and touches nothing of the display form. 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.

  • 🇳🇿New Zealand quietone

    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.

  • 🇷🇴Romania amateescu

    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.

  • 🇪🇸Spain juanolalla

    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.

  • 🇪🇸Spain juanolalla

    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.

  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    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.

  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    @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.

  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    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.

  • 🇪🇸Spain juanolalla

    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.

Production build 0.71.5 2024