backdrop-contrib / metatag

Add structured metadata, aka "meta tags", for various pages on your site.
GNU General Public License v2.0
3 stars 15 forks source link

Trying to get property of non-object in i18n_string_metatag_config_insert #75

Closed herbdool closed 1 year ago

herbdool commented 3 years ago

Getting this error:

Trying to get property 'instance' of non-object in i18n_string_metatag_config_insert() (line 3080 of /app/modules/custom_contrib/metatag/metatag.module).

with metatag and i18n_string enabled. And first adding a content type at admin/config/metadata/metatags/config/node%3Apage

quicksketch commented 1 year ago

I filed a PR at https://github.com/backdrop-contrib/metatag/pull/106 that fixes the PHP notices, but I'm not entirely sure how i18n_string module works.

It seems that the Backdrop version of hook_metatag_config_insert/update/delete() all pass an array instead of an object. I updated the documentation hook examples and fixed the i18n implementations.

quicksketch commented 1 year ago

Alternatively, we could stop using/supporting i18n_string in metatag module and use config translation strings instead.

herbdool commented 1 year ago

@indigoxela what do you think about this issue?

indigoxela commented 1 year ago

@indigoxela what do you think about this issue?

Not sure... as maintainer of i18n I find the function names Metatag uses for own hook implementations problematic.

Function names like i18n_string_metatag_config_* might make people (wrongly!) assume that this i18n code. At least people who are familiar with the Drupal/Backdrop naming conventions.

Note: I didn't check if that naming convention violation preexisted.

herbdool commented 1 year ago

In metatag.module there's this:

/**
 * Implements hook_metatag_config_insert() on behalf of i18n_string.
 */
function i18n_string_metatag_config_insert($config) {
  $context = 'metatag_config:' . $config->instance;
  metatag_translations_update($config['config'], $context);
}

/**
 * Implements hook_metatag_config_update() on behalf of i18n_string.
 */
function i18n_string_metatag_config_update($config) {
  // Defer to the 'insert' function.
  i18n_string_metatag_config_insert($config);
}

@indigoxela maybe it would more proper to have those hooks in a sub-module instead of using the i18n_string namespace. In case metatag needs to use the hook like metatag_metatag_config_insert() itself.

indigoxela commented 1 year ago

@herbdool I don't think, I'm familiar enough with Metatag module code to provide a solution or even recommend the appropriate approach.