We noticed that the language selection labels changed in one of our projects to the default language, instead of the current interface language.
A lot of debugging later, we tracked it down to #3080314: Load metatag defaults based on entity language. In our case, the entity has a fixed default language (german), so when adding content in english, that function set the config override language to german and any config that is loaded afterwards then displays in german.
Restore the current config language at the end of the function
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | metatag-default-config-3108052-12.patch | 3.89 KB | berdir |
| #12 | metatag-default-config-3108052-12-test-only.patch | 2.75 KB | berdir |
| #6 | interdiff-6.txt | 350 bytes | spadxiii |
| #6 | metatag-default-config-3108052-6.patch | 1.14 KB | spadxiii |
| #3 | metatag-default-config-3108052-3-interdiff.txt.patch | 730 bytes | berdir |
Comments
Comment #2
berdirComment #3
berdirOk, that second check doesn't work because entity has been loaded from the route at that point. This should be better.
Comment #4
damienmckennaDid you check to see if #3077442: Metatags' current language through the language manager doesn't work (Decoupled Drupal) resolves the problem?
Comment #5
berdirI didn't test it, but I'm pretty certain that's a very different issue. That is about the language used in the token replace call, this is about the config override language.
Comment #6
spadxiii commentedRan into this issue myself as well and after a bit of debugging and searching, found this issue. The patch seems to fix the issue for me, and I've restored the config override language on the other return in the function.
Comment #7
berdirOh, nice catch on no defaults. Wondering if we could just do it directly after the getGlobalMetatags() call then we only need to do it once?
Comment #8
spadxiii commentedIt might seem that it could be reset after grabbing the global meta tags. I cannot oversee the things that are done in the rest of the method and whether or not they would need to have the correct language set as well.
So, I'm for doing it only once, but cannot tell for sure if that would work properly :)
(And not exactly sure how to actually verify that it works)
Comment #9
damienmckennaSo for test coverage, loading an entity form in a non-default language should trigger the problem, right?
Comment #10
berdirTo reproduce, you need o have a site with at least 2 languages and have translations for configuration on that page, e.g. the language labels.
Then the entity bundle needs to be configured to use the default language for new entities, then you need to go to node/add/page on a not-the-default-language, and as a result, you should see that first, the page should use the not-default-language translations (e.g. the node type name), and then after calling the metatag widget, it switches (languages can be used as an example for this because they are rendered in a pre_render callback later on).
Comment #11
damienmckennaThis needs test coverage before we commit it, just to be sure.
Comment #12
berdirFair, but that's quite a bit of work because I don't think getting config translations like that is tested at all, the existing test coverage seems to be testing just the basic UI operations of config translations, which means the only thing all of that is testing is really just the correct config schema definitions.
Anyway, I decided to implement something by using the API directly instead of going through the form, which would require a lot more setup including several config translations of elements before and after the metatags in the form. To make that work, I also had to change the language-creation to use the API otherwise it doesn't exist in the test container. But IMHO that's common practice, there's no benefit in doing it through the UI and this is a lot faster as the UI is doing quite a few extra things ( a full localize.drupal.org import is luckily disabled by default in tests now).
As a bonus to make up for that, I also added explicit test coverage of that early return. My patch in #3 fails with this test.
Comment #15
damienmckennaThank you both for your work on this. Committed.