Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

i18n conflict #47

Closed
laryn opened this issue Mar 5, 2020 · 16 comments · Fixed by #69
Closed

i18n conflict #47

laryn opened this issue Mar 5, 2020 · 16 comments · Fixed by #69

Comments

@laryn
Copy link
Member

laryn commented Mar 5, 2020

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
Copy link
Member

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

laryn added a commit to laryn/metatag that referenced this issue Apr 27, 2021
@laryn laryn mentioned this issue Apr 27, 2021
@laryn
Copy link
Member Author

laryn commented Apr 27, 2021

PR for testing: #69

@mazzech
Copy link

mazzech commented May 5, 2021

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
Copy link
Member

indigoxela commented May 5, 2021

@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
Copy link

mazzech commented May 6, 2021

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
Copy link
Member Author

laryn commented May 6, 2021

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

@mazzech
Copy link

mazzech commented May 6, 2021

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

@mazzech
Copy link

mazzech commented May 9, 2021

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
Copy link
Member

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
Copy link
Member

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
Copy link

mazzech commented May 10, 2021

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
Copy link
Member

indigoxela commented May 10, 2021

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
Copy link
Member Author

laryn commented May 10, 2021

Good catch @indigoxela -- PR updated.

@herbdool
Copy link
Contributor

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

@herbdool
Copy link
Contributor

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
Copy link
Contributor

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);

jenlampton added a commit that referenced this issue Mar 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants