Support from Acquia helps fund testing for Drupal Acquia logo

Comments

neurojavi’s picture

Ok, I copy here my comments in other issue for best visibility:

NODES:

I'm getting the global tags for my nodes. If I set a global title (or other tags) for nodes in global settings I get this title and not the global one (I mean that overrides in global settings are working). But if I change the title (or other tag) in the node edit form I don't get the new title on the page.

TERMS:
I edit a term -> change metatag (title and description f.e.) -> save -> metags are changed (I can see the new ones) -> I edit the term again -> the metatags form is filled with default values (the new ones I typed in my first edit are lost) -> I save and I loose my metatag changes (I overwrite with default metatags).

I thought it was a problem with DS but I've completely disabled DS and I'm getting the same results.

I'm using metag 1.0-beta4 and Zen 3.1. My site has english and spanish languages active. Problem checked with default language=english

Thanks.-

DamienMcKenna’s picture

Could anyone who's having problems with beta4 please answer a few questions:

  1. Is the Locale module enabled?
  2. What version did you upgrade from? A recent dev version, beta2, beta3, etc?
  3. Did any errors show when you updated to beta4?
  4. Please check the {metatag} table in your installation, is there a 'language' field?
  5. If the answer to #4 was 'yes', did any of the records have values for the 'language' field?
  6. If the answer to #5 was 'yes', are there any records with no values?
  7. If the answer to #6 was 'yes', is there a pattern to the types of records that have invalid values, e.g. all nodes, all terms, etc?
DamienMcKenna’s picture

I've confirmed the problem with taxonomy terms. Aw crap.

DamienMcKenna’s picture

Status: Active » Needs review
FileSize
1.24 KB

This appears to work for term pages.

DamienMcKenna’s picture

This patch may works for the nodes too, please let me know if it works for your sites. It'd also help if you check the {metatag} table to make sure that records were saved correctly.

neurojavi’s picture

Ok... I've made some tests:

The patch seems to solve the problem with terms (values not loaded in edit form)
The problem in nodes was different: metatags were not loaded on page view (defaults were used). Since you pointed to language as a possible reason for the problem, I made some checks:

- My nodes have lang=es but I was using the site with default lang=en because is easier to export features in english (there's a bug in features that doesn't allow to export in english). With this config metatags where saved (I checked database table) but not used in node view.
- I changed my site default lang to es and metatags were used in node view.

So the problem is solved for me as I only use english to develop...

The only thing I want to know is: is this the expected behaviour? If there are no metatags for an entity in the language used to view the site what is best? to use the global ones or the ones in the original language of the node...?

Thanks!

PD: Just in case you need it I'll answer your questions
Is the Locale module enabled? YES
What version did you upgrade from? A recent dev version, beta2, beta3, etc? I upgraded from beta1 to beta3 and the to beta4
Did any errors show when you updated to beta4? NO
Please check the {metatag} table in your installation, is there a 'language' field? YES
If the answer to #4 was 'yes', did any of the records have values for the 'language' field? YES
If the answer to #5 was 'yes', are there any records with no values? terms are 'und' but this as in term tables I think (my node aliases are 'es' but my term aliases are allways 'all'... do this make sense?)
If the answer to #6 was 'yes', is there a pattern to the types of records that have invalid values, e.g. all nodes, all terms, etc? All nodes (using a different default lang as explained above)

DamienMcKenna’s picture

@neurojavi: Can you please take a look in metatag_entity_view() and add the following to the top of the function:

debug($langcode);

Then add this to metatag_metatag_view() right after the part where it decides the value of $translation but before it assigns the $metatags:

debug($translation);

Then clear the caches and let me know what it displays.

neurojavi’s picture

Hi:

when I have default site lang=en:

