Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
ContextHandler needlessly recreates context objects when applying contexts to a plugin.
This is wasteful but also can cause side-effects:
Given an EntityContext
object, it will recreate a plain Context
object.
Proposed resolution
Use setContext instead of setContextValue in ContextHandler::applyContextMapping
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#16 | 3016458-context-16-interdiff.txt | 843 bytes | tim.plunkett |
#16 | 3016458-context-16.patch | 7.87 KB | tim.plunkett |
#12 | 3016458-context-12-PASS.patch | 7.86 KB | tim.plunkett |
#12 | 3016458-context-12-FAIL.patch | 1.67 KB | tim.plunkett |
#3 | 3016458-context-3-PASS.patch | 6.18 KB | tim.plunkett |
Comments
Comment #2
tim.plunkettBlocks #2976148: Layout-based entity rendering should delegate to the correct section storage instead of hardcoding to either defaults or overrides
Comment #3
tim.plunkettWhoops, included that other issue.
Comment #5
tim.plunkettThis code was in the original version of ContextHandler #2277981: Provide a service for handling context-aware plugins, committed 2014-06-10.
Because at that time,
setContext()
hadn't been written. That wasn't until #2281635: \Drupal\Core\Plugin\Context\Context should use a ContextDefinition class instead of arrays, committed 2014-06-25.Comment #7
EclipseGc CreditAttribution: EclipseGc at Acquia commentedThis change seems very sensible to me.
Eclipse
Comment #8
tim.plunkettAlso I should note that if we were to have fixed this issue before fixing #3008431: A context object for a specific entity type will not match a generic requirement for any entity, we would have been exposing that bug in core.
But now since that issue landed, we're safe here.
Comment #9
xjmThanks @tim.plunkett for documenting the history of the bug and the relationship to #3008431: A context object for a specific entity type will not match a generic requirement for any entity. That gives me confidence that this is a good change and that any side effects from this fix are mitigated in 8.7.x.
One thing -- the final fail patch says "Build Successful". I'm going to queue another run to see if we can get a proper fail
Comment #10
xjmVia @tim.plunkett: CI is obscuring the fail, but it can be seen on Dispatcher:
https://dispatcher.drupalci.org/job/drupal_patches/78334/testReport/juni...
So we don't actually need a retest.
Comment #11
xjmLet's add a test verifying that we get the
EntityContext
object when appropriate as described in the IS (and add that test to the fail-patch).Thanks!
Comment #12
tim.plunkettBetter to be explicit in our testing.
FAIL patch is the interdiff.
Comment #14
phenaproximaLooks great.
Comment #16
tim.plunkettRerolled for #3014949: Deprecate 'context' on Block and Condition plugin annotations in favor of 'context_definitions'
Comment #17
alexpottCommitted and pushed b33232b7d2 to 8.7.x and 2b93e80229 to 8.6.x. Thanks!
Comment #21
xjmThis seems likely to have introduced:
https://www.drupal.org/pift-ci-job/1134931
It's possible this is this is just due to inattentive backporting given the relationship to #3008431: A context object for a specific entity type will not match a generic requirement for any entity documented above. We'll know in an hour or so. Meanwhile, I've reverted it from 8.6.x only.
Comment #22
alexpottYep my mistake. Thanks for the revert. As long as 8.7.x passes I suggest we close this unless the authors want to port a fix to 8.6.x
Comment #23
alexpottYep so the builds are fine. Setting to fixed as per #22. If we want this in 8.6.x then we'll need to port the patch.
Comment #25
tim.plunkettI've asked for #3008431: A context object for a specific entity type will not match a generic requirement for any entity to be backported, and would like the same for this once that's in.
Comment #26
xjmAs per #3008431: A context object for a specific entity type will not match a generic requirement for any entity, I think this should remain in 8.6.x for now, since there isn't actually direct benefit to the affected project from backporting them alone. The affected project should depend on the next minor.