Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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
Comment | File | Size | Author |
---|---|---|---|
#6 | interdiff.txt | 510 bytes | dawehner |
#6 | 2819593-6.patch | 3.09 KB | dawehner |
#5 | interdiff.txt | 3.24 KB | dawehner |
#5 | 2819593-5.patch | 3.59 KB | dawehner |
#2 | 2819593-2.patch | 2.89 KB | webflo |
Comments
Comment #2
webflo CreditAttribution: webflo at UEBERBIT GmbH commentedComment #3
webflo CreditAttribution: webflo at UEBERBIT GmbH commentedComment #4
JacineTHANK 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.
Comment #5
dawehnerIMHO 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.
Comment #6
dawehnerLet's fix some
Comment #7
joelpittetThank you, this looks like the way to go. It should do the same for Jacines case but maybe she could double check?
Comment #8
JacineYes! #6 works just fine. Thank you. :)
Comment #9
alexpottCommitted 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.