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.If you create a block that displays a node within it - e.g., by calling node_view() - then a very funny thing happens with the contextual links. The combined contextual links of the node+block (e.g., "Edit" and "Delete" for the node, and "Configure block" for the block) appear twice on the page. When you hover over the block you see all three of those attached to the block, and when you move your mouse inside the block and hover over the node, you see a second set attached to the node.
This is because the contextual links preprocess function gets called for each #theme and #theme_wrapper function attached to the element, and tries to add the contextual links each time.
I believe the proper behavior here is that we want the block to inherit the contextual links of its children (that's what already happens when a menu is displayed in a block), or more generally that we only want to display the contextual links when the element's outermost theme function is being run. The attached patch achieves that in a relatively simple way, I think without any side effects.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | contextual-links-rendered-multiple-times_2.patch | 1.15 KB | JacobSingh |
| contextual-links-rendered-multiple-times.patch | 1.25 KB | David_Rothstein | |
Comments
Comment #1
sunMakes sense to me. Are we able to test this? (looks like we still have zero tests for Contextual module...)
Comment #2
JacobSingh CreditAttribution: JacobSingh commentedHere's a re-roll of the same
Comment #3
sunComment #4
moshe weitzman CreditAttribution: moshe weitzman commentedLooks good to me too.
Comment #5
moshe weitzman CreditAttribution: moshe weitzman commentedIMO we can forego tests here. This is one line of code, and the theme_wrappers system is tested elsewhere.
Comment #6
Dries CreditAttribution: Dries commentedI don't think I understand this bugfix. For example, what happens when a block has two direct children (at the same level) -- would the links start to clash as they are merged? Can someone try to explain what will happen?
Comment #7
sunI also think that we're heavily building on assumptions in this patch. We definitely should add a test that asserts those expectations. Looking at the code, and even after reading the comment, I'm not sure what will actually happen.
Comment #8
David_Rothstein CreditAttribution: David_Rothstein commentedIt turns out this patch has a problem.
When the contextual_preprocess() function runs, it changes two template variables, $classes and $title_suffix. So by introducing a requirement that it can only do this for one theme template per element, this patch introduced an assumption that that theme template would always be responsible for rendering both those variables. That assumption holds true for blocks (the original use case), but not for some other things. For example, I ran into a problem with this when writing code for Views that attaches contextual links to the entire page. There, page.tpl.php is responsible for printing $title_suffix, but the wrapper template (html.tpl.php) is responsible for printing $classes, so this patch breaks things.
Not sure if there is a different solution we can do here. I think the original bug is still valid, but there's also a workaround for it. Instead of doing
$block['content'] = node_view(...)you can do$block['content']['some_key'] = node_view(...), and then pull out$block['content']['some_key']['#contextual_links']back to the top level, if you want the contextual links to render there. That technique seems to work well, and I've written a patch for Media Gallery (the original place where I encountered this issue) that does that instead: #1069750: Contextual links appear twice on the media gallery block