In metatag_entity_view() -> en
In metatag_metatags_view() -> en (I debuged a little more and this is assigned in $translation = $options['language']->language;

when I have default site lang=es:

In metatag_entity_view() -> es
In metatag_metatags_view() -> es (I debuged a little more and this is assigned in $translation = $options['language']->language;

Note the node I'm checking has its database language field='es'

1mundus’s picture

Damien, sorry for not reporting on this yet, I didn't have localhost available to perform testing. Hopefully I'll manage to do it during the weekend and then let you know.

DamienMcKenna’s picture

Status: Needs review » Needs work

@neurojavi: The fact that metatag_entity_view() returns 'en' is problematic when the node is set to 'es'. I'm doing some tests locally and I've reproduced the problem, so let me work on it a bit.

DamienMcKenna’s picture

I dug into this a bit and have requested support from the core maintainers on the correct approach: #1849122: Should modules use $langcode or $entity->language for determining language during hook_entity_view()?

DamienMcKenna’s picture

Priority: Normal » Critical

After spending over three hours on this I'm thoroughly confused, so I'm going to wait on feedback from someone who knows more than I. That said, I'm marking this as Critical as, well, it's pretty critical =)

DamienMcKenna’s picture

Status: Needs work » Needs review
FileSize
6.34 KB

Food for thought.

1mundus’s picture

My replies to #2

1. Yes
2. Beta3
3. No
4. Yes, all of them
5. Yes, all of them have the correct language (non-English) assigned EXCEPT one
6. This exception (taxonomy term) has „und“ value (more underneath)

I have edited this taxonomy term and added description different from default values. When I save the changes, the description field on term edit page reverts to default. However, custom description is displayed in HTML output! Database situation is even more strange – there are two records created for the same term ID – one with the correct language assigned and the other with „und“ in language field.

DamienMcKenna’s picture

I think we're boiling this problem down to two pieces: entities that have a 'language' value, e.g. nodes, and entities that do not have a language value, e.g. terms. The patch in #13 should work pretty well in both cases with the core Locale module, I need to test Entity Translation again.

agoradesign’s picture

Hi,
I've tried the patch from #13 and I got some errors after saving a node. It seems that it isn't guaranteed that the function call metatag_metatags_load_multiple() (used in metatag_entity_update()) returns something for the updated node. I guess that applies to all nodes without overriding default meta tags.

However, using a if condition to check if $metatags[$id] actually exists solves this problem

Stan Turyn’s picture

#13 fixed setting Taxonomy tags issue for me (using Locale)

Peacog’s picture

FileSize
6.39 KB

I had the same problem as #16 i.e. metatag_metatags_load_multiple() returns null for nodes that have default metatag values. I've modified the patch from #13 to check if anything is returned before trying to delete the old metatags.

Izvolsky’s picture

#18: metatag-n1845326-18.patch queued for re-testing.

DamienMcKenna’s picture

After doing further testing I don't see a way of changing the language for terms and user data, even though Entity Translation allows fields on those entities can be flagged as being translatable. Knowing this, the queries can be *greatly* simplified :)

Ivan_Dagreat’s picture

No sure if the information is still helpful.

1. My Locale was not activated (even after activation no meta)
2. I can recall my exact previous version but I believe it from the previous name metatags
3. No errors during update
4. Yes, I see a language field but the only value is und.
5. The only value is und
6. No

DamienMcKenna’s picture

FileSize
6.4 KB

A tiny adjustment, I don't like using the "if($metatags)" pattern for checking if a variable is not empty.

DamienMcKenna’s picture

Kristen Pol’s picture

I just applied #22 to the dev version and it doesn't fix metatags for entity-translated nodes (this issue: http://drupal.org/node/1688286#comment-6807742). Though I'm not sure if http://drupal.org/node/1844638#comment-6806626 needs to be applied first. If so, that one gave me an error: http://drupal.org/node/1844638#comment-6809838

Please advise :)

DamienMcKenna’s picture

@Kristen: correct, a working patch from #1844638 is necessary before this can be verified.

DamienMcKenna’s picture

There was an unfinished comment in #22.

DamienMcKenna’s picture

FileSize
6.42 KB

Missed the attachment.

DamienMcKenna’s picture

@Kristen: Does the patch in #27 resolve problems editing meta tags on new content (apply patch, create content, edit content) in your example site?

greggles’s picture

My symptom is that I've got a title pattern like "foo [node:title] | [site:name]" and while the site:name token is replaced, the node:title token is not replaced. I'm not sure if that description is the same as this one.

Is the Locale module enabled? - Yes.
What version did you upgrade from? A recent dev version, beta2, beta3, etc? beta2, but I also did a reinstall and that didn't help.
Did any errors show when you updated to beta4? no.
Please check the {metatag} table in your installation, is there a 'language' field? yes.
If the answer to #4 was 'yes', did any of the records have values for the 'language' field? no (I have no records there).

I also applied the patch from #27 without benefit.

greggles’s picture

update to #29 - my problem was in fact http://drupal.org/project/exclude_node_title was enabled for this node type. Filed #1864284: Incompatible with metatag module for my issue.

DamienMcKenna’s picture

FileSize
6.85 KB

I uncovered a problem with the code in metatag_entity_update in the patch from #27: if the language is modified the {metatag} record isn't updated accordingly as it uses entity_language() to obtain the language definition, which uses the last saved value, not the value from the form. So...

This patch has been tested with both the Locale and Entity Translation modules.

Dave Reid’s picture

When hook_entity_update is run, the entity *should* be saved already.

DamienMcKenna’s picture

@Dave: The problem is that metatag_metatags_save() updates an existing record based on a combination of the entity_type/entity_id/language, if the language has changed, because someone changed the language selector on the node page, then it will create a new record rather than updating the existing one.

DamienMcKenna’s picture

@Dave: btw the {metatag} record isn't saved until the end of metatag_entity_update.

DamienMcKenna’s picture

@Dave: I slightly misread what you said. I think it might be that old entity-updated-but-not-reloaded problem.

ezeedub’s picture

I came across this issue when I used views_bulk_operations to bulk save some nodes (to fire a presave hook for an unrelated reason).

Saving the node individually is fine, but through node_save_action() the content of the data column in {metatag} is wrapped in an array keyed by language, like so:

mysql> select data from metatag where entity_id = 1726;
+------------------------------------------------------------------------------+
| data                                                                         |
+------------------------------------------------------------------------------+
| a:1:{s:3:"und";a:1:{s:8:"keywords";a:1:{s:5:"value";s:13:"one,two,three";}}} |
+------------------------------------------------------------------------------+

