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.
#282191: TF #1: Allow different interface language for the same path includes changes to the global language variables to accompany a removal of content language negotiation UI from core.
To double check that nothing was broken, it's necessary to review all places using the global $language variable to ensure they have the correct language type and file patches for any issues found.
Comment | File | Size | Author |
---|---|---|---|
#14 | language-684982-14.patch | 5.53 KB | plach |
#13 | language-684982-13.patch | 5.1 KB | plach |
#12 | language-684982-12.patch | 5.01 KB | plach |
Comments
Comment #1
plachThis is needed because currently content language inherits the value of interface language, so in core using the former or the latter makes no difference. But contrib modules can alter this, so we have to ensure all places use the right language type.
Comment #2
plach$options['language']
is empty the value of$language
will be used, butdrupal_lookup_path()
uses$language_content
.$language_content
available in D6.Comment #3
plachComments are welcome.
Comment #4
plachUpgrading to critical as having parts of the drupal core using the wrong language type is a serious issue. OTOH this should not be a hard one to fix as it only involves switching some global variables around, but IMHO it's a must-fix.
Comment #5
Jolidog CreditAttribution: Jolidog commentedI don't think this is critical, but it shouldn't be forgotten.
Perhaps adding the priority major tag is a good compromise?
Comment #6
plachI'd like to have a committer feedback about this.
Comment #7
catchThe thing which makes this more critical than normal is the comment language - since that could lead to inconsistent data, but I think it's going to be sufficiently rare that 'major' would be fine if we had that priority, but I imagine this will get patched and fixed before that priority is on Drupal.org anyway.
Looking at the table those changes all look fine, but there is no patch here, so back to active.
Comment #8
plachA couple of tricky ones are:
drupal_render_cid_parts()
, I'd like the hear moshe about itComment #9
catchFor drupal_render_cid_parts() I would use content language - if you need to do something tricky with user language you could specify that yourself via 'cid' instead of using the helper iirc.
language direction, no idea :(
Comment #10
moshe weitzman CreditAttribution: moshe weitzman commenteddrupal_render_cid_parts basically tries to recreate http://api.drupal.org/api/function/_block_get_cache_id/6. so, i would do whatever you would do for that code. hope that helps.
Comment #11
haaiee CreditAttribution: haaiee commentedRemoved my own comment as it was not relevant.
Comment #12
plachThe patch performs the corrections suggested in #2.
The
drupal_render_cid_parts()
issue has been addressed by adding as cid parts all the configurable language types: this means that in core the behavior is exactly the same as before, but if a contrib module enables additional configurable language types (I'm thinking mainly about content language) they will be used as cid parts too. This choice should let us handle pages displaying content available in multiple languages which could have different cache entries for every combination of UI/content languages.I left unchanged the language direction issue because I think UI language is the correct choice in the scopes it is used (i.e. page level). In #780508: Set language by content language for LTR/RTL display we can try to introduce a content-level scope to achieve separate language directions for page-level and content-level scopes (i.e. what's sketched in http://drupal.org/files/issues/rtl.png).
Comment #13
plachI forgot a comment.
Comment #14
plachAgain wrong patch, sorry :(
Comment #15
sunAgreed on deferring language direction to #780508: Set language by content language for LTR/RTL display. The page/document must use UI language, inner elements may define language direction based on content language; specifying language itself additionally helps crawling machines/spiders.
Very nice - dynamically using zero to both is exactly what I wanted to suggest. This should support any, from simple to advanced, language environments.
If I did not overlook something, then this patch contains all required changes already? Or are we still missing something? If not, RTBC?
44 critical left. Go review some!
Comment #16
plach@sun:
Thanks for reviewing! The patch in #14 should address all the pending issues except language direction.
Feel free to RTBC if you don't have any remark.
Comment #17
sunComment #18
plachRelated issue #855380: $language_url should be used to lookup the current path alias.
Comment #19
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.
Comment #20
MustangGB CreditAttribution: MustangGB commentedComment #21
MustangGB CreditAttribution: MustangGB commentedTag update