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

i18n conflict #47

Closed laryn closed 2 years ago

laryn commented 4 years ago

I was testing a PR for i18n_node and when I enabled that module I got this fatal error:

Error: Call to undefined function ctools_include() in metatag_i18n_list_metatag_config() (line 134 of /path/modules/metatag/metatag.i18n.inc).

I had to delete that file to get the site to load again.

indigoxela commented 4 years ago

I could reproduce it. And it wasn't when enabling i18n_node, but when enabling i18n_string.

laryn commented 3 years ago

PR for testing: https://github.com/backdrop-contrib/metatag/pull/69

mazzech commented 3 years ago

So is this the same as I just saw here? Can't get the metatag module working on a multilingual site (i18n enabled)

Warning: Invalid argument supplied for foreach() in metatag_config_load_multiple() (line 312 of /home/lugoture/www/backdrop.mazzemedia.ch/modules/metatag/metatag.module).

Screenshot 2021-05-05 at 15 10 07 Screenshot 2021-05-05 at 15 08 48

indigoxela commented 3 years ago

@mazzech Rename the file metatag.i18n.inc to something else and flush caches - if that brings your site back, the problem is also related to this outdated include file.

Your error messages seems to be different, though. At least at first sight. But let's verify that first.

mazzech commented 3 years ago

Mmh, does not really fix it (after deactivation, deinstallation, reactivation), but now it's a Ctool dependency...? Screenshot 2021-05-06 at 11 01 40

laryn commented 3 years ago

@mazzech Did you test this PR by any chance? https://github.com/backdrop-contrib/metatag/pull/69/files

mazzech commented 3 years ago

Oh, no I did not... thank you @laryn

mazzech commented 3 years ago

Hi Laryn, I did apply the patches manually... at least the module activation does not throw an error anymore! But clicking the settings shows an error in the log like

Screenshot 2021-05-09 at 12 42 24

Line 135 is about the i18n_string dependency, right? Screenshot 2021-05-09 at 12 43 13

but when I got to the string settings page, I am not sure if I even can disable this sub module entierly, given the settings. Screenshot 2021-05-09 at 12 45 11

@indigoxela do you know more about i18n_string module status?

indigoxela commented 3 years ago

do you know more about i18n_string module status?

