I had an issue with a context having two conditions:

  • Content type (ctools): one content type selected
  • Vocabulary (ctools): one vocabulary selected

In the node pages of the content type selected, the first condition is being met and the context fires correctly, but in the taxonomy term page of a term of the selected vocabulary, the context doesn't fires.

I debugged and noticed that in the taxonomy term page i was having a MissingValueContextException applying the context mapping to the "content type" condition, resulting in returning FALSE without applying contexts to he rest of conditions.

# modules/contrib/context/src/ContextManager.php

protected function applyContexts(ConditionPluginCollection &$conditions) {

    foreach ($conditions as $condition) {
      if ($condition instanceof ContextAwarePluginInterface) {
        try {
          $contexts = $this->contextRepository->getRuntimeContexts(array_values($condition->getContextMapping()));
          $this->contextHandler->applyContextMapping($condition, $contexts);
        }
        catch (ContextException $e) {
           return FALSE;
        }
      }
    }

    return TRUE;
  }

I don't know exactly why applying context mapping to the "content type" condition is failing, but i think that that doesn't have to abort applynig contexts to the other conditions. Indeed, when i replaced the return FALSE; with a continue; the context fired and everything worked ok in the taxonomy term page

¿Anybody with a better Drupal insights understanding can review this?

Thanks!

Note: I applied a patch for this issue in order to have the Taxonomy ter route context, it may be neccesary to reproduce #2998826: Term route context.

Comments

vidorado created an issue. See original summary.

vidorado’s picture

Version: 8.x-4.x-dev » 8.x-4.0-beta2
vidorado’s picture

vidorado’s picture

Issue summary: View changes
bzoks’s picture

Patch #3 was not created in module root path, so I had to recreate it to be able to apply it.
It works correctly, tested on 8.x-4.x-dev on Drupal 8.8.

nace_fr’s picture

Status: Needs review » Reviewed & tested by the community

Tested the patch and it works as expected. Changing status to "Reviewed".

ronaldmulero’s picture

Related issues: +#2998826: Term route context

Thanks @vidorado

Patch #3 #5 works for me to fix Vocabulary (ctools) context Conditional only when combined with Patch #38 from https://www.drupal.org/project/drupal/issues/2998826#comment-13385527

Drupal core 8.8.1
Context 8.x-4.0-beta2

bircher’s picture

Status: Reviewed & tested by the community » Needs review
StatusFileSize
new752 bytes

The patch from #5 Makes the method always return TRUE. Even though the return value is not used anywhere the in this module the method is not private so we should maintain the same return value.

Attached is patch that solves the problem but also still returns FALSE when one of the contexts has a missing dependency etc.

loze’s picture

The patch in #5 worked for me. The one in #8 did not.

loze’s picture

Here is what I ended up needing to get this working.

Status: Needs review » Needs work
loze’s picture

Status: Needs work » Needs review
deaom’s picture

Status: Needs review » Reviewed & tested by the community

I can confirm that the patch under #10 is working, the patch under #5 did always return true, which is now corrected with the patch #10. And the patch under #8 did not work as it was returning FALSE if one of the conditions failed, which meant public function evaluateContextConditions(ContextInterface $context) returned false. So marking this as RTBC.

paulocs’s picture

Thanks! I'll fix on dev branch.

  • paulocs committed d31cd87 on 8.x-4.x authored by loze
    Issue #3045666 by vidorado, loze, bircher, bzoks, nace_fric,...
paulocs’s picture

Status: Reviewed & tested by the community » Fixed

  • paulocs committed 0924f09 on 8.x-4.x authored by silverham
    Issue #3177007 by paulocs, silverham: Sitewide context broken due to #...
bircher’s picture

Status: Fixed » Needs work

I think the patch from #10 is not a good solution.
It will return true if the last context doesn't throw an exception and false if the last context throws an exception.

If returning false when one of the contexts throws an exception doesn't work, then maybe we should return true if at least one of them doesn't throw an exception.
So false if all throw an exception and true if one of them in the loop doesn't.

bircher’s picture

Status: Needs work » Fixed

sorry for the noise!
It does what I suggest already.

Status: Fixed » Closed (fixed)

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

jonmcl’s picture

Version: 8.x-4.0-beta2 » 8.x-4.x-dev

Changing version to dev since that is where the patch was committed. It is not yet available in beta2 (or beta5)