Jump to:

Problem/Motivation

hook_menu() is still responsible for contextual links, so we can't remove the old menu router.

In addition the system for finding contextual links is based on a confusing combination of path hierarchy and hook_menu() type constants.

Proposed resolution

A totally new mechanism for finding contextual links is added using plugins that are very similar to those used for local tasks.

The plugin type which is discovered using YAML files and use routes to render the contextual links.

AJAX is still used to fetch all the rendered contextual links for a page. A minor change is made to the format of the contextual links ID that's rendered in the HTML so that we have named parameters compatible with route variables.

Remaining tasks

Follow-ups to convert remaining contextual links and update Views to use the new system (possible via plugin derivatives).

Yup, that's what I found out too. My mind thought "Derivatives? Annotated classes!".

This patch adds a default value for the weight, removes an erroneous copypaste code comment, and some Views-related code that @dawehner told me to get rid of. I have to go somewhere, so I can't look into the test failures just yet, but let's see what the removal of that Views-related code does for them.

And what's worse: the colon was used to separate each of the main components, but now it is apparently also being used to serialize the values of this component, so I don't see how this can be reliably deserialized. At least, this now seems to have become more complex/confusing.

I think Peter has a point: the serialization used to be super simple, but now there's apparently a need for key-value pairs to be serialized as well. Then it might be wiser to just switch to JSON.

Furthermore, it seems that this is excessive now:

list($module, $parent_path, $path_keys, $path_args, $metadata_raw)

It seems we can get rid of $module now, since every route must be unique anyway, and we don't need to call a module-specific hook anymore? And shouldn't $parent_path be renamed to $route_name now?

Also, where/how are the contextual links being defined right now? The combination of ContextualLinkInterface being added and then not being implemented by any of the things that provide contextual links is very confusing.

Which brings me to my final point: the resulting code is now so opaque that I don't understand anymore what's going on. It'd be useful to explain the code flow in the issue summary.

Also, where/how are the contextual links being defined right now? The combination of ContextualLinkInterface being added and then not being implemented by any of the things that provide contextual links is very confusing.

Which brings me to my final point: the resulting code is now so opaque that I don't understand anymore what's going on.

Contextual links are defined through YAML. Any definitions will automatically be applied to a base class that implements ContextualLinkInterface. Any developers who wish to add more logic to their links or override the base class can add a "derivative" key to the YAML definitions which points to a derivative discovery class which can dynamically declare link definitions with their own "class" keys that point to other classes than the base class.

Well, and you can also just directly specify the class key to hae a single custom implementation. This is this sort of thing I'l do for local actions and maybe a contextual link for clone module when I upgrade it.

I aslo agreee with Wim that we can flatten this a bit, but the top level key is really the "group" not the route.

"path args" are no longer exactly what they used to be before. They appear to be route parameters instead now. Which is fine. But then we should update the variable names as well.

More importantly, at least one of the parts of the pattern is now in fact the group name that is defined in the contextual links YAML files. But it's impossible to tell which: the module name, or the parent path? I think the parent path. If it's the parent path, then that variable name should be updated as well.

This structure change is most peculiar. Due to the values, it used to be very clear/explicit which is the module name and which is the path. Now it's impossible to tell which is which. Furthermore, I can't help but wonder why we need a nested array here?

NULL was used for local tasks in order to provide a different weight for the default tab, though yeah this is not needed here.

I personally find it more legible if there's a newline after a call to the parent class; but that might just be me.

I prefer that as well.

The "which should be shown" part seems inaccurate?

Yeah let's just drop that addition. The context is the function call, that's it.

Can't we delete all "context" properties of menu items?

You are totally right.

I think this should include the language ID? How else will these be translatable? And: how would e.g. French Drupal developers be able to define these in French first, English second?

Well, it was always the case in drupal that everything which is wrapped with t(), both in the .info file, .module/.php files or whatever is considered as some sort of English. There is not need to special case that here.

This seems wrong? However, I can still delete custom blocks … so maybe not?

Please just double-check :)

Ups ... this was a mistake.

Why not convert all contextual links? Follow-ups are not really accepted in this phase of D8 anymore AFAIK.

The problem is that we still have non-route menu_router entries.

This structure change is most peculiar. Due to the values, it used to be very clear/explicit which is the module name and which is the path. Now it's impossible to tell which is which. Furthermore, I can't help but wonder why we need a nested array here?

Well, we need the route parameters as additional array that is clear.

