TermForm::buildEntity does not process the parent property properly when the form element is #disabled

Do we not need to update the comment as well?
I have made that update.

I've applied termform_buildentity-2637538-4.patch to branch 8.0.x.

-Patch applied cleanly
-Taxonomy remained functional

A more experienced tested may want to create their own form and test.

I have done some additional testing of this issue, using xdebug.

When a term is saved, the parent value in the $form_state is structured as @hchonov said:

$form_state = array(
   //...
   'values' => array(
     //...
     'parent' => array(
       2 => '2',
      ),
     //...
    ),
    //...
  );

When the 'parent' field is disabled by setting the '#disabled' attribute to TRUE (I completed this using a form alter):

/**
 * Implements hook_form_alter().
 */
function termformtest_form_taxonomy_term_tags_form_alter(&$form, \Drupal\Core\Form\FormStateInterface $form_state, $form_id) {
  // disable parent attribute.
  $form['relations']['parent']['#disabled'] = TRUE;
}

Then the form state value for 'parent' is different. The keys are no longer the parent id's, but delta values.

$form_state = array(
   //...
   'values' => array(
     //...
     'parent' => array(
       0 => '2',
      ),
     //...
    ),
    //...
 );

So TermForm::buildEntity() will copy the delta values and not the parent ids in this instance.

I'm not sure if this patch is actually needed as the ability to set the parent term works without it and I think this is a very limited edge case. But it does take the array structure differences into account, handling both states (enabled and '#disabled') the same way.

RTBC?

This bug looks testable.

Bumping this to major since it's a PHP error from using the standard UI.

termform_buildentity-2637538-4.patch applies on 8.2.x.
Still needs tests.

termform_buildentity-2637538-4.patch does not allow creation of the term even with the patch applied. And still get the error "Warning: array_values() expects parameter 1 to be array, null given in Drupal\taxonomy\TermForm->buildEntity() (line 113 of core/modules/taxonomy/src/TermForm.php)." when creating the new term.

Hello, Patch #4 doesn't fix the problem, May this patch fix the problem.

For what it's worth I can confirm that abu-zakham's code:

if(isset($form['relations']['parent'])) {
      $term->parent = array_keys($form_state->getValue('parent'));
    }

solves the problem I had of terms not being added (or shown). The patch did not.
Working on a drupal 8.x.4-dev. Migrated from Drupal 6 to 8. Lots of contributed modules.

I was redirected here from https://www.drupal.org/node/2816497#comment-11810162

This needs a test cover the edge case situation.

One more minor thing, there should be a space after the "if".

+    if(isset($form['relations']['parent'])) {
+      $term->parent = array_keys($form_state->getValue('parent'));
+    }

I´m with the same error:
Drupal 8.4.4
php 7.1
* migrated content from D6 to D8
When add a taxonomy term appears to be ok, but the term is not listed and return this warning.

Sugestion #25, worked for me!

As per #26 suggestion. Update the patch file.

Fixed the problem. thank you.

Adding test for version 8.5.x and 8.6.x in patch #29

I don't think this is a bug.

According to W3C:

When set, the disabled attribute has the following effects on an element:

Disabled controls do not receive focus.
Disabled controls are skipped in tabbing navigation.
Disabled controls cannot be successful.

(A successful control is "valid" for submission.)

This means that if you implement hook_form_alter to set the 'disabled' attribute on your form element, you should not expect that element to be processed on the server when the form is submitted.

That being said, although there *may* be something improper about the way this is being handled, the end result (I think) is the correct one.

So what to do if you want to make a dropdown select non-selectable and yet still be "successful"? That is a complicated question (and obviously not specific to Drupal). See: (Stack Overflow) HTML form readonly SELECT tag/input

