If you add a form element of type machine_name and the source element for the machine name element is defined after the machine name element itself, notices are thrown upon submitting the form:

    * Notice: Undefined index: #id in form_process_machine_name() (line 3293 of /home/mrbaileys/workspace/Drupal-7-HEAD/includes/form.inc).
    * Notice: Undefined index: #id in form_process_machine_name() (line 3306 of /home/mrbaileys/workspace/Drupal-7-HEAD/includes/form.inc).
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sun’s picture

Status: Active » Needs review
FileSize
1.52 KB

Should have used #after_build instead of #process.

mr.baileys’s picture

Status: Needs review » Needs work

Still getting notices after applying the patch, but now in the after_build() function:

    * Notice: Undefined index: #id in form_after_build_machine_name() (line 3293 of /home/mrbaileys/workspace/Drupal-7-HEAD/includes/form.inc).
    * Notice: Undefined index: #id in form_after_build_machine_name() (line 3306 of /home/mrbaileys/workspace/Drupal-7-HEAD/includes/form.inc).

For reference, here's the partial/shortened form I'm using:

  $form['name'] = array(
    '#type' => 'machine_name',
    '#title' => t('Name'),
    '#default_value' => 'default',
    '#machine_name' => array(
      'source' => array('title'),
    ),
  );
  
  $form['title'] = array(
    '#type' => 'textfield',
    '#title' => t('Title'),
    '#default_value' => $title,
  );
sun’s picture

Status: Needs work » Needs review
FileSize
842 bytes

I see, let's try this one instead.

mr.baileys’s picture

Status: Needs review » Needs work

Nope, sorry:

    * Notice: Undefined index: #parents in form_process_machine_name() (line 3295 of /home/mrbaileys/workspace/Drupal-7-HEAD/includes/form.inc).
    * Warning: implode(): Invalid arguments passed in form_process_machine_name() (line 3295 of /home/mrbaileys/workspace/Drupal-7-HEAD/includes/form.inc).
sun’s picture

hm... so it actually would have to be a #pre_render, but still needs access to $form_state, and that callback would have to be invoked before either of both elements is attempted to be rendered... very tricky. :-|

dwightaspinwall’s picture

Subscribing. Happened to me too; confusing until I saw this issue.

Pol’s picture

If you manually add the '#id' to the machine name and the field in 'source', the error is gone.

sun’s picture

Version: 7.x-dev » 8.x-dev
Issue tags: +Needs backport to D7
torotil’s picture

Issue summary: View changes

In D7 this also leads to a JS error: The empty #id is put into Drupal.settings.machineName and the behavior then tries to do a $('#', …) which throws a syntax error.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

taggartj’s picture

Still having this issue in a config entity form (8.3.4)

// Works 
$form['label'] = [
      '#type' => 'textfield',
      '#title' => $this->t('Label'),
      '#maxlength' => 255,
      '#default_value' => $fee_info->label(),
      '#description' => $this->t("Label for the Fee info."),
      '#required' => TRUE,
      '#weight' => -5,
    ];
// Never ever ever put this field at the top of  the form ¯\_(ツ)_/¯.
    $form['id'] = [
      '#title' => $this->t('FEE ID'),
      '#type' => 'machine_name',
      '#default_value' => $fee_info->id(),
      '#machine_name' => [
        'exists' => '\Drupal\dpe_fee_data\Entity\FeeInfo::load',
      ],
      '#disabled' => !$fee_info->isNew(),
      '#field_prefix' => 'FEE-',
      '#weight' => -10,
    ];

but if you put the machine_name field above the you get the "label" notice:
"Notice: Undefined index: #id in Drupal\Core\Render\Element\MachineName::processMachineName() (line 174 of core/lib/Drupal/Core/Render/Element/MachineName.php). "

tim.plunkett’s picture

Version: 8.3.x-dev » 8.4.x-dev
Status: Needs work » Needs review
FileSize
1.51 KB

I don't see any realistic approach to fixing this. I think we need to inform the developer that they must define the form elements in the correct order.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

nortmas’s picture

I've added a new field in the custom module through the yml file for the commerce_store entity which doesn't implement field api.
Despite the fact that I've added wight property into core.entity_form_display.commerce_store.online.default.yml it doesn't affect anything and I still see this error, however, the machine_name field is placed after source field.
Unfortunately, I don't see any approach to change the render order in this case.
Maybe someone knows how to fix it?

borisson_’s picture

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

I think #17 is a different problem. I agree with #14 on this being really hard to solve in a good way and that this is a good fix. We should get a test for this as well, to make sure we don't regress the new behavior.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Lendude’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
1.93 KB
3.62 KB

Here is a test for the new thrown exception. I've modified the message a bit to specify that you can also set an id on the source element to work around the problem.

The test-only patch test is a little different than the one with the fix but it illustrates the notice that gets thrown.

Lendude’s picture

fix the cs fail

The last submitted patch, 25: 990218-25-TEST_ONLY.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 26: 990218-26.patch, failed testing. View results

Lendude’s picture

Status: Needs work » Needs review
FileSize
944 bytes
3.6 KB

Fix the test

joachim’s picture

+++ b/core/lib/Drupal/Core/Render/Element/MachineName.php
@@ -183,6 +184,13 @@ public static function processMachineName(&$element, FormStateInterface $form_st
+      throw new \LogicException(sprintf('The machine name element "%s" is defined before the source element "%s", it must be defined after or the source element must specify an id.', $element_parents, $source_parents));

Is this always true? Would this exception also be thrown if the source element was missing from the form?

Lendude’s picture

@joachim there is a guard statement for that before reaching this logic

    $source = NestedArray::getValue($form_state->getCompleteForm(), $element['#machine_name']['source'], $key_exists);
    if (!$key_exists) {
      return $element;
    }

So no, it won't be thrown in that case.

joachim’s picture

Status: Needs review » Reviewed & tested by the community

Thanks! In which case, happy to say RTBC.

alexpott’s picture

Version: 9.3.x-dev » 9.2.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 25718d36d4 to 9.3.x and 4ba14b66af to 9.2.x. Thanks!

  • alexpott committed 25718d3 on 9.3.x
    Issue #990218 by Lendude, sun, tim.plunkett, mr.baileys, joachim:...

  • alexpott committed 4ba14b6 on 9.2.x
    Issue #990218 by Lendude, sun, tim.plunkett, mr.baileys, joachim:...

Status: Fixed » Closed (fixed)

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