Block forms should use #type => machine_name

I think this is a duplicate of #1875016: Automatically generate machine names for block plugins?

I guess doing it in two steps is probably better. I indicated that that issue should be postponed on both this one and the title visibility change.

I think #1875016: Automatically generate machine names for block plugins proposes making it a machine name with a source attribute, this one is about making it a machine name not necessarily with a source attribute. So if this one is implemented then #1875016: Automatically generate machine names for block plugins is down to a *one line change* I believe. Is it valuable to track issues on this granularity?

http://drupal.org/files/block_machine_name.png is a big enough WTF usability-wise, that I believe fixing it should be (at least) a major task. Escalating the postponed issue to major as well. I kinda agree with Gábor that we could just do this all in one patch, and that'd be easier to wrangle.

@tim: interestingly I cannot reproduce the block language test failure. WIth the custom block test I'm getting "Fatal error: Call to a member function label() on a non-object in [....]/core/modules/block/lib/Drupal/block/Tests/BlockTest.php on line 283"

So looks like the fails were down to machine names failing due to uppercase letters. Some places in the test already ensured lowercase machine names, filled in the missing ones :) Same appeared in block language test, seems like that not failing for me for the first time was of sheer luck :D

YAY! Exciting.

Before:

After:

MUCH more betterer!

If I go to configure the Powered By Drupal block twice, both times it gives me the machine name powered_by_drupal. Interestingly, when you do this, the latest configuration wins. So even if you had a "Recent comments" block with the machine name of why_do_i_have_to_always_fill_this_out_by_hand_argh in the left sidebar, if you add a "Powered by Drupal" block with the same machine name to the footer, the Recent Comments block goes away.

However, this seems to be caused by #1887654: Creating a config entity with an existing machine name shouldn't work, which is a pre-existing issue, regardless of this patch. Therefore!

Committed and pushed to 8.x. Thanks! :D

Retroactively tagging as Spark as well, since this is part of the stuff we're doing at http://buytaert.net/spark-update-unified-in-place-editing.

+++ b/core/modules/block/lib/Drupal/block/BlockBase.php
@@ -241,11 +241,15 @@ public function form($form, &$form_state) {
       '#description' => t('A unique name to save this block configuration. Must be alpha-numeric and be underscore separated.'),
...
+        'replace_pattern' => '[^a-z0-9_.]+',

@@ -419,12 +423,7 @@ public function blockForm($form, &$form_state) {
+    if (!empty($form['machine_name']['#disabled'])) {
       $config_id = explode('.', $form_state['values']['machine_name']);
       $form_state['values']['machine_name'] = array_pop($config_id);
     }

The #description is still incorrect and misleading though? It doesn't mention that a dot is allowed, and, that there's some special logic is going on for the dot.

Speaking of, why do we allow the dot, if

A) the code doesn't actually support multiple dots in the machine name?

B) everything before the last dot is stripped off the machine_name value in the form validation handler (for existing blocks)?

It looks like this could be vastly simplified, if

1) the machine name prefix (before the dot) was moved into a separate #type 'value' in the form already

2) the machine_name only ever contains the user-supplied machine_name

3) the user-supplied machine_name cannot contain dots, thus, removing the custom replace_pattern entirely.

#15 sounds like a better idea to me as well. Allowing a in the machine name would make it harder to solve things like #1831774: Config import assumes that 'config_prefix' contains one dot only and #1888218: Default configuration entities provided by a module should include the module name in the machine name, and it's still a bit weird for the theme prefix to magically appear after saving the block instance.

Edit: see also #1875016-7: Automatically generate machine names for block plugins and -8.

Reopening at normal for further discussion.

Oops, there's a bug: Without opening the machine name field, for a non-custom block, blank out the title and click save. And click save again. And again. Nothing happens! And there's no explanation of why. :)

#1875260: Make the block title required and allow it to be hidden will solve this indirectly, but it's a separate bug with our machine name field here, independently of the title being required. I tested the Field API, and if you leave a field label blank and submit the form, the machine name field still opens up when it fails.

As followup to #15 I did try making a stark.starkers block in Bartik to see if I could magically place it in a different theme by naming it incorrectly, but it didn't mysteriously appear in Stark, so we don't have to worry about that at least. It might be worth checking to see if it gets loaded but not rendered.

@xjm: That sounds like a general bug with the machine name field. I tried extensively to reproduce this bug. It can only be reproduced with adding a NEW block, because you cannot edit the machine name for an existing block. So if you are adding a new block and do not provide any title, then indeed, you can try to save the block, but the button will not work and no explanation will be provided. This is since the machine name is a required field (but is not shown) but the title is not a required field (however it is shown).

I went to try and reproduce this with (1) vocabularies, (2) content types or (3) fields, however all of them have their label/title as required, so you cannot just set the title to an empty string. That field exposes the error message to the user so no problem on either. The block form has this issue because it makes a non-visible required field depend on a non-required visible field. I think this is easily solved by making the title required that is in our goals anyway to increase usability. See #1875260: Make the block title required and allow it to be hidden. If we believe the general machine_name facility needs to work with this kind of bugos setup, then that should be it's own issue submitted for machine names I think.

As for problems actually related to this issue, I agree with @sun's notes on the non-standardness of the machine name from #15 (I said earlier as well). Not sure whether this should be the issue to fix that. @sun: do you mind opening a new issue for those?

(Moving back to earlier status).

@Gábor Hojtsy:
Mmmm... we're past feature freeze, so the overall and entire focus of Drupal core development is on cleaning up that entire mess ;) Therefore, we should never mark issues as fixed before all problems have been addressed.

But ok, I've copied my comment into #1925024: Block plugin machine name UX is flawed — we perhaps want/need to add @xjm's concerns to that.

Thanks!

@sun: thanks! I already recategorized #1875260: Make the block title required and allow it to be hidden as a bug for @xjm's concern.