Needs work
Project:
Drupal core
Version:
main
Component:
theme system
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
29 Jun 2011 at 13:52 UTC
Updated:
31 Jul 2023 at 16:16 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
jessebeach commentedI've been wanting to do this for a long, long time. It took me a little while to figure out where the default arguments were being set, but once I found them in common.inc, it was smooth sailing.
Comment #2
jessebeach commentedAssigning this to me.
Comment #3
jessebeach commentedApparently using the array key "#type" threw the contextual links into a tizzy.
https://skitch.com/jesse.beach/feb93/developer-tools-http-drupal.dev
Changed the array key so that the contextual links render correctly.
Comment #4
ericduran commentedI think we might need to check the $list variable needs before just printing it.
Comment #5
ericduran commentedcheck plain should be enough being that the only expected option is a text string.
Comment #6
h3rj4n commentedRewrote the patch to work with the new folder structure.
I think this feature improves HTML5 in Drupal. It might be better that the $list value is limited by a set of options to prevent the li tags of being wrapped out of context. Not shure if this should be checked on this location or that this is something a user should be aware of.
I created two patches. One with the limited options and one without it.
Comment #7
h3rj4n commentedforgot to set it to needs review
Comment #8
jessebeach commented@ericduran, good thought about checking plain. We should be covered though, because the only three values allowed for list are 'menu', 'ul', or 'ol'.
It should be possible to sneak anything else by.
This is really one of those Drupal WTF in the theming layer.
Thanks for updating this @h3rj4n.
I'm setting to RTBC to get this on folks' radars. It shouldn't be controversial.
Comment #9
catchThis needs tests.
Prefer the version that limits to known tags, definitely don't want a check plain here - if you let people put arbitrary HTML via a UI then you're responsible for adding appropriate permissions to that UI.
Comment #10
mgifford#6: 1203908-add_type_theme_links_with_options.patch queued for re-testing.
Comment #11
mgifford6: 1203908-add_type_theme_links_with_options.patch queued for re-testing.
Comment #13
joelpittetI'd rather not do this dynamic tags approach, maybe a template suggestion would be better but not sure. Regardless of my opinion this is not going to be in 8.0.0 so we need to move it to 8.1.x feature if someone wants to continue this approach.