Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
If I set taxonomy.settings override_selector to true, creation of terms fail in PHP 8.0.
TypeError : array_values(): Argument #1 ($array) must be of type array, null given dans array_values() (/opt/drupal/web/core/modules/taxonomy/src/TermForm.php ligne 120)
TypeError : count(): Argument #1 ($value) must be of type Countable|array, null given dans count() (/opt/drupal/web/core/modules/taxonomy/src/TermForm.php ligne 173)
Steps to reproduce
drush cset taxonomy.settings override_selector 1
I need to set this configuration to true because I have a vocabulary with a huge amount of terms (> 500 000) without tree structure.
If I let this config to false, this leads to memory allocation fatal error in term form.
But when, this config is set to true, the creation of term fails because $form_state->getValue('parent') returns null and in PHP 8.0 array_values and count don't allow null argument (warning in PHP 7.4).
Proposed resolution
See patch
| Comment | File | Size | Author |
|---|---|---|---|
| 20211011_term_creation_fix_php8.patch | 463 bytes | mounir_abid | |
| #13 | 3242538-13-FAIL.patch | 1.45 KB | danflanagan8 |
| #13 | 3242538-13.patch | 2.44 KB | danflanagan8 |
| |||
| #13 | interdiff_13FAIL-13FIX.txt | 1021 bytes | danflanagan8 |
Issue fork drupal-3242538
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
compare
Comments
Comment #2
cilefen CreditAttribution: cilefen commentedThe default value of this setting is
false. How could automated tests not be failing on this?Comment #3
mounir_abid CreditAttribution: mounir_abid commentedComment #4
mounir_abid CreditAttribution: mounir_abid commentedComment #5
mounir_abid CreditAttribution: mounir_abid commentedSorry, I was wrong, it's the opposite.
you must pass the configuration to true.
Comment #6
cilefen CreditAttribution: cilefen commentedWhat do you mean by that?
Comment #7
mounir_abid CreditAttribution: mounir_abid commentedTo reproduce the issue, you have to set
taxonomy.settings override_selector to true.
Comment #8
cilefen CreditAttribution: cilefen commentedIf this is a broken feature then it is possible to add an automated test that will demonstrate the bug.
Comment #9
danflanagan8This is a duplicate of #3090877: Warnings on terms creating when override_selector = TRUE. This IS is much clearer though (for example, the summary exists!), so I think I'm going to close that one. I'm updating the issue title here too.
Comment #10
danflanagan8Here's a lil test to expose the bug.
There are some other issues related to
override_selectorthat will naturally be able to add to this test. For example, #2985762: Terms created when override_selector = TRUE don't appear on Terms List/Overview page. First things first, though.Comment #11
danflanagan8The patch from @super_romeo on the duplicate issue looks good to me: https://www.drupal.org/files/issues/2019-10-30/3090877-3.drupal.Warnings...
I'm going to test that patch against the new fail test here. The interdiff I'm posting here is identical to @super_romeo's patch.
That patch also appears to solve a related issue #2985762: Terms created when override_selector = TRUE don't appear on Terms List/Overview page. So I've updated the fail test to include a test for that issue. And I'm going to close that as a duplicate.
So we definitely need to give credit for @super_romeo and @DigitalFrontiersMedia, who each put patches up for the issues I've closed.
Comment #12
danflanagan8Comment #13
danflanagan8Ugh, first problem with branch then problem with cspell. I should mark all my test as FAIL. Let's try again and ignore the previous one...
Comment #15
DigitalFrontiersMedia@danflanagan8 Very much appreciated! After 3 years, I figured I was the only one with that issue. :-D
Comment #16
danflanagan8Hey @DigitalFrontiersMedia!
It's possible your issue is technically outdated rather than being a duplicate because I couldn't reproduce it. But your fix was essentially identical to this one. Whatever the case may be, I've added test coverage for your issue here and it passes.
Comment #18
larowlanCredits
Comment #20
larowlanFixing credits, sorry for the noise
Comment #23
smustgrave CreditAttribution: smustgrave at Mobomo commentedTested this out and confirmed the issue on PHP 8.1
With the patch I was able to create terms again.
See the tests-only patch failed too so looks good.
Comment #25
catchKicked off a 10.1.x test run since the last successful one here was against 9.3.x.
Comment #30
alexpottI committed this on 1st august but forgot to mark as fixed :)