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
When adding the contextual links field to a view that is configured to show fields, it won't show anything, plus notices in log.
User error: "attributes" is an invalid render array key in Drupal\Core\Render\Element::children() (line 98 of /app/web/core/lib/Drupal/Core/Render/Element.php)
Steps to reproduce
- Fresh Drupal 9 install.
- Create some content.
- Create a view of that content type.
- Display fields
- The field Title shows up by default.
- Add the field Link to edit content and hide it.
- Add the general Contextual links field and select to use the Link to edit the content.
- Save the view and visit it.
- You will not get the contextual link for node edit. Of course you will see the contextual link to edit the view, so do not get confused here.
- Check the logs and you will see some notices too.
Remaining tasks
- Write patch
- Reroll patch
- Add test coverage
Comment | File | Size | Author |
---|---|---|---|
#64 | 2532200-64-9.5.x.patch | 9.15 KB | juanolalla |
| |||
#53 | drupal-2532200-41.png | 380.37 KB | merlin06 |
Issue fork drupal-2532200
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:
Comments
Comment #1
dawehnerDid you updated from an older Drupal core version to beta 12? In case yes,
please try out the head2head module and there especially the beta2beta module.
Note: We don't have an update path yet in core: https://www.drupal.org/node/2341575
Comment #2
larowlanUntil we get confirmation of whether this was a new install
Comment #3
Psycle Interactive CreditAttribution: Psycle Interactive commentedComment #4
jibranComment #5
OlafskiI see exactly the notices mentioned in the summary, testing a fresh Drupal 8.01 (new install). My goal was to add a contextual edit link to a view of fields. Steps to reproduce:
- Install Drupal 8.01
- Create some articles
- Create a views block of articles, Display: Fields, and add some article fields, e.g. Title and Body.
- Add an edit link field to the display, and hide it.
- Add the field "Global: Contextual Links", and incluce the edit link.
- Save the view.
- Configure the block to show up somewhere.
As a result (besides the above mentioned notices), the contextual links field shows up, but it doesn't show the edit link. (Tested with Bartik.) That's what I see in the HTML:
Comment #6
dawehner@Olafski
If you want to make the life of people easy, please provide an export of the view.
Comment #7
OlafskiSorry, that I missed that. Please see the attached YAML file.
Comment #8
LendudeI can reproduce this issue when adding the contextual links field. The notices are due to parent::buildOptionsForm not being called.
However, solving that only gets rid of the notices but the contextual links will still be empty.
During rendering, $rendered_field = $this->view->style_plugin->getField($values->index, $field); is called which looks in the rendered_fields, which is still empty during render, so this will always skip all fields.
When working around that, the contextual button is rendered but will still not work (and when used inside a table layout be unclickable due to positioning).
It's a mess. Attaching a patch to get rid of the notices mostly to point to where the trouble comes from, but this needs much more work and the ContextualLinks handler needs test coverage.
Comment #9
Wim LeersComment #10
dawehnerBack to contextual module, where the code lives.
Comment #11
Wim LeersThis lives in the contextual module, yes, but all the complexity here is in Views:
I just thought it'd have a better chance of getting the right people to look at it if it's in the Views issue queue. The contextual module doesn't need this to function. It's an enhancement for Views. (That used to live in the Views module in D7.) But, if you disagree with that, fine.
Just be aware that the contextual module has no active maintainer. I try to take on some of it, but I won't do this issue, because it goes so deeply into Views complexity.
Comment #12
Lendude@Wim Leers That's what the VDC tag is about, I always thought. Views related stuff that isn't in the views or views ui module.
I'll take a look at this, at least try to make it semi-functional and provide some basic test coverage for starters.
Comment #13
Wim LeersFair enough :)
Comment #14
dawehnerWell, complexity is always relative.
Comment #15
LendudeFirst steps.
Some tests that illustrate that this thing is really broken.
The fix just makes the tests pass and makes the handler a little less broken. But no actual links are rendered yet (but the trigger button is atleast now). The information passed to
_contextual_links_to_id
seems to be out of date. That function seems to want some sort of structured data, but I can't tell what it wants (haven't looked really hard). I just saw that the data it gets passed for the View contextual link is structured differently then what gets passed by the views handler.So that data
needs to be restructured to make this work (added the route_parameters key to make the tests pass), but that complexity lives in the contextual module and I'm not really familiar with that :)
Putting it on needs review to run the testbot, but this obviously still needs work to get this working.
Comment #17
LendudeLooked at the information passed to _contextual_links_to_id a bit more closely but can't really find a format that actually works for this field.
Comment #18
mr.york CreditAttribution: mr.york commentedAfter applying this patch, the content of the data-contextual-id is the following:
If i bring my mouse above the item, then the contextual link icon appears, however if i click on it, then the menu does not appear.
The menu list is not in the div (<ul class="contextual-links">...</ul>).
Line 165 in contextual.js:
var html = storage.getItem('Drupal.contextual.' + contextualID);
the content of the html variable is empty, the key exists, but its empty.
Comment #25
chegor CreditAttribution: chegor at OAO Solution Spark commentedComment #26
ivnish CreditAttribution: ivnish commentedPatch doesn't apply to Drupal 8.7.2
Comment #27
andypostComment #28
aleevasThe last patch was updated.
The tests was actualized.
Comment #30
aleevasWas uploaded fix for the last patch
Comment #31
Lendude@aleevas thanks for looking at this, but this thing is so broken it's never getting fixed. This is still based on how contextual links worked in D7 and that has zero connection to how it works in D8.
Just use the Dropbutton field if you need this kind of functionality. And maybe add some styling to make it look like contextual link or something.
Comment #34
NWOM CreditAttribution: NWOM commented#30 fails to apply on 9.06 for those interested in fixing this on D9
Comment #38
geek-merlinI took a stab at this and pushed to issue fork. Did not test it though.
Comment #39
geek-merlinOK i debugged and fixed everything, we now have a field and area handler that worksforme 🌠 in MR 99.
I do not currently have bandwidth to work on test coverage, but i reviewed patch #30 and it looks like it is an excellent starting point.
(Attaching patch for my deployment workflow, please ignore.)
Now going to fix #2983655: Contextual links are not displayed correctly.
Comment #40
geek-merlin(Patch only for deployment.)
Comment #41
geek-merlinNow i'm happy with it code-wise. (Patch only for my deployment, work happens in MR.)
Comment #42
geek-merlinHuh, i'm too sleep deprived and hacked in the wrong issue. Reverting the last commit.
Comment #43
ivnish CreditAttribution: ivnish commented@geek-merlin
How can I test it?
I added field to my view "link to edit node"
I added field "contextual link"and choose my field
But nothing. I don't see contextual link near my views row
Comment #45
jenlamptonI have applied the patch from #41 and the contextual links are appearing for me. Marking RTBC.
Comment #46
xjmThanks everyone for reviving this and coming up with a D9-compatible solution.
One thing that's missing here is test coverage. An automated test was added way back in #15 for 8.0.x, so that might be a starting point, although I don't recommend using such old config exports etc. and obviously the test API has changed drastically since then. (It'd be better to recreate the fixtures if we use a similar approach for adding tests this time around.)
Comment #47
xjmOh, also, let's please write an issue summary. :) Thanks!
Comment #50
phenaproximaI think this is fixable, or at least automatically testable, but I have no idea how to reproduce it. Can someone please update the issue summary with clear steps to reproduce the problem? Postponing until then.
Comment #51
ecvandenberg CreditAttribution: ecvandenberg as a volunteer commentedReproduction steps:
Comment #53
merlin06 CreditAttribution: merlin06 at Australian Competition and Consumer Commission (ACCC) commentedI patched to https://www.drupal.org/files/issues/2021-01-31/drupal-2532200-41.patch but I had to alter this line:
From:
$rendered_field = $this->view->field[$field]->last_render;
To:
$rendered_field = $this->view->field[$field]->last_render ?? FALSE;
See uploaded file "drupal-2532200-41.png" for more details about what caused the error.
Comment #55
mherchelAdding steps to reproduce. Unpostponing (poning?)
Ran into this today.
Comment #56
catchLooks like this mostly needs a re-roll and tests?
Comment #57
tintoAdding remaining tasks (and headings) to issue summary.
Comment #58
mherchelThe diff in the merge request still applies (haven't tested it yet), but we need to move it to 9.5.x.
Comment #59
tintoComment #61
juanolalla CreditAttribution: juanolalla at Peak Digital, LLC commentedRe-rolled https://git.drupalcode.org/project/drupal/-/merge_requests/99.patch for 9.5.x.
Comment #62
_utsavsharma CreditAttribution: _utsavsharma at OpenSense Labs for DrupalFit commentedFixed CCF for #61.
Please review.
Comment #63
_utsavsharma CreditAttribution: _utsavsharma at OpenSense Labs for DrupalFit commentedComment #64
juanolalla CreditAttribution: juanolalla at Peak Digital, LLC commentedI've reviewed the last two patches 61 and 62 and I think I made a mistake somehow when re-rolling it. According to the latest version of changes in the merge request (for 9.2.x), we shouldn't be altering any JavaScript files.
I've merged this merge request branch into 9.5.x and here I'm uploading the diff patch, which is very similar to the merge request changes. Unless there is something wrong from the latest work in the merge request, we should be reviewing and using this one.
Comment #65
juanolalla CreditAttribution: juanolalla at Peak Digital, LLC commentedComment #66
juanolalla CreditAttribution: juanolalla at Peak Digital, LLC commentedComment #67
smustgrave CreditAttribution: smustgrave at Mobomo commentedThis issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.
Previously was tagged for tests in #8 which still need to happen.
Comment #68
Bohus UlrychI tested 2532200-64-9.5.x.patch (#64) and it seems to be working well. Tested with Drupal 9.5.1
Comment #69
Akhil Yadav CreditAttribution: Akhil Yadav commentedAdded patch against #64 in 10.1 version
Comment #70
BramDriesenHiding patch #69 as it's omitting the added tests.
Adding it to the list here: #3339883: Employees of Dotsquares are posting mass re-roll patches which are invalid and/or incomplete