This structure change is most peculiar. Due to the values, it used to be very clear/explicit which is the module name and which is the path. Now it's impossible to tell which is which. Furthermore, I can't help but wonder why we need a nested array here?

I agree that this nested structure is odd. I guess the reason is to easily add new links to an existing module but yeah what about adding a follow up to improve that, so we don't have to touch existing non-converted code?

I agree that this nested structure is odd. I guess the reason is to easily add new links to an existing module but yeah what about adding a follow up to improve that, so we don't have to touch existing non-converted code?

This sounds to me like this patch should (sadly :() be blocked on having completed routing system conversions? Only then will we be able to make all this very explicit, very clear, non-ambiguous?

This patch removes all access control from the contextual links system. For example, someone with permission to use contextual links but not to configure blocks will get a "Configure block" link on the block with this patch applied.

I'm not particularly religious about this (especially because I was originally on the other side of the debate in Drupal 7) but this is adding extra work for developers; if they want their contextual links to have the same title and weight as regular links, they now need to duplicate that information in two places.

It's quite possible the extra flexibility is worth it, but it's surprising that this issue has managed to sidestep that whole debate, and adding extra work for developers is something we should always be careful with. It's not actually obvious to me that contextual link definitions should be separated from regular link definitions (i.e. a large part of the premise of this issue), since they aren't really separate links so much as they are different presentations of the same links.

This patch removes all access control from the contextual links system. For example, someone with permission to use contextual links but not to configure blocks will get a "Configure block" link on the block with this patch applied.

Added the functionality and a test.

I'm not particularly religious about this (especially because I was originally on the other side of the debate in Drupal 7) but this is adding extra work for developers; if they want their contextual links to have the same title and weight as regular links, they now need to duplicate that information in two places.

I totally see the point that coupling paths, menu links and routes into a single syntax helps to keep the amount of typing as low as possible. On the other hand though
we need to get rid of the old routing system. One additional point is that we already moved local actions and local tasks to its own file, so we should try to make things consistent and as simple as possible. Therefore
we sadly have to port many examples in order to find out common problems. (we did that for example for local actions which really helped).

Why are you turning "Delete" into a tab? Shouldn't this entry just be removed?

On this was not on purpose. Good catch, really!

Are there any performance concerns with this patch? I'm noticing, for example, that menu_contextual_links() has static caching which this new system doesn't seem to repeat.

which means that we end up with really a few cache entries (there are like ~5 groups in core at the moment). This could be a potential improvement for big sites can cache this information in memcache.

I think a static cache would totally make sense given that contextual links, on node for example, appear multiple times per page. Do you think we should get in the static cache already in this patch or open a follow up?

If I'm reading this right, this is a totally new and different hook compared to drupal_alter('menu_contextual_links', $links, $router_item, $root_path) in menu_contextual_links().

You are right, this is not about the actual rendering contextual links, but about the definitions of them. This is needed for some advanced usecases like for example views. I renamed the actual hook here.

It's also cached (whereas hook_menu_contextual_links_alter() isn't) so if you want to use it to e.g. dynamically change the titles or otherwise dynamically edit the links, you'd be out of luck?

The amount of alter hooks is a bit confusing. Is it enough to have one alter information for the static information and one for the links determined before returning to the rendering?

I think a static cache would totally make sense given that contextual links, on node for example, appear multiple times per page. Do you think we should get in the static cache already in this patch or open a follow up?

Normally I'd say a followup (in the spirit of not adding static caches unless they're known to be necessary) but since there was already one in the existing menu_contextual_links() code I think it would be good to add here. And I think it was there for the reasons you mention above.

The amount of alter hooks is a bit confusing. Is it enough to have one alter information for the static information and one for the links determined before returning to the rendering?
...
Do we really even need a run-time alter hook? These are plugins so if you need a dynamic title just define your class to provide it.

I would say the second one (the dynamic one) is important to have; it's not just about dynamic titles but also about things like removing links based on certain conditions, etc. Given the second, perhaps the first (the static one) isn't necessary though.

Are both of these necessary? If so, it could be stored in a variable rather than determined twice.

The second one was a leftover from some of my experiments but I decided to not at the contextual link in the first place, as the old menu based system did the same.

Not sure offhand if this example is correct, either security-wise or translation-wise...

Can you think of a better idea what to provide as example?

I would say the second one (the dynamic one) is important to have; it's not just about dynamic titles but also about things like removing links based on certain conditions, etc. Given the second, perhaps the first (the static one) isn't necessary though.

