Follow-up to #2693725: Add <nolink> to allow for non-link links

Problem/Motivation

The fix in #2693725: Add <nolink> to allow for non-link links exposed the fact that LinkGenerator is doing a lot of pre processing when it comes to GeneratedLink objects. We can do better.

Proposed resolution

Make the GeneratedLink object do the HTML building.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#32 2718795-nr-bot_axte90qw.txt2.27 KBneeds-review-queue-bot
#12 interdiff.txt3.06 KBdawehner
#12 2718795-12.patch8.61 KBdawehner
#10 interdiff.txt1.61 KBdawehner
#10 2718795-10.patch5.55 KBdawehner
#5 interdiff.txt3.62 KBdawehner
#5 2718795-5.patch7.16 KBdawehner
#2 2718795-2.patch5.08 KBalexpott

Issue fork drupal-2718795

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:

About issue forks

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Active » Needs review
StatusFileSize
new 2718795-2.patch5.08 KB
dawehner’s picture

dawehner
Primary language German
commented 
  1. +++ b/core/lib/Drupal/Core/GeneratedLink.php
@@ -24,7 +26,42 @@ class GeneratedLink extends BubbleableMetadata implements MarkupInterface, \Coun
    *
    * @var string
    */
-  protected $generatedLink = '';
+  protected $generatedLink;

    Should we document that this is lazy now ... potentially a subclass might access it directly. 

  2. +++ b/core/lib/Drupal/Core/GeneratedLink.php
@@ -41,18 +88,21 @@ public function getGeneratedLink() {
    * @param string $generated_link
    *   The generated link.
    *
-   * @return $this
+   * @return \Drupal\Core\GeneratedLink
+   *   A new generated link.
    */
   public function setGeneratedLink($generated_link) {
-    $this->generatedLink = $generated_link;
-    return $this;
+    // Copy over ¶
+    $new_object = static::createFromObject($this);
+    $new_object->generatedLink = $generated_link;
+    return $new_object;
   }

    Yeah we should point to ::create() and deprecate it 

  3. +++ b/core/lib/Drupal/Core/Utility/LinkGenerator.php
@@ -11,7 +10,6 @@
 use Drupal\Core\Routing\UrlGeneratorInterface;
-use Drupal\Core\Template\Attribute;
 use Drupal\Core\Url;

    I really like how this removes the dependency to the template part

Status: Needs review » Needs work

The last submitted patch, 2: 2718795-2.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
StatusFileSize
new 2718795-5.patch7.16 KB
new interdiff.txt3.62 KB

Some adaptions on top of the points mentioned above.

Status: Needs review » Needs work

The last submitted patch, 5: 2718795-5.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented

dawehner-- We should not touch the URL here.

alexpott’s picture

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

@dawehner i agree :)

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

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now 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.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
StatusFileSize
new 2718795-10.patch5.55 KB
new interdiff.txt1.61 KB

Removed my sillyness

Status: Needs review » Needs work

The last submitted patch, 10: 2718795-10.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
StatusFileSize
new 2718795-12.patch8.61 KB
new interdiff.txt3.06 KB

Let's fix the failures ... these failures though bring up the question whether this potential BC break might be actually a problem.

Status: Needs review » Needs work

The last submitted patch, 12: 2718795-12.patch, failed testing.

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

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now 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.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now 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.

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.

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.

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.

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.

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.

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.

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.

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.

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.

smustgrave’s picture

smustgrave commented
Status: Needs work » Postponed (maintainer needs more info)
Issue tags: +stale-issue-cleanup

Thank you for creating this issue to improve Drupal.

We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.

Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

Thanks!

catch’s picture

catch
he/him
Primary language English
commented
Status: Postponed (maintainer needs more info) » Needs work

This is still relevant, could use the patch converting to an MR.

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

libbna opened merge request !13061

libbna’s picture

libbna commented
Status: Needs work » Needs review

I’ve created an MR for patch #12, but the patch references the file ThemeRenderAndAutoescapeTest.php, which doesn’t exist in the current codebase. Should I create this file?

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 2718795-nr-bot_axte90qw.txt2.27 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.

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.