Placing a block with same machine name of region causes region to disappear in admin UI [#2508547]

I created a menu named Header with machine name of header. When I placed the block into a region of the same name (header), the region is no longer listed in the UI. The block does get placed, and the block's region dropdown shows being set to Header. Screenshot attached of block UI. Deleting the block causes the Header region to show in the UI again.

Steps to reproduce:

Create a block with machine name of "header" ( Menus and Custom blocks both produce the behavior)
Place block in Block UI, adding it to "header" region

Expected Behavior:
Block is placed and shows in "header" region in UI.

What happened instead:
Block gets placed in header region, but header region disappears from Block UI.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category	Bug because the UI can break under certain circumstances
Issue priority	Normal because the UI is still functional and it doesn't affect runtime
Disruption	Not disruptive because no real changes.

Comment	File	Size	Author
#10	2508547-block-10.patch	2.34 KB	tim.plunkett
#10
#7	interdiff.txt	303 bytes	tim.plunkett
#7	2508547-block-7.patch	21.31 KB	tim.plunkett
#7
#4	Block layout \| Drupal-2015-06-23.png	73.74 KB	tim.plunkett
	Screen Shot 2015-06-18 at 1.16.29 PM.png	98.91 KB	tannerjfco
#2	2508547-block-2.patch	21.34 KB	tim.plunkett
#2
#3	2508547-block-3-PASS.patch	22.11 KB	tim.plunkett
#3
#3	2508547-block-3-FAIL.patch	788 bytes	tim.plunkett
#3

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett at Acquia commented 19 June 2015 at 20:12

Version:	8.0.0-beta11	» 8.0.x-dev
Assigned:	Unassigned	» tim.plunkett

I was able to reproduce. Taking a look into this.

Comment #2

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett at Acquia commented 19 June 2015 at 22:01

Status:	Active	» Needs review
Issue tags:		+Needs tests

File	Size
2508547-block-2.patch	21.34 KB

Still needs tests. I had to refactor a bit to track down the bug.

Comment #3

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett at Acquia commented 23 June 2015 at 15:33

Issue tags:

-Needs tests

File	Size
2508547-block-3-FAIL.patch	788 bytes

2508547-block-3-PASS.patch	22.11 KB

Here's a test.

Comment #4

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett at Acquia commented 23 June 2015 at 15:47

Issue summary:

View changes

File	Size
Block layout \| Drupal-2015-06-23.png	73.74 KB

Also note, it not only removes the region name, it incorrectly puts the faulty block into that section of the page.

Comment #5

23 June 2015 at 16:03

The last submitted patch, 3: 2508547-block-3-FAIL.patch, failed testing.

Comment #6

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett at Acquia commented 23 June 2015 at 16:55

Title:

Placing a block with same machine name of region causes region to disappear in UI

» Placing a block with same machine name of region causes region to disappear in admin UI

The title makes this sound like a MUCH bigger problem, hah.

Comment #7

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett at Acquia commented 30 June 2015 at 19:53

File	Size
2508547-block-7.patch	21.31 KB

interdiff.txt	303 bytes

hook_rebuild() doesn't return anything.

Postponed #2513534: Remove the 'disabled' region from Block UI on this bug fix, since it refactors the block UI code.

Comment #8

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett at Acquia commented 30 June 2015 at 19:56

Issue summary:

View changes

Comment #9

neclimdul

He/Him

CreditAttribution: neclimdul commented 30 June 2015 at 21:41

Status:

Needs review

» Reviewed & tested by the community

Lots of good refactoring. Tried really hard to find a mistake in it but the one I thought I found in block_rebuild even turned out to just be cleanup.

+++ b/core/modules/block/src/BlockListBuilder.php
@@ -192,36 +179,63 @@ public function buildForm(array $form, FormStateInterface $form_state) {
-      $form['blocks'][$region] = array(
+      $form['region-' . $region] = array(
...
-      $form['blocks'][$region]['title'] = array(
...
+      $form['region-' . $region]['title'] = array(
...
-      $form['blocks'][$region . '-message'] = array(
+      $form['region-' . $region . '-message'] = array(

@@ -230,8 +244,8 @@ public function buildForm(array $form, FormStateInterface $form_state) {
-      $form['blocks'][$region . '-message']['message'] = array(
...
+      $form['region-' . $region . '-message']['message'] = array(

The fix. Prefixing the form element so it doesn't collide.

Comment #10

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett at Acquia commented 30 June 2015 at 22:13

Issue summary:	View changes
Status:	Reviewed & tested by the community	» Needs review

File	Size
2508547-block-10.patch	2.34 KB

Okay I got called out for the scope creep. Opening another issue to do the refactoring.
Also we lost the test coverage somewhere.

Comment #11

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett at Acquia commented 30 June 2015 at 22:22

Moved the refactor to #2514998: Reduce fragility in the monolithic BlockListBuilder

Comment #12

neclimdul

He/Him

CreditAttribution: neclimdul commented 30 June 2015 at 22:39

Status:

Needs review

» Reviewed & tested by the community

back to rtbc

Comment #13

jibran

he/him

Urdu

Sydney, Australia

CreditAttribution: jibran as a volunteer commented 1 July 2015 at 01:49

RTBC +1

Comment #14

Wim Leers

Ghent 🇧🇪🇪🇺

CreditAttribution: Wim Leers at Acquia commented 1 July 2015 at 08:34

What an amazing bug btw! :D

Comment #15

5 July 2015 at 09:57

xjm committed 760cd40 on 8.0.x

Issue #2508547 by tim.plunkett, tannerjfco, neclimdul: Placing a block...

Comment #16

xjm

she/her

English

CreditAttribution: xjm at Acquia commented 5 July 2015 at 09:58

Priority:	Normal	» Major
Status:	Reviewed & tested by the community	» Fixed

This is delightful. Bumped to major as someone who encountered this on a site would have no clue what was going on.

I manually tested to double-check that the change to the form structure is not linked into block entity configuration (in order to confirm that there is no data model change and no update path needed).

This issue is a major bug fix, and the only disruption is in changing the array structure of the block form, so it is allowed per https://www.drupal.org/core/beta-changes.