I would like to keep the static one, as every plugin type in core basically has this alter hook.
For local tasks really special usecases like views have to provide local tasks based on other ones,
so you can use a static alter method.

This fixes the existing test errors ... aka. I have just ran the unit tests, shame on me.

#70: thank you so much for that review, it very clearly shows that this patch was definitely inappropriately RTBC'd earlier.

Translation handling is still opaque to me.

I agree that this nested structure is odd. I guess the reason is to easily add new links to an existing module but yeah what about adding a follow up to improve that, so we don't have to touch existing non-converted code?

I am against the approach of doing everything at once. You know, what really matters is progress and this can be achieved with this patch. Ripping out things once it is done is damn simple.

I agree that progress matters. But quite often, these kinds of follow-up tasks are not completed timely, which results in people not knowing how to make things right.
Furthermore, I still feel that the nested structure is highly problematic. It's very, very hard to understand. Previously, it was implicitly clear because of its values. But now that the values have become so similar, it's a puzzling mess.
Hence I ask again to let this patch wait until all non-route menu_router entries have been converted.

If I can still find problems in this patch so easily, then I don't think this should be RTBC'd unless David Rothstein and maybe another person reviews this patch another time after a reroll for these problems I just found:

+++ b/core/lib/Drupal/Core/Menu/ContextualLinkInterface.php@@ -0,0 +1,60 @@+ * Returns the group this contextual link should be rendered on.

s/on/in/?

Also, the concept of a "contextual link group" is not explained anywhere.

And the important fact that it must be *unique* inside all of Drupal should also be mentioned, I think.

