Problem/Motivation

I have noticed that active contexts are sometimes duplicated. I'm primarily seeing this when using both the theme negotiator and block reactions together.

This causes the code in the reactions to be executed twice and in some cases elements on the page are duplicated.

Steps to reproduce

  • Create a context using the theme negotiator
  • Create a 2nd context with a block reaction.
  • Enable the context reaction debugger block
  • Visit a page.
  • you will see that the active contexts are duplicated.

Proposed resolution

I have a MR incoming

Issue fork context-3516562

Command icon 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:

About issue forks

Comments

loze created an issue. See original summary.

loze opened merge request !64

loze’s picture

loze commented
Status: Active » Needs review

Im not sure if this is the correct approach, but it does fix the problem for me.

Im keying the activeContexts array by context id which makes sure any context is only set once.

loze’s picture

loze commented

I would have thought the $this->contextConditionsEvaluated variable takes care of this but apparently when both these reactions are used the value is somehow getting reset? I tried figuring that out but not sure whats going on there.

benstallings’s picture

benstallings commented
Version: 5.0.0-rc2 » 5.x-dev
Status: Needs review » Reviewed & tested by the community

Claude Code says:

Three things happening in the single-line change:

1. Keys the array by context ID — $this->activeContexts[$context->id()] = $context instead of $this->activeContexts[] = $context. This prevents the same context from appearing multiple times.
2. Skips already-evaluated contexts — empty($this->activeContexts[$context->id()]) short-circuits before calling evaluateContextConditions() if the context is already in the active set.
3. Reorders checks — !$context->disabled() is checked first (cheap), then the duplicate check (cheap), then evaluateContextConditions() (expensive) last. The original checked evaluateContextConditions() before disabled().

Assessment

The changes are sensible. A couple of observations:

Minor: conditionsHasBeenEvaluated() already guards against re-evaluation

getActiveContexts() at line 237 checks conditionsHasBeenEvaluated() and returns early if already evaluated, so the duplicate-entry guard in evaluateContexts() should only matter if evaluateContexts() is called directly more than once, or if there are somehow duplicate context entities returned by getContexts(). It's defensive but reasonable.

Minor: callers may depend on numeric array keys

The original $this->activeContexts[] produced a numerically-indexed array. The new code produces an associative array keyed by context ID. Any caller doing something like $activeContexts[0] would break — though this is unlikely since the typical usage is foreach. Worth verifying no callers index numerically.

The reorder of disabled() before evaluateContextConditions() is a good optimization

In the original, disabled contexts still had their conditions evaluated (wastefully) before being filtered out. Moving !$context->disabled() first avoids unnecessary evaluation work.

Overall: clean, small, and beneficial. Good to merge assuming no callers rely on numeric array indexing.

loze’s picture

loze commented

For the record, I have been using this in production on 3 separate sites for the past year w/o any issues.