Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.To reproduce:
- Enable locale module and Content translation module
- Configure the site with more that one language
- Configure Language negotiation at admin/settings/language/configure
- Add a new content type with machine-readable name negotiation (or default) and configure it's Multilingual support
The language negotiation setting (language_negotiation variable) or the language default setting (language_default variable) is overwritten by the content type setting.
The patches rename the affected configuration variables in locale.module and translation.module
The variable settings for content types should have a prefix to protect the namespace, such like this:
language_content_type_*
Note: This problem may occurs in other modules where a similar approach is used to store content type settings and site-wide settings may be overwritten.
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | typo-206021-14.patch | 1.84 KB | dropcube |
| #13 | 206021_typo.patch | 1.69 KB | keith.smith |
| #12 | locale.patch | 6.92 KB | Gábor Hojtsy |
| #11 | locale-translation-variable_rename-206021-11.patch | 6.77 KB | dropcube |
| #7 | locale-translation-variable_rename-206021-7.patch | 6.32 KB | dropcube |
Comments
Comment #1
Gábor Hojtsy1. Please merge the patches.
2. We need an update function to alter existing settings from Drupal 6 RC1 sites.
This is hardly critical. Critical bugs make your site go broken, this one only happens with a specific node type, with a specific config.
Comment #2
dropcube CreditAttribution: dropcube commented1. Patches merged.
2. Update function under development.
Comment #3
dropcube CreditAttribution: dropcube commentedThe patch provides the required modifications and an update function to alter existing settings from currently installed 6 RC1 websites.
Comment #4
Gábor Hojtsy- You use tabs in the update function, while you should use two spaces.
- There is a locale() function hunk which is already in Drupal 6.
Comment #5
dropcube CreditAttribution: dropcube commented- Fixed the above.
- Added notification messages to the update function to alert the users about the overwritten variables.
Comment #6
Gábor HojtsyGetting better! I have there comments on the patch:
- we should not use t() in the update process, we use English text, so we do not rely on the localization system being in place
- if a content type name is 'default', you set an object to the variable where an integer should be possibly... you should check for the type of language_default and only set it for the content type if it is an integer, not an object (and keep the language_default setting if it is an object) IMHO
- a same issue evolved around language_negotiation, but that is an integer as well, so it makes it harder to detect... in this case, I would reset the content type setting instead, which makes no harm, not the language_negotiation setting... although as said, it is hard to tell what's in the variable at this stage if there is a 'negotiation' content type
Comment #7
dropcube CreditAttribution: dropcube commentedYes, you are right.
In the update function:
- If a content type name is default and the variable language_default is still and object, then it has not been overwritten by the content type setting and it's not required to update the variable, just leaves it as it is. If the variable language_default is not an object, then it is reset to is default value and the user is alerted about that.
- If a content type name is negotiation, it is not possible to detect if it has been overwritten (as you said, the variable language_negotiation is an integer in any case), so the variable language_negotiation is not reset and the variable language_content_type_negotiation is created with the current value of language_negotiation. The user is alerted about this situation.
- I have removed t(), but I am confused beacuse I saw them in system.install and thought that it could be used here.
Comment #8
dropcube CreditAttribution: dropcube commentedComment #9
Gábor HojtsyThis looks good code-wise (with some formatting exceptions which I can do on commit), so we just need some more testing as far as I see.
Comment #10
meba CreditAttribution: meba commentedWorks for me and no errors occurred.
Comment #11
dropcube CreditAttribution: dropcube commentedOh, I forgot to rename the variable name in the language selector field for nodes at
locale_form_alter. Please, test this new patch. Thanks.Comment #12
Gábor HojtsyOK, I sat down to clean this up. Did a few things:
- merged an if() { if() {}} to if(..&&..) {}
- used NULL as default value, so we can skip missing settings
- used is_numeric(), so we can test for what we need, instead of what we don't
- used string concatenation instead of strtr() and l() with HTML formatting
Tested this with a Drupal 6 dev to Drupal 6 dev upgrade. A 'default' and a 'negotiation' content type produces lots of notices, when E_ALL is enabled, but obviously not visible on live sites without E_ALL. So this solves a namespace clashing issue well.
Committed the attached patch.
Comment #13
keith.smith CreditAttribution: keith.smith commentedTwo very small typos in code comments, patch attached.
Note also that the sentence Either it is the negotiation setting or the content type setting, it is an integer. is missing a word or something.
Comment #14
dropcube CreditAttribution: dropcube commentedOk, take a look now, English is not my native language.
Comment #15
dropcube CreditAttribution: dropcube commentedComment #16
Gábor HojtsyGood, committed, thanks.
Comment #17
Anonymous (not verified) CreditAttribution: Anonymous commentedAutomatically closed -- issue fixed for two weeks with no activity.