+++ b/core/modules/system/system.api.php@@ -979,6 +979,62 @@ function hook_menu_contextual_links_alter(&$links, $router_item, $root_path) {+ * So for example it is an array like that:

Replace all this with just "e.g."

+++ b/core/modules/system/system.api.php@@ -979,6 +979,62 @@ function hook_menu_contextual_links_alter(&$links, $router_item, $root_path) {+ // Dynamically use the menu name for the title of the menu edit contextual

Overall, I still feel this makes for an unnecessarily complex system. Mitigating facts:

very few developers will ever use this API

the old API was cumbersome as well

Only by rationalizing it this way, I can convince myself that it'd be acceptable for this to go in. So: I'm fine with this if e.g. David Rothstein is fine with it. However, I continue to think that nested array structure is a major WTF.

Also, the concept of a "contextual link group" is not explained anywhere.

And the important fact that it must be *unique* inside all of Drupal should also be mentioned, I think.

Hopefully the new lines match your requirement.

So the route parameters must be the same for all contextual links. Which makes sense. But I think it's valuable to have that documented/explained explicitly here.

It wasn't necessary before, because you could only do "node/X/OPERATION" things. But now that you can add arbitrary routes, i.e. arbitrary paths, so an additional explanation is warranted IMO.

Right, this behavior is assumed by the full new routing system all over the place.

This for example have to be figured out in the generic views implementation of contextual links, which basically requires another bunch of code. This is one of the the reasons why I don't want to drop all
implementations in this patch. It just blocks itself.

In fact, why was $metadata renamed to $options? That is such a broad term, that it makes things even harder to grok.

Adapted. It seems to be for me that options are used in many more places in core, so people are more familiar with it than metadata.

Regarding 6 and 7, these have been copy and pasted lines of documentation, but nevermind, I just changed it.

Should be wrapped in t()?

Good idea.

It says "keyed by plugin ID" here, but AFAICT this is not a plugin ID, it's a "contextual link ID".

Now we are getting really nitpicky, don't we?

Accidentally ended up in patch, I think :)

Oh damn.

Overall, I still feel this makes for an unnecessarily complex system. Mitigating facts:

very few developers will ever use this API
the old API was cumbersome as well
Only by rationalizing it this way, I can convince myself that it'd be acceptable for this to go in. So: I'm fine with this if e.g. David Rothstein is fine with it. However, I continue to think that nested array structure is a major WTF.

Well, if we have converted all instances in core we should maybe switch to keyed parameters like this:

I left out the "module" key because it isn't actually used for anything. And yes, since the first "block" (the array key) isn't actually used for anything either, it could just be a numeric key as in #80, but that wouldn't be good for anyone who is trying to alter this. So it should be a meaningful string, and by convention it could just be the same as the group.

* @param array $route_parameters- * The incoming route parameters.+ * The incoming route parameters. This route parameters need to have the+ * same name on all contextual link routes, e.g. you cannot use node and+ * entity in parallel.

Following up on the discussion above, I wonder if that's actually true? I would expect that if you pass all possible route parameters along, multiple links within the same group could still use only a subset of them.

So if you pass array('node' => $node, 'entity' => $entity) as the route parameters, and some contextual links within the group point to routes which require $node whereas others point to routes which require $entity, maybe it just works?

By the way, (minor) in the above "This route parameters" should be "These route parameters".

So I tried it out and actually it looks like we don't need to sanitize this. When I did it led to double-escaping (since ultimately this title gets passed into theme_links() which sanitizes the title). So perhaps we want !label in this example.

I assume the translation is needed here, of course. Though I'm not sure I understand where regular (non-altered) link titles from the .yml file are getting translated, it does seem to be happening somewhere.

I left out the "module" key because it isn't actually used for anything. And yes, since the first "block" (the array key) isn't actually used for anything either, it could just be a numeric key as in #80, but that wouldn't be good for anyone who is trying to alter this. So it should be a meaningful string, and by convention it could just be the same as the group.

As far as I remember this is used for some parts of the CSS but I agree we can drop it.

I do really like your alternative syntax.

So if you pass array('node' => $node, 'entity' => $entity) as the route parameters, and some contextual links within the group point to routes which require $node whereas others point to routes which require $entity, maybe it just works?

Sadly this is not the case. The symfony routing system has indeed some flaws like here. Every parameter which does not appear in the url pattern would be added as a query parameter, so you end up innode/123?entity=123

Doesn't work currently, since $group should be $group_name.
I discovered that when looking into the next item....

Extended the unit test.

I assume the translation is needed here, of course. Though I'm not sure I understand where regular (non-altered) link titles from the .yml file are getting translated, it does seem to be happening somewhere.

These ones are translated in the getTitle() method of ContextualLinkDefault, see the following piece of code:

I went we keeping the module name so you have 'node.node' => array('route_parameters' ...) to keep the changes as small as possible.

Additional this patch converts the content_translation as well as views_ui contextual links. There is now just a single place where the old
contextual links api is used. The remaining one is the dynamic support of configured views to provide a contextual link. I think we have to
change the UI in order to support group based contextual links.

+++ b/core/includes/theme.inc@@ -1713,8 +1713,18 @@ function theme_links($variables) {+ // @todo theme_links() should *really* use the same parameters as l(),+ // and just take an array of '#type' => 'link' elements.

+++ b/core/modules/block/block.module@@ -124,6 +123,7 @@ function block_menu() { // that the plugin can appropriately attach to this URL structure.+ // that the plugin can appropriately attach to this URL structure.

<li><code>+++ b/core/lib/Drupal/Core/Menu/ContextualLinkInterface.php@@ -0,0 +1,67 @@+ * A contextual link group is a bunch of contextual link displayed together on+ * the page. For example the block group displays all links related with the+ * block, like the block instance edit link as well as the views edit link,+ * if it is a view block.

I had a bunch of separated comments, but instead I'm just proposing an improved version of this comment. Feel free to take over whichever parts you like:

A contextual link group is a set of contextual linksthat are displayed together on a certain page. For example, the 'block' group displays all links related to the block, such as the block instance edit link as well as the views edit link, if it is a view block.

+++ b/core/lib/Drupal/Core/Menu/ContextualLinkInterface.php@@ -0,0 +1,67 @@+ * The name of a group has to be unique.

It would be cool if we could be more explicit about this. Unique among what? Per page? per route?

"based to" sounds weird. Maybe just "Returns an array of link options" as the @return already specifies what is meant. Maybe also add a @see to LinkGeneratorInterface::generate (with the FQNS, of course).

For extra credit, this would extend ContextualLinkManagerInterface which defines the added functions here, i.e. getContextualLinkPluginsByGroup() and
getContextualLinksArrayByGroup().

Since that would be a tad more work, and this is a rather pressing issue, I think it would be fine to do that in a follow-up.

+++ b/core/lib/Drupal/Core/Menu/ContextualLinkManager.php@@ -0,0 +1,179 @@+ * The incoming route parameters. The route parameters need to have the same+ * name on all contextual link routes, e.g. you cannot use node and entity+ * in parallel.

Just for clarity I think it would make sense to put 'node' and 'entity' in quotes.

+++ b/core/lib/Drupal/Core/Menu/ContextualLinkManager.php@@ -0,0 +1,179 @@+ * A list of links as array, keyed by the plugin ID. Each entry is an+ * associative array with the following keys:

Is there a reason for this high weight? I think 'Translate' is one use-case for showing up between 'Edit' and 'Delete' in fact.

+++ b/core/modules/contextual/contextual.module@@ -237,13 +238,15 @@ function contextual_pre_render_placeholder($element) {- * keyed array. Each key is the name of the implementing module, and each- * value is an array that forms the function arguments for- * menu_contextual_links(). For example:+ * keyed array. Each key is the string "$implementing_module:$group".

This is missing documentation for the array value. I.e. that is in array of 'route_parameters' and 'metadata'

+++ b/core/modules/system/system.api.php@@ -840,6 +840,62 @@ function hook_menu_contextual_links_alter(&$links, $router_item, $root_path) {+ * ID. Each entry contains the following keys:+ * - title: The displayed title of the link+ * - route_name: The route_name of the contextual link to be displayed+ * - group: The group under which the contextual links should be added to.+ * Possible values are e.g. 'node' or 'menu'.

A contextual link group is a set of contextual links that are displayed together on a certain page. For example, the 'block' group displays all links related to the block, such as the block instance edit link as well as the views edit link, if it is a view block.

Just took that bit as it is.

It would be cool if we could be more explicit about this. Unique among what? Per page? per route?

Well, i just wanted to make clear that all contextual links with the same group are rendered in the group. This though should be also somehow covered by the previous block of docs.

"based to" sounds weird. Maybe just "Returns an array of link options" as the @return already specifies what is meant. Maybe also add a @see to LinkGeneratorInterface::generate (with the FQNS, of course).

Just replaced it with "passed to"

For extra credit, this would extend ContextualLinkManagerInterface which defines the added functions here, i.e. getContextualLinkPluginsByGroup() and
getContextualLinksArrayByGroup().

Added the interface.

Is there a reason for this high weight? I think 'Translate' is one use-case for showing up between 'Edit' and 'Delete' in fact.

No there is not. Set back to 2 as it is in the hook_menu() entry.

This is missing documentation for the array value. I.e. that is in array of 'route_parameters' and 'metadata'

Fixed.

Also 'weight' I think and 'options'?

Added weight. Options is localized_options, ... we should clean that up later.

I'm going to say "block.block" is basically equivalent to the confusion that Wim Leers was worried about above...

This gets used in list($module, $group) = explode('.', $module_group) but I don't see how $module is ever used in a meaningful way? I would expect the group to always be enough to identify the group of links, so either like #81 (where 'group' is explicitly defined) or #87 (where the group is the array key) and not bothering with $module at all seem like they would be a lot simpler for module developers and with no apparent loss in functionality.

Well, module is for example used in the code which still exists (menu_contextual_links()) which we cannot drop yet.
Battle-plan: Get this patch in, convert the last usage of hook_menu() based contextual links and then drop the $module. Would that be acceptable?

I'm not seeing how the changes in the examples match the changes in the pattern.

Sorry, but why can't the group thing be done in this patch? In theory we are in the phase of the release where we're trying not to actively break peoples' modules. This is already going to be a huge break, why cause another one a couple of weeks later?

Right. So I think basically the premise is, "This is holding up a critical [remove the old router system] and so we're asking to commit this as an interim step."

I think that's probably a fair request, but I think either David or Wim should probably be the ones to RTBC it, rather than a co-author of the patch.

That also by definition means this won't hit alpha4, but by holding it until alpha5, we have a chance to complete the views/contextual links follow-ups and remove the module names by then, so to module devs chasing alphas, it's just one API change, not two.

I RTBC'd this in #100.
@David_Rothstein bumped it in #101 because the $module/$group thing was confusing, but also because the example was wrong.
@dawehner fixed the example in #102 and explained why we need this in before fixing anything else

So the re-RTBC in #104 is just a hold-over from mine.

I would mark this RTBC again, but @webchick specifically asked for Wim or David to do that, so I'll sit on my hands for a bit :)

I looked this over and I agree with @webchick in #105. It's not clear to me why we can't just change the API a single time now. Introducing $module as part of the new API only to remove it later shouldn't be necessary.

So I tried getting rid of it in the attached patch. Let's see how it goes with the testbot.

Also, the menu_contextual_links() call in #102 doesn't look like it would have ever worked? From the patch there:

But menu_contextual_links() expects the second parameter to be a path, so I don't see how $group could have been the right thing.

Given that the tests still passed in #102, that makes me wonder if we can't just go ahead and remove that whole menu_contextual_links() function now too... But I left it in this patch (and fixed it) for the time being.

I made a couple other tiny fixes in the attached patch too, but the vast majority of it is just replacing $module.$group with $group.

That looks like an unfortunate change (and there are a few others like it elsewhere). It wasn't obvious to me where this comes from (something to do with the plugin ID being used to build up the array of contextual links?).

OK, hm, so it looks like the tests in #102 only passed due to a coincidence. In the case of nodes, the $group is "node", which also happens to be the first part of the path for node URLs ("node/%/edit", etc). That is why the menu_contextual_links() call worked - but only in that specific case. Furthermore, the tests only pass because the test user doesn't have access to other node-related contextual links (besides the one provided by the view)... In real life, the Views functionality for contextual links is still broken.

I can make #111 work the same way - see attached. I think that will get the tests passing. However, I don't see the point of hacking core to make a test pass, if the functionality the test is trying to test doesn't work in practice... I think it's preferable to just remove the broken test.

For clarity, attached interdiffs are compared to #102 and to #111. Note that I restored one code comment ("A list of places where contextual links should be added") that the original patch changed but that shouldn't have been.

Here's the version that would make more sense to me - since it doesn't seem like we can make the Views stuff work at all in this issue and the only reason menu_contextual_links() was left behind was to try to make it work, just remove menu_contextual_links() instead and then leave a @todo about the broken Views functionality.

That's not great of course (it's a rather large @todo) but it's more honest about the state that this patch leaves Drupal in, plus it means we get the whole API change done at once.

The main reason why the contextual link part of views worked without issues on one of my patch was that the group "node" matched the pattern "node", now that I realized that.

The actual problem with removing the feature out of views temporarily is that we don't seem to have a clear idea how to implement
it inside the new system, because otherwise it would be part of the patch itself. I do have some potential implementations though which all require changes in the UI.

The actual problem with removing the feature out of views temporarily is that we don't seem to have a clear idea how to implement it inside the new system, because otherwise it would be part of the patch itself. I do have some potential implementations though which all require changes in the UI.

Yeah, exactly. It seems like the user interface would have to change to allow the administrator to choose a contextual links group?

---

By the way, since this discussion may be confusing to others, the functionality we're talking about is the one that allows you to configure page displays to appear as contextual links. For example:

cp core/modules/node/tests/modules/node_test_views/test_views/views.view.test_contextual_links.yml core/modules/node/config/ (quick way to get a View that has this functionality)

Install Drupal.

Create a node and while logged in as an administrator look for the "Test contextual link" contextual link on it.

As far as I understand, all patches in this issue currently break that functionality.

does seem to be hitting the database cache an awful lot (for example, once for each node on the page, in the case of contextual links on nodes). So it's missing the static caching that menu_contextual_links() had to try to limit this to one database hit for all nodes on the page, and this would be a good thing to add (or at least make sure we have an issue to track adding it).

I'm trying to adapt config translation to this new API in #2130075: Config translation should work with new contextual links API. Honestly I'm having a hard time and I think the DX of the group system is not very good. So what I figured out by trial and error is basically the contextual links system is works with groups. You either establish a new group with links to be collected by altering the build #contextual_links array to include your group or you know an existing group name and add your links to that. Eg. content translation uses existing group names per entity name.

Now for config translation, we want to add contextual links to translate things at different places, eg. Views, blocks, etc. Whenever a contextual operations list may show up. So we don't really know the group names for those routes either (and I don't know if there would be a way for us to figure out, the contextual links are not defined based on their parent routes but rather their actual routes and the grouping system holds them together). We could also define new groups but that would mean we need to alter in all kinds of build arrays, which I'm not sure how is possible generically.

At this point I think I'm probably missing something since before this all we did is we included a key in our hook_menu() and the item fell into the right place being at the right parent-child point for the path. Now that is not an option at all and figuring out existing groups and/or defining new ones with build altering seems much harder.

BTW for local tasks the grouping is based on tab root route ID, so while it is harder to do than D7, it can be looked up based on routes. Much more predictable, not a free-to-pick group name. For contextual links seems like the interaction of the build arrays and the groups is much less predictable to extend to me. :/

Documented the current process for contextual links to the best of my knowledge at https://drupal.org/node/2133283. This is NOT a change notice, it does not compare to Drupal 7, it compares to how local tasks / actions are done which are similar :)