If I load the node form in this state, metatags are reported as "using defaults". If I edit the keywords, and save the node, the array in {metatag} returns to this format:

mysql> select data from metatag where entity_id = 1726;
+--------------------------------------------------------------+
| data                                                         |
+--------------------------------------------------------------+
| a:1:{s:8:"keywords";a:1:{s:5:"value";s:13:"one,two,three";}} |
+--------------------------------------------------------------+

Not sure if that helps. I drilled into this a couple days ago, but sorry I don't have any time to do so anymore right now.

metatag schema_version is 7004 btw

DamienMcKenna’s picture

I'm seriously considering dropping the 'language' field from the primary key, I now seriously think that was a huge mistake and has made this entire thing much more complex than it needs to be. Unless someone can give me a reason why the 'language' value should be a part of the primary key, I'll be removing it in a few days so that this and #1844638: metatag_update_7004 goes into an infinite loop can be finished, allowing us to move on to releasing beta5/rc1.

I would really like to hear from Kristen Pol and Colan, both of whom have lots of experience with internationalization and who's experience I respect in this field, if they can show why the 'language' field must be treated this way then I'll accommodate it but I strongly suspect it should not be.

DamienMcKenna’s picture

Status: Needs review » Needs work

Per the latest comments in #1844638: metatag_update_7004 goes into an infinite loop, this needs to be updated.

DamienMcKenna’s picture

After further research I've realized that Entity Translation *does* use the same entity_id for different language pairs, so the language value does need to be a foreign key after all. What I was seeing in the node table is because node titles are not translatable.

DamienMcKenna’s picture

Status: Needs work » Needs review
FileSize
8.4 KB

I've tested this with plain nodes and Entity Translation-powered nodes with various scenarios. I think this is pretty close to exact to perfect.

DamienMcKenna’s picture

A related issue I identified while digging around in the data structures: #1876034: Ensure $entity->metatags can be saved programmatically from a loaded entity only

plach’s picture

Status: Needs review » Needs work

I had just a quick look to this patch so I can't tell for sure, but the changes look sensible to me.

