Problem/Motivation

Follow-up for #2744517: Twig template link() call to accept markup.

LinkGenerator::generate create a GeneratedLink object. The Markup passed to GeneratedLink::setGeneratedLink is safe according to the comment, but Link::preRenderLink creates another instance of Markup later because GeneratedLink contains the literal string. It should hold the Markup object instead.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Comments

webflo created an issue. See original summary.

webflo’s picture

webflo’s picture

Status: Active » Needs review
Jacine’s picture

THANK YOU @webflo!

I have a custom icon template that I use in many places, including `template_preprocess_menu()` in which I change the menu link title to a render element using this icon template. The upgrade to 8.2 broke the template preprocess implementation (all the others still worked). The code is here in case that helps (maybe a test case): https://gist.github.com/jacine/d97fdaef19a7cbe60a39b85f90f18740 (left the search form example, since it never stopped working for comparison).

This patch, combined with the one in #2744517: Twig template link() call to accept markup solved the above issue.

dawehner’s picture

IMHO we can fix it better. GeneratedLink is already a Markup object, so we should leverage that information.
I also changed the test to actually test anything. Before, it really just tested that twig is calling the link generator, no matter what its output is. IMHO the better test is to test the actual HTML.

dawehner’s picture

Let's fix some

joelpittet’s picture

Status: Needs review » Reviewed & tested by the community

Thank you, this looks like the way to go. It should do the same for Jacines case but maybe she could double check?

Jacine’s picture

Yes! #6 works just fine. Thank you. :)

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed f08a96e to 8.3.x and 549d5c5 to 8.2.x. Thanks!

I've committed to 8.2.x because #markup is not API and this is a bug.

  • alexpott committed f08a96e on 8.3.x
    Issue #2819593 by dawehner, webflo: Link title double escaping in Link::...

  • alexpott committed 549d5c5 on 8.2.x
    Issue #2819593 by dawehner, webflo: Link title double escaping in Link::...

Status: Fixed » Closed (fixed)

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