Problem/Motivation

When using PHP 8.5 with the metatag module it emits a PHP deprecation warning:

Deprecated function: Using null as an array offset is deprecated, use an empty string instead i Drupal\metatag\Form\MetatagDefaultsForm->form() (linje 164 af /app/platform_drupal_11_4_x/modules/contrib/metatag/src/Form/MetatagDefaultsForm.php) #0 /app/platform_drupal_11_4_x/core/includes/bootstrap.inc(104): _drupal_error_handler_real()
      #1 /app/platform_drupal_11_4_x/modules/contrib/metatag/src/Form/MetatagDefaultsForm.php(164): _drupal_error_handler()
      #2 /app/platform_drupal_11_4_x/core/lib/Drupal/Core/Entity/EntityForm.php(108): Drupal\metatag\Form\MetatagDefaultsForm->form()
      #3 [internal function]: Drupal\Core\Entity\EntityForm->buildForm()
      #4 /app/platform_drupal_11_4_x/core/lib/Drupal/Core/Form/FormBuilder.php(559): call_user_func_array()
      #5 /app/platform_drupal_11_4_x/core/lib/Drupal/Core/Form/FormBuilder.php(299): Drupal\Core\Form\FormBuilder->retrieveForm()

Steps to reproduce

- Install Drupal 11.4 running PHP 8.5
- Install and activate latest 2.2.x dev version of drupal/metatag.
- Activate dblog.
- Update/save the metatag information for content - /admin/config/search/metatag/node
- Observe the error in the message container or in dblog - /admin/reports/dblog

Proposed resolution

See merge request.

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork metatag-3618075

Command icon 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:

About issue forks

Comments

flemming.fridthjof created an issue. See original summary.

flemming.fridthjof opened merge request !260

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Active » Needs work

Thank you for reporting this and creating the MR.

This is annoying - the !empty() logic used to handle this transparently and it was a useful feature.

I think it would be better like this:

    if (isset($entity_type) && !is_null($entity_bundle) && !empty($entity_type_groups[$entity_type]) && !is_null($entity_bundle[$entity_type]) && !empty($entity_type_groups[$entity_type][$entity_bundle])) {
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Parent issue: » #3548158: Plan for Metatag 2.2.1

damienmckenna changed the visibility of the branch 2.2.x to hidden.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

I'm just surprised that this error doesn't show more often..

Out of interest, are you using any extra modules that interact with Metatag, or have any custom meta tag plugins on your site?

flemming.fridthjof’s picture

flemming.fridthjof commented

Don't think so. The warning also only appears for me if I edit/update the default metatag information for Node (MetatagDefaultsForm), which also explain why entity_bundle is null.
Do you want me to refactor the code too what you prefer in https://www.drupal.org/project/metatag/issues/3618075#comment-16752928 (Comment no. #3) ?

I'm a bit uncertain about !is_null($entity_bundle[$entity_type]) because as I read the code, $entity_bundle will never be an array, but either a string or null.
Lines, in src/Form/MetatagDefaultsForm.php, which sets the value is 155 (null) and 160 ($type_parts[1] ?? NULL).
So !is_null($entity_bundle[$entity_type]) will generate a PHP fatal error:
Fatal error: Uncaught TypeError: Cannot access offset of type string on string.

brian@graith’s picture

brian@graith commented

I reproduced this on Drupal 12.0.0-alpha1 with PHP 8.5.10 and tested MR !260.
The code change is correct and I would RTBC it, but two of the three files in
the MR should come out first, and there is a reason the suggestion in #3 will
not work.

Reproducing it

With metatag 2.2.x unmodified, visiting /admin/config/search/metatag/node logs

Deprecated function: Using null as an array offset is deprecated, use an
empty string instead in Drupal\metatag\Form\MetatagDefaultsForm->form()
(line 164)

exactly as reported. The trigger is that the "node" default has no bundle, so
$entity_bundle stays NULL at line 160 and line 164 does
$entity_type_groups['node'][NULL].

Why #3 would not work

@DamienMcKenna, the suggestion in #3 adds !is_null($entity_bundle[$entity_type]).
@flemming.fridthjof is right in #6 that this is a fatal rather than a stricter
check. $entity_bundle is a string or NULL, never an array, so on PHP 8.5:

$bundle = 'article'; $bundle['node'];
-> TypeError: Cannot access offset of type string on string

$bundle = NULL; $bundle['node'];
-> Warning: Trying to access array offset on null (returns NULL)

So on any bundle-level default, which is the common case, that line would take
the site down. The simpler !is_null($entity_bundle) in the MR is the right
shape.

Verifying the MR's code change

With only the MetatagDefaultsForm.php hunk applied, on the same site:

/admin/config/search/metatag/node 200, no deprecation
/admin/config/search/metatag/global 200, no deprecation
/admin/config/search/metatag/node__article 200, no deprecation

I also checked the behaviour the guard protects rather than just the absence of
the warning. With entity_type_groups set to node.article = basic:

node__article renders only "Basic tags", so the group filtering still works
node renders "Basic tags" and "Advanced", so the fallback to all
groups still works

That matches the old behaviour, because $array[NULL] was being read as
$array[''] and was always empty, so this branch was already falling through to
the else. The change makes that explicit instead of accidental.

metatag's unit tests pass on Drupal 12: 10 tests, 43 assertions.

The two files I would take out of the MR

1. composer.json, "php": ">=8.0" to ">=8.5". This one worries me. metatag
declares drupal/core ^10.3 || ^11, and those need:

Drupal 10.3 and 10.6 PHP >= 8.1
Drupal 11.4 PHP >= 8.3
Drupal 12.0 PHP >= 8.5

So requiring PHP 8.5 would make metatag uninstallable through Composer on
every Drupal 10 and Drupal 11 site, which is the module's whole supported
range. The deprecation fix itself does not need PHP 8.5; !is_null() behaves
identically on 8.1 through 8.5. Fixing a warning should not drop support for
the versions the warning does not even appear on.

2. .gitlab-ci.yml, CORE_PHP_MIN: "8.5". Same reasoning from the other side.
That stops CI exercising the PHP versions most installs actually run, which
is where a regression would hurt.

With those two hunks removed I think this is ready.

Two things I noticed while testing, not for this issue

Both are separate from the deprecation and I have not touched either.

- metatag's functional tests cannot run on Drupal 12 at all. Every one of the
24 classes under tests/src/Functional errors with "Functional/FunctionalJavascript
test classes must specify the #[RunTestsInSeparateProcesses] attribute". None
of them carry it.
- The kernel suite aborts before it starts because
tests/src/Kernel/Migrate extends Drupal\Tests\migrate_drupal\Kernel\d6\MigrateDrupal6TestBase,
and Drupal 12 removed the d6 and d7 migrate test infrastructure; migrate_drupal
is now marked lifecycle: obsolete.

Given 2.2.x already declares ^12 in its info file, those two are probably worth
their own issue. Happy to open them with the detail if that helps.

This work was done with AI assistance. Every error string, version number and
result above came from running the code on the site described.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs work » Reviewed & tested by the community

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Reviewed & tested by the community » Fixed

Committed. Thanks everyone!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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