Blocks are invisible while selecting content types

Hmm, so why didn't this test fail before then ?

Quick guess why the test did not fail: probably because it sets the block configuration up in a different way from the interface?
The patch also changes the test to set the context mapping as 'node' not 'node.node'

Besides the unclear reason for this change, it seems to work fine!

Side note: shouldn't the test use assertBlockAppears and assertNoBlockAppears instead of assertText/assertNoText with the block-label ?

Looks like both the code and the set up of the test had the same error (using node.node). In the test we are creating the settings via entity API instead of posting to the block settings page, that's why it didn't fail. But NodeRouteContext::onBlockAdministrativeContext uses node (see http://cgit.drupalcode.org/drupal/diff/?id=633d842, #2339151: Conditions / context system does not allow for multiple configurable contexts, eg. language types).

I suggest to use node as the patch already does, as this is also used as an example in Drupal/block/EventSubscriber/BlockContextSubscriberBase docblocks. If we decide to switch to node.node that should be changed too.

Do we really need to alter the tests for using a post to the block settings form?

Attached patch allowed me to select content type block filters again. All worked as I expected.

Patch needs reroll for latest drupal 8.0.x

You beat me to it. Thanks @sidharthap.

I think this is the wrong fix. Contexts need to be namespaced otherwise once contrib comes in we're going to have problems.

Also this is at least a major bug.

I agree with alex, let me look at this today, and I'll see if I can make sense of the problem.

Eclipse

<?php
public function onBlockAdministrativeContext(BlockContextEvent $event) {
    $context = new Context(new ContextDefinition('entity:node'));
    $event->setContext('node', $context);
  }
?>

Maybe this can change to node.node? Just a guess, not too sure. I tried it, seems to work.

Side note: shouldn't the test use assertBlockAppears and assertNoBlockAppears instead of assertText/assertNoText with the block-label ?

Could be a good improvement.

Agreed with @alexpott, so I have added context as namespace

Sorry, I didn't understand the change in #23, this uses #15 as base.

I used the namespaced context 'node.node' and changed the example as said in #9. IMHO the improvement of the test is not a major, so let's do that in a follow-up and get this fixed.

This needs tests in this issue, not in a follow-up.

We could also make it more clear what we are using as a namespace (module name, etc.) Add doc to setContext() ?

So for tests, we add the block settings form post?

We have tests, is only that they are bogus. Let's do what I suggested at #9 for posting the block config instead of (ab)using the API.

The attached patch creates the block by posting to the block settings form instead of using the entity API. The only-test patch exposes the bug as it is in core.

Updated issue summary and added beta evaluation template.

1. +++ b/core/modules/node/src/Tests/NodeBlockFunctionalTest.php
   @@ -120,18 +120,15 @@ public function testRecentNodeBlock() {
   +    $theme = $this->config('system.theme')->get('default');
   

   Please use \Drupal::service('theme_handler')->getDefault();

2. +++ b/core/modules/node/src/Tests/NodeBlockFunctionalTest.php
   @@ -120,18 +120,15 @@ public function testRecentNodeBlock() {
   +    $block = entity_load('block', $edit['id']);
   

   Please use Block::load($edit['id']);

Sure, thanks!

Good find, good fix, good test coverage.
Thanks!

Minor comment length fix.

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