It's an alpha release. However, these bugs are in the metatag module and have to get fixed here. I can't help much, as I usually avoid to install metatag (and it's a pretty complicated module).

indigoxela commented 3 years ago

The error message is odd... $settings->get('i18n_disabled') - that setting ships with metatag.settings.json, I'm not sure why it should be null.

Some lines above the setting gets loaded: $settings = config('metatag.settings');

@mazzech could you inspect your config? You can export it via /admin/config/development/configuration/single/export. Choose "Configuration" and "Metatag settings".

mazzech commented 3 years ago

Thank you @indigoxela for checking... here's my export, hope this helps

{
    "_config_name": "metatag.settings",
    "load_defaults": 1,
    "load_all_pages": 1,
    "entity_no_lang_default": 0,
    "tag_admin_pages": 0,
    "extended_permissions": 0,
    "cache_output": 0,
    "leave_core_tags": 0,
    "token_sanitize": 0,
    "pager_string": "Page PAGER | ",
    "i18n_disabled": 0,
    "i18n_translate_output": 0,
    "i18n_enable_watchdog": 0,
    "metatag_enable_node": 1,
    "metatag_enable_node__page": 1,
    "metatag_enable_node__post": 1,
    "metatag_enable_taxonomy_term": 1,
    "metatag_enable_taxonomy_term__tags": 1,
    "metatag_enable_user": 1,
    "enabled_tags": {
        "title": "title",
        "description": "description",
        "image_src": "image_src",
        "canonical": "canonical",
        "content-language": "content-language",
        "og:site_name": "og:site_name",
        "og:type": "og:type",
        "og:url": "og:url",
        "og:title": "og:title",
        "og:description": "og:description",
        "og:updated_time": "og:updated_time",
        "og:image": "og:image",
        "article:published_time": "article:published_time",
        "article:modified_time": "article:modified_time",
        "abstract": 0,
        "keywords": 0,
        "news_keywords": 0,
        "standout": 0,
        "rating": 0,
        "referrer": 0,
        "generator": 0,
        "rights": 0,
        "shortlink": 0,
        "original-source": 0,
        "prev": 0,
        "next": 0,
        "geo.position": 0,
        "geo.placename": 0,
        "geo.region": 0,
        "icbm": 0,
        "refresh": 0,
        "revisit-after": 0,
        "pragma": 0,
        "cache-control": 0,
        "expires": 0,
        "robots": 0,
        "og:determiner": 0,
        "og:see_also": 0,
        "og:image:url": 0,
        "og:image:secure_url": 0,
        "og:image:type": 0,
        "og:image:width": 0,
        "og:image:height": 0,
        "og:latitude": 0,
        "og:longitude": 0,
        "og:street_address": 0,
        "og:locality": 0,
        "og:region": 0,
        "og:postal_code": 0,
        "og:country_name": 0,
        "og:email": 0,
        "og:phone_number": 0,
        "og:fax_number": 0,
        "og:locale": 0,
        "og:locale:alternate": 0,
        "article:author": 0,
        "article:publisher": 0,
        "article:section": 0,
        "article:tag": 0,
        "article:expiration_time": 0,
        "profile:first_name": 0,
        "profile:last_name": 0,
        "profile:username": 0,
        "profile:gender": 0,
        "og:audio": 0,
        "og:audio:secure_url": 0,
        "og:audio:type": 0,
        "book:author": 0,
        "book:isbn": 0,
        "book:release_date": 0,
        "book:tag": 0,
        "og:video:url": 0,
        "og:video:secure_url": 0,
        "og:video:width": 0,
        "og:video:height": 0,
        "og:video:type": 0,
        "video:actor": 0,
        "video:actor:role": 0,
        "video:director": 0,
        "video:writer": 0,
        "video:duration": 0,
        "video:release_date": 0,
        "video:tag": 0,
        "video:series": 0
    }
}
indigoxela commented 3 years ago

The setting is clearly there... BUT... I just realized, that's a different file - metatag.admin.inc

In that file the config isn't loaded and the "->get" is done in a wrong way.

Two problems:

if (module_exists('i18n_string') && !$settings->get('i18n_disabled', FALSE)) {

  1. Properly load it: $settings = config('metatag.settings');
  2. Properly use it: $config->get('i18n_disabled'); (that function has no second parameter)
  3. Alternatively: config_get('metatag.settings', 'i18n_disabled') - do it on one step

That should fix it. @laryn mind to give it a try in your PR?

laryn commented 3 years ago

Good catch @indigoxela -- PR updated.

herbdool commented 3 years ago

@laryn your PR works for me using latest metatag release and i18n alpha release.

herbdool commented 3 years ago

Even with this patch I'm getting:

Warning: Invalid argument supplied for foreach() in metatag_config_load_multiple() (line 312 of /httpdocs/modules/custom_contrib/metatag/metatag.module).

I need to look into this further, but I wasn't getting this error before applying the patch.

herbdool commented 3 years ago

I made the following changes to @laryn patch and it prevented the previous error. (I should note that on admin/config/metadata/metatags/config/add I was getting the error and broke the select field):

diff --git a/metatag/metatag.module b/metatag/metatag.module
index 7d38d091..e6a21bf9 100644
--- a/metatag/metatag.module
+++ b/metatag/metatag.module
@@ -289,15 +289,14 @@ function metatag_config_load($instance) {
  * @see metatag_config_load()
  */
 function metatag_config_load_multiple(array $instances) {
-  if (!empty($instances)) {
-    $configs = array();
-    foreach ($instances as $instance) {
-      $filename = 'metatag.instance.' . str_replace(':', '.', $instance);
-      $configs[$instance] = config_get($filename);
-    }
+  if (empty($instances)) {
+    $instances = config_get_names_with_prefix('metatag.instance');
   }
-  else {
-    $configs = config_get_names_with_prefix('metatag.instance');
+
+  $configs = array();
+  foreach ($instances as $instance) {
+    $filename = 'metatag.instance.' . str_replace(':', '.', $instance);
+    $configs[$instance] = config_get($filename);
   }

   // Translate the configuration.
@@ -309,6 +308,9 @@ function metatag_config_load_multiple(array $instances) {
     $options['watchdog'] = $settings->get('i18n_enable_watchdog');

     foreach ($configs as $instance => &$config) {
+      if (empty($config['config'])) {
+        continue;
+      }
       foreach ($config['config'] as $tag => &$value) {
         if (isset($value['value']) && is_string($value['value'])) {
           $value['value'] = i18n_string_translate(array('metatag', 'metatag_config', $instance, $tag), $value['value'], $options);