The entity context plugin (plugins/contexts/entity.inc) does not define a 'placeholder name' property as part of it's plugin definition. This prevents generic context handling without a priori knowledge that you are processing an entity context.

To reproduce this bug: 

$context_info = array(
	'name' => 'entity:node',
	'context_settings' => array(),
	'keyword' => 'node',
	'identifier' => 'Node being viewed',
);

// Load node with nid = 1234 as a context
$context = ctools_context_get_context_from_context($context_info, 'context', '1234');

// $context should contain a fully created ctools context with node 1234, but instead this throws an error

This example is somewhat artificial because i've defined $context_info by hand, but I ran into the bug in code where $context_info is created programatically and may contain any context type.

This can be fixed by adding 'placeholder name' => 'entity_id', to the $plugin definition for the entity context.

CommentFileSizeAuthor
#6 entity_context_plugin-2797449-6.patch551 bytesrivimey
#3 2797449-2.patch467 bytesphayes
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

phayes created an issue. See original summary.

phayes’s picture

phayes CreditAttribution: phayes commented
Issue summary: View changes
phayes’s picture

phayes CreditAttribution: phayes commented
FileSize
2797449-2.patch467 bytes

Attached is a patch that fixes this issue.

phayes’s picture

phayes CreditAttribution: phayes commented
Status: Active » Needs review
DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented

The 'placeholder name' value has almost no documentation for its usage, the only place in the codebase it appears is in includes/context.inc:

    if (isset($argument) && isset($plugin['placeholder name'])) {
      $context[$plugin['placeholder name']] = $argument;
    }

Based upon this, it does seem this may be a reasonable request.

rivimey’s picture

rivimey
she/her
CreditAttribution: rivimey as a volunteer commented
FileSize
entity_context_plugin-2797449-6.patch551 bytes

The patch in #3 applies but with a whitespace error - there is a space at the end of the line.

The code listed in #5 was (mostly) added by @merlinofchaos in this change from 2009:

Expand the context to allow the plumbing for replacing placeholder contexts with real contexts for preview
http://cgit.drupalcode.org/ctools/commit/?id=9847a855fe62f71bcdcb53e52bb...

with a subsequent change by Sam Boyer in:
Integrate 7.x port work done in git back into CVS.
http://cgit.drupalcode.org/ctools/diff/includes/context.inc?id=a40482acb...

It is clear from this that the ctools_context_get_context_from_context() value $argument was added purely to support 'placeholder name', and as such it would seem that adding the key as @phayes suggests is benign and very probably what @merlinofchaos intended.

I have been looking around for somewhere to extend the documentation on this. I think at a minimum we need a line added to the plugin definition, which I have added in "entity_context_plugin-2797449-6.patch", but this feels insufficient. Would it be sensible to add to "plugins-creating.html" as well and if so, what?

rivimey’s picture

rivimey
she/her
CreditAttribution: rivimey as a volunteer commented
Status: Needs review » Reviewed & tested by the community
Parent issue: » #2828925: Plan for CTools 7.x-1.13 release

Proposing RTBC for the 1.13 'features' release.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented

I think this is ok, it really feels like an oversight.

japerry’s picture

japerry
Location KVUO
CreditAttribution: japerry commented
Status: Reviewed & tested by the community » Fixed

#3 committed with the suggestions offered in #6.

  • japerry committed 7412afc on 7.x-1.x authored by phayes 
    Issue #2797449 by phayes, rivimey, DamienMcKenna: entity context plugin...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.