Problem/Motivation

BlockViewBuilder currently only sets a block plugin's cache contexts if the block is cacheable.

This is wrong: cache contexts must always be set.

Proposed resolution

Fix it.

Remaining tasks

None.

User interface changes

None.

API changes

None.

CommentFileSizeAuthor
#5 interdiff.txt804 byteswim leers
#5 block_view_builder_cache_contexts-2458413-5.patch4.26 KBwim leers
#3 interdiff.txt1.07 KBwim leers
#3 block_view_builder_cache_contexts-2458413-3.patch3.81 KBwim leers
#1 block_view_builder_cache_contexts-2458413-1.patch2.79 KBwim leers

Comments

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Active » Needs review
StatusFileSize
new block_view_builder_cache_contexts-2458413-1.patch2.79 KB

Status: Needs review » Needs work

The last submitted patch, 1: block_view_builder_cache_contexts-2458413-1.patch, failed testing.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Needs review
StatusFileSize
new block_view_builder_cache_contexts-2458413-3.patch3.81 KB
new interdiff.txt1.07 KB

And voila, the expected test failure, proving that we indeed have test coverage for this.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Needs work 
+++ b/core/modules/block/src/BlockViewBuilder.php
@@ -63,21 +69,20 @@ public function viewMultiple(array $entities = array(), $view_mode = 'full', $la
+          'contexts' => Cache::mergeContexts($default_cache_contexts, $plugin->getCacheContexts()),

Sigh, I forgot to delete the original line below.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Needs review
StatusFileSize
new block_view_builder_cache_contexts-2458413-5.patch4.26 KB
new interdiff.txt804 bytes

Such a silly mistake.

fabianx’s picture

fabianx commented
Status: Needs review » Reviewed & tested by the community
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

This issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed fd3a981 and pushed to 8.0.x. Thanks!

  • alexpott committed fd3a981 on 8.0.x 
    Issue #2458413 by Wim Leers: BlockViewBuilder should specify cache...

Status: Fixed » Closed (fixed)

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