Block page visibility option "Show for the listed pages" does not appear to save

I can confirm this behavior is happening in the latest 8.0.x.

A quick test does show that the change noted in the issue summary does appear to solve the problem, but I'm not familiar enough with D8 FAPI so say whether it would have other side-effects.

Here's what I did to test the solution, after following the steps to reproduce above:

--- a/core/lib/Drupal/Core/Render/Element/Radio.php
+++ b/core/lib/Drupal/Core/Render/Element/Radio.php
@@ -57,7 +57,7 @@ public static function preRenderRadio($element) {
     $element['#attributes']['type'] = 'radio';
     Element::setAttributes($element, array('id', 'name', '#return_value' => 'value'));
 
-    if (isset($element['#return_value']) && $element['#value'] !== FALSE && $element['#value'] == $element['#return_value']) {
+    if (isset($element['#return_value']) && $element['#value'] == $element['#return_value']) {
       $element['#attributes']['checked'] = 'checked';
     }
     static::setAttributes($element, array('form-radio'));

Fixing the STR HTML.

It's even worse than the IS suggests: no radio button is selected initially:

(This is when configuring the "Search form" block after a fresh install.)

Seems like this problem goes back to the way condition plugins (see #2339151: Conditions / context system does not allow for multiple configurable contexts, eg. language types, where this was introduced IIRC). The problem doesn't lie in \Drupal\Core\Render\Element\Radio, but in how condition plugins map data from Block config entities to the form.

Pinging @EclipseGc and @TimPlunkett to help.

Adding #2451461: Hide/show block page visibility negate form element if pages form element is/is not empty — created in #4 — to the related issues.

\Drupal\Core\Condition\ConditionPluginBase::buildConfigurationForm uses ['#type' => 'checkbox'] for the negate portion of the form.

For UX reasons, \Drupal\block\BlockForm::buildVisibilityInterface changes the type to radios, and doesn't cast the value.
We used to cast the values, it was probably removed in that commit as well.

@tim.plunkett++

Yep, sure enough.

This was removed in #2339151: Conditions / context system does not allow for multiple configurable contexts, eg. language types:

-    if (isset($form['visibility']['request_path'])) {
-      $form['visibility']['request_path']['#title'] = $this->t('Pages');
-      $form['visibility']['request_path']['negate']['#type'] = 'radios';
-      $form['visibility']['request_path']['negate']['#title_display'] = 'invisible';
-      $form['visibility']['request_path']['negate']['#default_value'] = (int) $form['visibility']['request_path']['negate']['#default_value'];
-      $form['visibility']['request_path']['negate']['#options'] = array(
-        $this->t('Show for the listed pages'),
-        $this->t('Hide for the listed pages'),
-      );
-    }

and replaced with

+    if (isset($form['request_path'])) {
+      $form['request_path']['#title'] = $this->t('Pages');
+      $form['request_path']['negate']['#type'] = 'radios';
+      $form['request_path']['negate']['#title_display'] = 'invisible';
+      $form['request_path']['negate']['#options'] = [
+        $this->t('Show for the listed pages'),
+        $this->t('Hide for the listed pages'),
+      ];
+    }

Turns out that cast *was* important!

However, this was just a confusing UI problem, the values were saved. So this can probably be downgraded.

Yay for having to work around FAPI.
We need this because \Drupal\Core\Condition\ConditionPluginCollection::getConfiguration() will compare 0 and FALSE strictly.

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