As an extra FYI, there's a bit of documentation in FormBuilder::handleInputElement that explains Drupal's handling of 'disabled' and 'readonly':

 // Setting #disabled to TRUE results in user input being ignored regardless
  // of how the element is themed or whether JavaScript is used to change the
  // control's attributes. However, it's good UI to let the user know that
  // input is not wanted for the control. HTML supports two attributes for:
  // this: http://www.w3.org/TR/html401/interact/forms.html#h-17.12. If a form
  // wants to start a control off with one of these attributes for UI
  // purposes, only, but still allow input to be processed if it's submitted,
  // it can set the desired attribute in #attributes directly rather than
  // using #disabled. However, developers should think carefully about the
  // accessibility implications of doing so: if the form expects input to be
  // enterable under some condition triggered by JavaScript, how would someone
  // who has JavaScript disabled trigger that condition? Instead, developers
  // should consider whether a multi-step form would be more appropriate
  // (#disabled can be changed from step to step). If one still decides to use
  // JavaScript to affect when a control is enabled, then it is best for
  // accessibility for the control to be enabled in the HTML, and disabled by
  // JavaScript on document ready.
  if (!empty($element['#disabled'])) {
    if (!empty($element['#allow_focus'])) {
      $element['#attributes']['readonly'] = 'readonly';
    }
    else {
      $element['#attributes']['disabled'] = 'disabled';
    }
  }

Cheers -

OK, so read through the issue and here's my thoughts:

• The issue summary is a valid description of the issue and the patch in #2 fixes it correctly
• #4 adds a comment to the patch in #2 but I don't think the comment is correct. The code does actually re-key the values so that they have keys starting with 0, both before and after the patch. (The patch is just changing the values to be correct.)
• #9 contains a very nice and thorough explanation of the issue and a manual verification that it is correct
• #10 correctly points out this automated tests are needed for this to go in
• The patches in #20 and #29 deal with a completely separate issue that seems like a won't fix to me, but in any case should be dealt with in a separate issue. This is already pointed out by #23
• #33 elaborates on the details of disabled form elements in general. I don't think that is relevant here, though, as Drupal is not actually processing those the parents from the user input if the form element is disabled, but from the server-prepared #default_value key.

So this issue has been derailed for a while and really just needs an automated test.

So this now provides a test. Unfortunately I didn't find a test which deals with parents through the UI at all, so I wrote one which at least tests the basic functionality of adding and editing parents before testing with a disabled parents form element. This revealed that the order of parents (i.e. the field item list deltas) will be different for terms with multiple parents depending on whether or not the parent form element is disabled or not. This is a bit quirky, but not really a bug, so the test explicitly tests this for now, so if we ever decide to make this consistent, we can update the test directly.

The test-only patch is also the interdiff, which is relative to #2.

The issue is still there. I'm using Drupal 8.5.4. I've used migrate module to migrate my existing vocabularies from drupal7. The vocabularies gets created. But, when you try to add terms i'm getting the message, term is saved but with following warnings

Warning: array_values() expects parameter 1 to be array, null given in Drupal\taxonomy\TermForm->buildEntity() (line 118 of core/modules/taxonomy/src/TermForm.php).
Warning: array_values() expects parameter 1 to be array, null given in Drupal\taxonomy\TermForm->buildEntity() (line 118 of core/modules/taxonomy/src/TermForm.php).
Warning: count(): Parameter must be an array or an object that implements Countable in Drupal\taxonomy\TermForm->save() (line 144 of core/modules/taxonomy/src/TermForm.php).

And the terms that got created are missing from the list. Tried applying #34's patch. But its still failing

I upgraded from 6 to 8.5

Currently running 8.5.4 along with php 7.0

Warning: array_keys() expects parameter 1 to be array, null given in Drupal\taxonomy\TermForm->buildEntity() (line 118 of core/modules/taxonomy/src/TermForm.php) #0

Re #36. I think you are hitting a similar - but different - issue. It seems as though the parent field is completely removed in your case, not just disabled. Maybe you can open a new issue and add some steps to reproduce your error as otherwise it will be hard to provide any help.

I believe it's time to set it to RTBC. Arguments laid out in #34 seem valid to me, patch applies to 8.9.x, fixes the issue, and provides the overall test coverage for term parents. Just the minor thing - shouldn't be the test class called TermParentTest instead of TermParentsTest? Drupal always sticks to singular.

Fixing the test defaultTheme deprecation

So @jeetendrakumar and @abu-zakham are not credited because as per #34 the patches were fixing another issue entirely.

Credited @mikemiles86 for a reviewing the issue.

I think we should apply this to 8.8.x as a bug fix asking another committer for a +1.

Discussed with @catchwho +1'd to 8.8.x backport