+++ b/metatag.module
@@ -536,12 +534,30 @@ function metatag_entity_update($entity, $entity_type) {
+    if (module_exists('entity_translation') && isset($entity->translation)) {

This should be $entity->translations.

DamienMcKenna’s picture

Status: Needs work » Needs review

@plach: When saving a translation of an object, e.g. via es/node/123/edit, in hook_entity_update() there's an attribute $entity->translation that contains content like this:

translation (Array, 4 elements)
    status (Integer) 1
    retranslate (Integer) 0
    name (String, 5 characters ) admin
    created (String, 0 character)

When saving a translation it isn't possible to change the language therefore we don't want to waste time seeing if any content should be purged, i.e. this is how it should work.

DamienMcKenna’s picture

This needs some further work, there's a little confusion over how to handle users.

DamienMcKenna’s picture

Ok, with users there is no way to translate the data, even if a translatable field is added to the User entity, so Metatag needs to know to ignore the language settings for User objects. Argh.

DamienMcKenna’s picture

FileSize
9.39 KB

I really need input from some of the translation team (plach, Kristen Pol, colan) on how to handle entities other than nodes. Am I right in saying that if an entity type does not have the setting $type['translation']['locale'] set to TRUE that it should use either the system default language or maybe even LANGUAGE_NONE? That's my current guess.

This patch changes the logic so that if $type['translation']['locale'] is not TRUE it will use LANGUAGE_NONE and I need to test it again to ensure nothing that was fixed with patch #40 has been undone.

plach’s picture

Actually $type['translation']['locale'] means that core (Locale module) is handling field language for $type. You shouldn't need to inspect that property directly, there are API functions for doing that:

field_has_translation_handler()
field_is_translatable()

Entity Translation registers itself as a translation handler for each fieldable entity type, which means that it will handle field language for those, i. e. you will find the following value defined: $type['translation']['entity_translation'].

If I'm not mistaken, here you are are dealing with the entity language, correct? In this case you could check if the $type['entity keys']['language'] key is defined, but this is not reliable since this key has been introduced in Drupal 7.15. If you are trying to provide a fallback logic on drupal versions below 7.15 you could have a look to pathauto_entity_language(). Otherwise I'd need to understand what is the goal of:

This patch changes the logic so that if $type['translation']['locale'] is not TRUE it will use LANGUAGE_NONE and I need to test it again to ensure nothing that was fixed with patch #40 has been undone.

DamienMcKenna’s picture

@plach: The problem is that entity_language() returns useless information when you're dealing with entities that may not be translated, e.g. users. I added a translation-enabled field to the User entity but there was no functionality presented for translating it, unlike nodes where it all works correctly; without any way of translating user objects there's no point in bothering supporting multiple languages and it should instead use LANGUAGE_NONE.

DamienMcKenna’s picture

@plach: There are two specific cases where I need to do something abnormal involving the language - when creating a new translation it clones the 'original' object's settings (metatag.module line 1172), and when updating a translated object there's no need to purge any old data (metatag.module line 540). If you are aware of better ways of handling the necessary logic for these, please let me know.

DamienMcKenna’s picture

@plach: To further explain the problem with users, without the Locale module being enabled the {users}.language field is set to an empty string, with Locale enabled it sets the field to the site's default language instead; I'm concerned with (again) running into problems of the language value being mismatched and data disappearing.

plach’s picture

Well, users are a bit of a special case because their language property does not necessarily indicate the language of the attached content:

http://drupalcode.org/project/drupal.git/blob/refs/heads/7.x:/modules/us...

This is the reason why entity_language() returns NULL in this case. If you wish to match the Field API behavior, we should assume that the metatags "field" is always translatable and thus it needs to follow the entity language for the original values. For entities with no language support such as users or taxonomy terms, translatable fields usually get the site's default language. Hope this helps :)

DamienMcKenna’s picture

@plach: thanks for the response. My main problem with using $GLOBALS['language_content']->language is what happens if the language changes?

plach’s picture

Actually I'm suggesting to use language_default()->language when entity_language() returns NULL. This is the standard behavior of field_attach_form() for instance:

http://drupalcode.org/project/drupal.git/blob/refs/heads/7.x:/modules/fi...
http://drupalcode.org/project/drupal.git/blob/refs/heads/7.x:/modules/fi...

DamienMcKenna’s picture

So, again, what's the standard procedure for when someone changes the default language? Could that not potentially make user data, including Metatag data, inaccessible?

DamienMcKenna’s picture

@plach: Hopefully one last thing. I've noticed that the entity form is loaded twice when using Entity Translation and that on the second time through some of the arguments to hook_field_attach_form have changed. Is there any problem using this code to skip the second execution?

function metatag_field_attach_form($entity_type, $entity, &$form, &$form_state, $langcode) {
  // Entity_Translation will trigger this hook again, skip it.
  if (!empty($form_state['entity_translation']['is_translation'])) {
    return;
  }
  ...
}
DamienMcKenna’s picture

@plach: Another scenario I'm wondering about.. Say you've got a few languages enabled and the default is set to e.g. English. You create a new node, set the node's language to e.g. French and customize the meta tags. When you view the node it does not show the customized meta tags, instead it shows the default for that content type because hook_entity_view was passed a $langcode of 'en' rather than using the node's default language. So, what logic should be used to determine whether to use the $langcode argument to hook_entity_view or the results of entity_language($entity_type, $entity)?

DamienMcKenna’s picture

Status: Needs work » Needs review

A list of the outstanding questions:

  1. Is there any reason to not use LANGUAGE_NONE for occasions when entity_language() returns NULL?
  2. If language_default('language') is supposed to be used instead of LANGUAGE_NONE for entities that are not translatable (users, terms), what about scenarios when the system language is changed, would that not lead to data becoming detached from the entity due to the language change, or are there API calls / functions that can help with this situation?
  3. Is there any problem skipping the second execution of hook_field_attach_form when using Entity Translation?
  4. What logic should be used to determine whether to use the $langcode argument to hook_entity_view or the results of entity_language($entity_type, $entity), e.g. when viewing an untranslated node with its language changed to 'fr' while the system default language is 'en'?

Also, a list of known problems that need to be resolved:

  1. After enabling Locale & Entity Translation and adding some additional languages, when a node is edited that previously had the language unset ('und') and the node's language is changed the old record for meta tags is not purged (needs additional logic for metatag_entity_update).
  2. Viewing a node which has a different language value (via entity_language()) to the current page's language (via hook_entity_view()) will result in any customized meta tags not being displayed (see item 4 above).

Two other notes:

  • In order to simplify the logic I'm going to make the module require core 7.15 or newer.
  • Once the logic is finally sorted I'm going to write a bunch of tests for it, but maybe not 'til the next beta release.
DamienMcKenna’s picture

Status: Needs review » Needs work

(the correct status)

DamienMcKenna’s picture

FileSize
12.88 KB

This new patch has the following changes from the previous one:

  • Metatag now requires Core 7.15 or newer.
  • When loading an entity for a requested language that is different from the language turned by entity_translation(), if there is no data for the requested language then the data for the entity's default language will be used instead.
  • The problem in metatag_entity_update() of what to do when changing an entity's language vs saving a new translation has been solved, it appears to work correctly now.

I'm going to run through some test scenarios a little more, but I think this is pretty good now. Some additional testing would be appreciated.

DamienMcKenna’s picture

Status: Needs review » Needs work

I've identified an odd scenario with the patch in #59 that results in data being deleted:

  • Locale is installed along with multiple languages, Entity Translation is not installed.
  • The Article content type is configured to allow translations.
  • Create a node, assign it a specific language, customize the meta tags (node/123).
  • Edit the node (node/123/edit), change the language, do whatever tasks you want, everything behaves correctly.
  • Enable Entity Translation, change the article content type to allow content to be translated.
  • Go to the translation page for the node created above (node/123/translate). The meta tags created above will be immediately deleted.

Argh.

DamienMcKenna’s picture

FYI the scenario detailed in #60 does not happen the first time you go to the node/123/translate page if the node is created when Entity Translation is already installed, it only happens the first time you load that page for nodes created before Entity Translation is enabled. Like I said, argh.

DamienMcKenna’s picture

The problem stems from hook_entity_translation_insert() being triggered in entity_translation_overview() as it initializes the translations for an existing, untranslated entity. This is a problem because it no meta tag data is passed in via hook_entity_translation_insert(), thus when it gets to metatag_metatags_save() it has an empty $metatags array, thus the existing record for that language is purged. So one way of resolving this would be to skip the code in metatag_entity_translation_insert() when the submitted $values array is empty.

A bit of further testing..

When a node is first being created, hook_entity_translation_insert() is triggered and again the $values array is empty. When this happens the main metatag_entity_insert() code will save the data, there's no need for any additional processing.

When a node is being translated into a new language hook_entity_translation_insert() is triggered but once again the $values array is empty, and once again the core hooks are able to handle everything.

I'm starting to wonder if the hook implementations in metatag.entity_translation.inc are actually needed?

DamienMcKenna’s picture

Ok, in my testing it seems that metatag_entity_translation_delete() is still needed, but I need to further test metatag_entity_translation_update().

DamienMcKenna’s picture

After another bit of testing I'm unable to see why metatag_entity_translation_update() is needed at all - updating the translated version of a node still triggers hook_entity_update(), the meta tag record is still updated even if metatag_entity_translation_update() is commented out.

DamienMcKenna’s picture

Status: Needs work » Needs review
FileSize
15.47 KB

This patch is the same as #59 but I've moved metatag_entity_translation_delete() into the main module and removed the rest of metatag.entity_translation.inc.

hugov’s picture

subscribe

DamienMcKenna’s picture

agoradesign’s picture

Hi,
I've just upgraded Meta tags module on a site with Entity Translation enabled from beta2 to beta4 with the patch from #65. These are the changes I could observe:

Before in beta2, changing the description in one language naturally also affected the other language(s). With patched beta4 this seems to work as expected :)

Before in beta2, you could have different meta descriptions, if you use the default [node:summary] token and make the body field translatable. But unfortunately this seems to work no longer now. When I now create a node in german as source language, fill in the summary text of the body field and use the [node:summary] token as meta description, and afterwards translate the node to english setting a different summary text, always the german text is shown in the meta description when viewing the page.

fago’s picture

Status: Needs review » Needs work

I tested #65, however it did not work for me in a multi-lingual environment with entity-translation either. I debugged it a bit and figured out it almost works - the right metatags are figured out on display and get written to $entity->content + output. But they are also written to the static cache via metatag_page_set_metatags() such that they are written again during hook_page_build() - but this time the language context seems to broken.

I don't see why the module does write the metatags twice though? If I remove the call to metatag_page_set_metatags() in metatag_entity_view() it seems to work fine.

DamienMcKenna’s picture

Status: Needs work » Needs review

@fago: The only time metatag_page_set_metatags() is called is during metatag_entity_view(), I don't see where it's called a second time?

For problems with [node:summary], was something manually inserted into the summary field in order to avoid #1300920: The [node:summary] token does not output anything for body fields without a manual summary?

DamienMcKenna’s picture

Version: 7.x-1.0-beta4 » 7.x-1.x-dev

I'm unable to reproduce the problems mentioned in #68 and #69 regarding entity_translation. Here's my scenario:

  • Multiple languages enabled - English, French, Spanish. Default language: English. URL language detection enabled.
  • Create a node, set it to French, add a node summary and add some custom meta tags.
  • Display the node (/node/123), everything's fine.
  • Translate the node into English, customize the body, summary and meta tags.
  • Display the node with the language default (node/123), everything's showing in English, as it should be.
  • Click over to the French version of the node, everything displays in French.

If you're seeing something different could you please provide further details?

BTW please use the patch with the latest -dev release rather than beta4.

fago’s picture

@fago: The only time metatag_page_set_metatags() is called is during metatag_entity_view(), I don't see where it's called a second time?

It's not called twice, it are the metatags that get added twice: Once via that function and once via entity rendering.

>BTW please use the patch with the latest -dev release rather than beta4.
I'll give that a try.

agoradesign’s picture

For problems with [node:summary], was something manually inserted into the summary field in order to avoid #1300920: The [node:summary] token does not output anything for body fields without a manual summary?

Yes, I'm aware of that problem, so I did manually insert a text in the summary.

BTW please use the patch with the latest -dev release rather than beta4.

I will try this now on a different site with a similar setup and report back

agoradesign’s picture

I'm back with some bad news... Here's my scenario:

  • Drupal 7.19
  • latest dev of Entity Translation (7.x-1.0-beta2+12-dev)
  • latest dev of Metatags (7.x-1.0-beta4+37-dev) with the patch from #65 and the Views patch from http://drupal.org/node/1804356#comment-6972054
  • English and German enabled - German as default language
  • create a German node, add a node summary
  • translate into English, set a different node summary
  • view the page in German, everything's ok
  • view the page in English -> BUG: showing the German text in meta description
  • create another node and choose English as source language for this node and again add a node summary
  • translate into German, set a different node summary text
  • view the page in English, everything's ok
  • view the page in German -> BUG: showing the english meta description here

So it seems, that always the node's source language is chosen, if I use the [node:summary] token for the meta description.

At first glance, the only difference I can see between our desriptons of our setups, is, that you still have English as your system default language and I'm using German. While this of course should not make any difference, it might be worth trying out in a test environment... Both sites I mentioned are in active development for customers, so I don't want to play around with changing the site's default language. I could setup a test environment on friday or saturday and play around with different settings and debug...

But first, I'll try fago's solution at the current site

DamienMcKenna’s picture

@fago: Ok, I see what you mean; I'm not sure why the code is doing that, but it still works if the output isn't added to $entity->content['metatags'].

DamienMcKenna’s picture

Corrected the interdiff.

DamienMcKenna’s picture

@agoradesign: I updated to entity_translation 1.0-beta2 (it was beta1) and it's still working correctly. I also then changed the default language to Spanish and I now get:

  • node/123 - pulls the node's default language, which is French.
  • fr/node/123 - pulls the node's French content.
  • en/node/123 - pulls the node's English content.

Again, this is working correctly in my tests. :-\

Out of interest, what language detection mechanisms have you enabled? As mentioned earlier, on my test install I'm using the URL to change, but I'll test the others too.

DamienMcKenna’s picture

A weird one. Without any language detection methods enabled it loads the meta tags for the entity's default language if there are no meta tags defined for that language, however Drupal core + entity_translation just picks the first language found (presumably using array_unshift); in my test case the result is that the node body is in English while the meta tags are French. So, is the logic on 618 of metatag_entity_view() correct or should it just pull the first item?

plach’s picture

Sorry for being vacant, but I'm really busy with core stuff and my day job.

So, is the logic on 618 of metatag_entity_view() correct or should it just pull the first item?

ET uses the core language fallback logic provided by language_fallback_get_candidates().

agoradesign’s picture

I also have URL detection. To be more precise, I'm using URL as main method + others. And I was calling the nodes via URL prefix. But I turned off the others for testing purposes and nothing changed :(

I also tried your patch from #75 and it didn't help me either. By the way, I've found a bug in your patch: while the interdiff patch is correct, you have commented out line 265 of the patch file by accident:

// metatag_page_set_metatags($instance, $output);

I hope, I'll find some time this evening to debug through the code and find the root of the problem..

agoradesign’s picture

I took the time to debug the code a little bit. I may quote Dr. King Schultz now: "Tata!"

The difference between our scenarios was, that you've obviously set some other meta tags besides meta description, whereas I was leaving everything at default state. Therefore my node entity doesn't have anything set in its metatags property, while your's wasn't empty.

When we look into metatag_entity_view() method at line 617, we see the problem: the $entity_language is different from the $lang_code parameter and in my case $entity->metatags is completely empty, as my node hasn't overwritten any defaults. So we enter the if condition, and the $lang_code parameter, e.g. originally set to 'de', gets overwritten with the $entity_language = the source language of the node, e.g. 'en'

That means, that this fallback needs to be reconsidered. If a fallback is provided, it must respect the default settings of the given entity / bundle type. But I'm basically not sure, if we really need a fallback to the entity's source language anyway? Because if we don't have values for a specific language set, the default metatag configuration would be applied later on in metatag_metatags_view() method. And that would be exactly the behaviour I'd expect. Maybe there are use cases, where a fallback to the source language is needed. But then we need to load the default metatag config first, and then merge in the specific ones from the source language. But even this feels wrong in my eyes

PS: just tested: removing the above mentioned line 618 solved my problem as expected

DamienMcKenna’s picture

FileSize
16.35 KB

@agoradesign: Gah, was hurrying myself too much on the patch, missed the commented line. This one is fixed.

DamienMcKenna’s picture

FileSize
16.02 KB

This patch has removed the failover to load the $langcode from the entity, something for consideration and I'm going to test it.

DamienMcKenna’s picture

Status: Needs review » Needs work

So now there's the problem that if a content type isn't set to be translated then it gets a different $langcode in metatag_entity_view() (NULL) vs metatag_field_attach_form() (the default language).

DamienMcKenna’s picture

Oh.. the problem I was experiencing in #84 was because Panels was displaying the page. Which evidently is another problem.

DamienMcKenna’s picture

@agoradesign: The problem is the following: when the node is saved with a different language that the site default and there is no translation available the entity's language is used to save the meta tag data, but when you *view* the node's primary page (node/123) hook_entity_view is passed the site's default language and not the entity's language. If you look in field_data_body for such a node it has the same $language value as the {metatag} record, so core is using different logic to identify the appropriate language.

DamienMcKenna’s picture

Poking around I see that core uses field_language() to identify the language to use to display each field, via field_attach_view(), and in my scenario in #86 it does indeed return 'en' as the language for the body field in my test node. Would it be safe to use field_language() to identify the entity's display language? Of course then there's the problem that just running field_language($entity_type, $entity) returns an array of *all* attached fields and their associated language, so then which value gets picked? Like I said, argh.

agoradesign’s picture

@agoradesign: The problem is the following: when the node is saved with a different language that the site default and there is no translation available the entity's language is used to save the meta tag data, but when you *view* the node's primary page (node/123) hook_entity_view is passed the site's default language and not the entity's language. If you look in field_data_body for such a node it has the same $language value as the {metatag} record, so core is using different logic to identify the appropriate language.

Just to be sure that I understand your scenario. You do something like this:

  1. let's assume you have english as default language and also german enabled
  2. you create a node in german only - no english translation - and you also set custom meta tags
  3. than you view this german node in default (=english) language
  4. as there's no english translation available, Drupal intelligently falls back to show the german content. But the custom meta tags don't show up, after you removed your fallback in the last patch

Is this correct?

I think, in order to provide a fallback for this case, without causing field-based translated entities that do not override default meta tags to show the wrong tags, instead of checking for $entity_language != $langcode && !isset($entity->metatags[$langcode]), we should check, if there are field translations available for the $langcode. If we have field translations but no custom meta tags for this language, we can assume, that we should use the default configuration and don't switch $lang_code to the $entity_language. If there's no entity translation at all for that language, then you may switch to the entity's language.

DamienMcKenna’s picture

@agoradesign: That scenario is correct. I'll try adding field_language() to the mix, see how it goes.

willvincent’s picture

I found what prevented metatags from properly loading for me..

Line 357 of metatag.module:

$metatags[$record->entity_id][$record->language] = unserialize($record->data);

However, the serialized data already has language set, so in my case this was generating an array like this:

  $metatags
     [node_id]
        ['und']
          ['und']
            ['page_title'] = ...
            ['description'] = ...

So, I'm not entirely sure whether language code is supposed to be included in the serialized data, but I would assume it's not since the schema for that table also has a field for language code...

At any rate, if it's there, then line 357 of metatag.module (part of metatag_metatags_load_multiple()) should read:

$metatags[$record->entity_id] = unserialize($record->data);

So, something is causing this to get included in the serialized data on save, and then after cache clears and it tries to reload the metatags it gets an improperly formed array so ends up reverting to defaults.

I would suggest that whatever solution ultimately comes of this issue, that an update hook be included that will load, unserialize, sanitize, and resave each row of the metatag table so that data isn't lost when updating. Having to re-enter metadata for every entity that has it would royally suck.

VladB1989’s picture

Hi,

The fix that worked for me and solved my problem on a basic Drupal 7 setup is this one:

http://drupal.org/node/1845326#comment-6752736

DamienMcKenna’s picture

@willvincent: There was a bug that caused the data to be serialized twice (#1871020: Conflict with Workbench Moderation: tags revert to defaults after moderation change), I've just added another issue to fix existing data: #1919070: Fix any meta tags that may have been serialized twice

willvincent’s picture

I've reverted back to beta2 for the time being until there's a stable 1.0 release I'll probably stick with beta2. Everything was working fine for me there, and this issue was preventing completion of a client site.

DamienMcKenna’s picture

Status: Needs work » Needs review
FileSize
16.73 KB

This patch uses field_language() to help identify the language to use when the requested language for the page is different to the entity's language; it also adjusts the code to hopefully improve the performance on pages when the above scenario occurs.

Status: Needs review » Needs work

The last submitted patch, metatag-1845326-93.patch, failed testing.

sylus’s picture

Status: Needs work » Needs review

#94: metatag-1845326-93.patch queued for re-testing.

axe312’s picture

I applied #94 to newest dev and it didnt work.

I jumped into the code and figured out, that the hook_entity_view() is the problem. After adding following code to the beginning of the hook, it worked for me!

if (!isset($langcode)) {
  $langcode = $GLOBALS['language_content']->language;
}

(Stolen from http://api.drupal.org/api/drupal/modules%21node%21node.module/function/n...)

My $langcode parameter was NULL and the documentation of the hook says: "$langcode: (optional) A language code to use for rendering. Defaults to the global content language of the current request."

So I've added the code above to function metatag_entity_view($entity, $entity_type, $view_mode, $langcode) and now the metatags are loaded in the correct language!

Maybe its also important that our website is a panelized one!

axe312’s picture

Status: Needs review » Needs work
DamienMcKenna’s picture

@axe312: Please read above, using $GLOBALS['language_content']->language is not going to work.

I'll test out Panels integration again..

DamienMcKenna’s picture

For the record, the patch in #94 works for me on a site that doesn't have Locale or EntityTranslation enabled.

DamienMcKenna’s picture

Ok. So, I tested Panels out a bit. I've realized that the change made in #94 to double-guess the language assignment is causing the problem with Panels and needs a little work, though it'll be a performance hit.

DamienMcKenna’s picture

Status: Needs work » Needs review
FileSize
16.8 KB

Ok, this patch moves the language identification code in metatag_entity_view to before the cache $cid is defined; this fixes the problem with Panels but may make the page loading a little slower.

Could everyone please help test this? I'd like to commit this ASAP so we can move on with beta5.

DamienMcKenna’s picture

Poieo’s picture

I can confirm the latest dev with this patch has resolved the issues discussed here #1900382: descripton on taxonomy not saved.

Summit’s picture

So can this be set to RTBC?
Greetings, Martijn

DamienMcKenna’s picture

@Poeio: thanks for the review.

DamienMcKenna’s picture

Status: Needs review » Reviewed & tested by the community

This needed to be put through its paces. Again.

Site setup:

  • Drupal core 7.21-dev, Entity Translation v7.x-1.0-beta2.
  • Entity type: node
  • Customize the default Article content type to enable field translations, i.e. EntityTranslation.
  • Enable English, French and Spanish.
  • Set English as the default language.

Language set to LANGUAGE_NONE:

  • Create a new Article node, set the language to "none", customize the meta tags, save the node.
  • View the node, the meta tags appear correctly.
  • Edit the node, change the meta tags and save.
  • The meta tags updated and display correctly.
  • Examined the {metatag} table, the language value was saved correctly as "und".

Language set to LANGUAGE_NONE, change language:

  • Create a new Article node, set the language to "none", customize the meta tags, save the node.
  • View the node, the meta tags appear correctly.
  • Examined the {metatag} table, the language value was saved correctly as "und".
  • Edit the node, change the language to "French" change the meta tags and save.
  • The meta tags updated and display correctly.
  • Examined the {metatag} table, the language value was saved correctly as "fr".

Language set to the same as the site default:

  • Create a new Article node, set the language to "English" (the default), customize the meta tags, save the node.
  • View the node, the meta tags appear correctly.
  • Examined the {metatag} table, the language value was saved correctly as "en".
  • Edit the node, change the meta tags and save.
  • The meta tags updated and display correctly.
  • Examined the {metatag} table, the language value was saved again as "en".

Language set to the same as the site default, change language:

  • Create a new Article node, set the language to "English" (the default), customize the meta tags, save the node.
  • View the node, the meta tags appear correctly.
  • Edit the node, change the language to "French", change the meta tags and save.
  • The meta tags updated and display correctly.
  • Examined the {metatag} table, the language value was saved correctly as "fr".

Language set to the same as the site default, translate to another language:

  • Create a new Article node, set the language to "English" (the default), customize the meta tags, save the node.
  • View the node, the meta tags appear correctly.
  • Examined the {metatag} table, the language value was saved correctly as "en".
  • Go to the Translate tab, add a translation for French.
  • Customize the meta tags and save.
  • The meta tags display correctly.
  • Switch back to the English node, the meta tags still display correctly.
  • Examined the {metatag} table, there are now two records, one with the language set to "en", one with "fr".

Language set to a non-default:

  • Create a new Article node, set the language to "French", customize the meta tags, save the node.
  • View the node, the meta tags appear correctly.
  • Examined the {metatag} table, the language value was saved correctly as "fr".
  • Edit the node, change the meta tags, save.
  • The meta tags updated and display correctly.
  • Examined the {metatag} table, the language value is still "fr".

Language set to a non-default, change language:

  • Create a new Article node, set the language to "French", customize the meta tags, save the node.
  • View the node, the meta tags appear correctly.
  • Examined the {metatag} table, the language value was saved correctly as "fr".
  • Edit the node, change the language to "Spanish", change the meta tags, save.
  • The meta tags updated and display correctly.
  • Examined the {metatag} table, the language value was updated correctly as "es".

Language set to a non-default, translate to another language:

  • Create a new Article node, set the language to "French", customize the meta tags, save the node.
  • View the node, the meta tags appear correctly.
  • Examined the {metatag} table, the language value was saved correctly as "fr".
  • Go to the Translate tab, add a translation for Spanish.
  • Customize the meta tags and save.
  • The meta tags display correctly.
  • Switch back to the French node, the meta tags still display correctly.
  • Loading /node/123 shows the French meta tags and content - correct.
  • Loading /fr/node/123 shows the French meta tags and content - correct.
  • Loading /es/node/123 shows the Spanish meta tags and content - correct.
  • Examined the {metatag} table, there are now two records, one with the language set to "es", one with "fr".

Language set to a non-default, translate to the two other languages:

  • Create a new Article node, set the language to "French", customize the meta tags, save the node.
  • View the node, the meta tags appear correctly.
  • Examined the {metatag} table, the language value was saved correctly as "fr".
  • Go to the Translate tab, add a translation for Spanish.
  • Customize the meta tags and save.
  • The meta tags display correctly.
  • Switch back to the French node, the meta tags still display correctly.
  • Loading /node/123 shows the English meta tags and content - correct.
  • Loading /fr/node/123 shows the French meta tags and content - correct.
  • Loading /es/node/123 shows the Spanish meta tags and content - correct.
  • Examined the {metatag} table, there are now three records, one with the language set to "es", one with "fr" and one with "en".

Just to see, I also tested editing a user:

  • Edited my user account, which had the language set to English.
  • Customize the meta tags, save the user.
  • View the user, the meta tags appear correctly.
  • Examined the {metatag} table, the language value was saved correctly as "und".
  • Edit the account, changed the meta tags and save.
  • The meta tags updated and display correctly.
  • Examined the {metatag} table, the language value was still saved as "und".

All of the use cases worked correctly. We also have confirmation from Poieo that it resolved a problem editing term pages. I've also tested it on a few sites, all worked correctly.

I think this is good. Finally.

DamienMcKenna’s picture

Status: Reviewed & tested by the community » Fixed

"Now lets [commit] this thing and go home!"

Summit’s picture

@Damien, thanks for your thorough and great job! fingers crossed :)
Greetings, Martijn

Sleavely’s picture

This sounds like superbly great news! Do you have an ETA on beta5?

fago’s picture

Thanks, that works in my case also.

DamienMcKenna’s picture

Status: Fixed » Closed (fixed)

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