Contextual links on blocks should change DRUPAL_CACHE_GLOBAL to DRUPAL_CACHE_PER_ROLE

Proper component

I was able to reproduce, I'll investigate further.

Steps to reproduce:

1. Fresh install
2. Enable forum module
3. Move block "new forum topics" to first sidebar
4. Access home as anonymous

Contextual links are being cached for this block, apparently.

EDIT: clear cache and reload page as anonymous user, the link is gone.

Consistently, after doing #5, reload page as admin and the link will also be gone.

No. The bug is in forum module.

I found a solution. The block can be cached by role, and this solves issues with contextual links permissions.

Adding "Needs tests", but actually the test case would be to check contextual links on cached blocks.

Where are the contextual links being added from? If it's block module or contextual module those modules should fix the caching rules.

catch, the forum module does a custom cache handling, which means other modules won't affect it at all. It renders the full block an cache it for himself. I don't know the reason for this behavior, but I doubt there is anything contextual.module can handle.

If forum isn't handling the contextual links, it should be responsible for the caching changes. In fact forum doesn't use block caching at all so that makes this even more the case.

OK reviewed the code a bit.

forum module does not do anything about contextual links.

block module adds contextual links in _block_get_renderable_array().

Really it's here that it should force the renderable array to be cached per role, this would mean altering the cache key provided by forum module.

Otherwise every block that does it's own caching_block_get_renderable_array also needs to be cached per role, not just forum module.

Actually, if you want to pull it into a more generic fix, then the very presence of contextual module would completely invalidate any DRUPAL_NO_CACHE block. This is not at all bad, but I'm unsure where exactly this should get implemented. If my assumption is true, there could be a well documented hook_block_alter implementation on contextual.module to make this change, as it would be easy to find and understand later.

Ugh, name it all wrong.

That's DRUPAL_CACHE_GLOBAL and not DRUPAL_NO_CACHE. Also, I believe DRUPAL_CACHE_PER_PAGE is also affected.

And that's hook_block_info_alter().

There you go.

Wanted to roll a new patch, but this is getting a lil bit tricky...:

/**
 * Implements hook_block_info_alter().
 *
 * @todo This may have to run last, after all other modules?
 */
function contextual_block_info_alter(&$blocks) {
  foreach ($blocks as &$module_blocks) {
    foreach ($module_blocks as &$block) {
      // @todo For some contextual links, caching per role would be sufficient,
      //   but certain blocks (e.g., node [access] related) would have to be
      //   cached per user.
      if (isset($block['cache']) && (($block['cache'] & DRUPAL_CACHE_GLOBAL) || ($block['cache'] & DRUPAL_CACHE_PER_PAGE))) {
        $block['cache'] |= DRUPAL_CACHE_PER_ROLE;
      }
    }
  }
}

Doesn't node access restriction disable block cache globally?

Sorry this is actually a duplicate of #914382: Contextual links incompatible with render cache.

READ: http://drupal.org/node/914382#comment-4792776

See my comment there too, there may have to be a generic fix and a one off hack, but it is the same bug with contextual module do let's keep it in one place please.