The idea is to conditionally load translated values when loading values.
Menu links, variables, and any other arbitrary module data - as long as the values are stored in own columns.
This actually works. Trust me. :)
function ot_query_menu_links_alter(QueryAlterableInterface $query) {
$tables = &$query->getTables();
$fields = &$query->getFields();
$expressions = &$query->getExpressions();
// Replace ml.* with actual columns, excluding 'link_title'.
$columns = db_query("SHOW COLUMNS FROM {menu_links} WHERE Field <> :exclude", array(':exclude' => 'link_title'))->fetchCol();
unset($tables['ml']['all_fields']);
$query->fields('ml', $columns);
// Join on menu link translation table.
$query->leftJoin('ot_menu_links', 'tml', 'tml.mlid = ml.mlid AND tml.language = :language', array(':language' => $GLOBALS['language']->language));
// Conditionally use translation or source value.
$query->addExpression('CASE WHEN tml.link_title IS NOT NULL THEN tml.link_title ELSE ml.link_title END', 'link_title');
}
The patch just adds a query tag "menu_links" to all menu link queries and adds the current language code to menu tree/links caches.
Comment | File | Size | Author |
---|---|---|---|
#44 | drupal.translatable.44.patch | 22.45 KB | sun |
#43 | 593746_block.patch | 1.25 KB | andypost |
#40 | drupal.translatable.40.patch | 20.37 KB | sun |
#39 | drupal.translatable.39.patch | 21.77 KB | sun |
#37 | drupal.translatable.37.patch | 21.73 KB | sun |
Comments
Comment #2
catchsubscribing.
Comment #3
mattyoung CreditAttribution: mattyoung commented.
Comment #4
Crell CreditAttribution: Crell commentedAmazingly I think it would actually work. :-) The CASE statement in particular worries me for performance, though. I'd want to see core benchmarks on this to see what if any impact there is when this hook runs.
Comment #5
plachsubscribe
Comment #6
sunFurther discussion with Crell in IRC:
What we will do as preparation for this in D7 core though is:
Note that the original patch already used an optimized approach: The tag "menu_links" was added to three different queries in menu.inc, which all belong to different functionality, but all retrieve menu links.
For example: Filter formats have a user-customizable title, so when formats are queried, db_select() should be used.
If we can agree on that, I'd roll a patch.
Thoughts?
Comment #7
sunoh, and we may need a schema change in {menu_links}, because menu links not only have titles, but also descriptions, but the description is serialized into the 'options' column. We don't want to translate the options, just the title + description.
Comment #8
sunnedjo raised the question whether we need to add a condition for queries that (also) run on editing pages. A very good point. As of now, I assume that we have sufficiently separated queries running for different cases; for example, menu.inc's menu_tree* functions only run when entire menu trees are retrieved, and (some of) menu.module's queries only run when you intend to edit a menu link. The latter we don't want to alter, because it's the job the contrib module to load translated values and attach and provide those (somehow) in the UI.
Comment #9
Damien Tournoud CreditAttribution: Damien Tournoud commented* you want COALESCE, not CASE
* I believe it would make a lot of sense to add additional columns instead of JOINing to a different table (especially because it allows you to properly support conditions on translated columns...)
* Note that there are technical limits to the size of a row in MySQL that might make the previous idea impossible
Comment #10
nedjoThis is a great idea.
Some questions I discussed with sun (no solid answers yet):
1. Language type. If/when #282191: TF #1: Allow different interface language for the same path is applied, we'll need to distinguish between which language type we're wanting to translate to. E.g. if we're using this approach for field titles, we'll want the UI language when editing but the content language when outputting a field label for node view.
2. Serialization. We have the challenge of what to do with serialized fields, which might mix some translatable strings with other non-translatable data. E.g., serialized views and field data.
Other thoughts:
I think ideally we'd have a single query tag to use. E.g., 'translate'. If we've marked translatable fields in the schema, that should be enough. (Or possibly we need one for each language type per #282191.)
We'll need to do something to facilitate storage of the original strings for translation. We need to know when a source string has been entered or updated, so that a contrib module can respond accordingly by e.g. adding the source string to the list of strings to be translated or marking existing translations as outdated. We could consider adding an invocation to drupal_write_record(). module_invoke_all('write_record', $object, $primary_keys) or possibly a translation-specific invocation.
Comment #11
sunTagging absolutely critical clean-ups for D7. Do not touch this tag. Please either help with this or one of the other patches having this tag.
Comment #12
nedjoAlongside translatable fields for content, this issue looks to be the breakthrough in thinking we've been needing for localization in Drupal.
Localization of UI strings has two major problems:
1. Because translation is done after the fact, it's possible only through overriding previously-loaded data. If - as is the case for many strings - no explicit opening is provided for such overrides, we get a mish-mash of overrides using numerous different hooks, when in fact localization is possible at all. The D6 Internationalization module reflects this problem, requiring a new module for each type of data to be localized.
2. Performance: translations usually require a new query for every string being translated.
sun's approach solves both problems: localization is done as part of the original load operation, meaning no subsequent overriding and no performance impact.
Yes, we should aim to put the minimum needed in D7 to enable a solution in contrib.
The three minimum steps in #6 look right, with the addition of: add a schema property to fields containing translatable strings.
Suggested schema field property and query tag: 'translate'.
Comment #13
sunThings I left out:
- Aggregator: Feed titles are usually unique/untranslatable.
- Block: #430886: Make all blocks fieldable entities
- Blog, Book, Color, Comment, DB Log, Forum, Help, Image, Locale, OpenID, Path, PHP, SimpleTest, Statistics, Syslog, Tracker, Translation, Update.
- Field, Field UI: Dunno where field labels/descriptions are stored
- Profile: uhm.
- Search: Not sure, whether needed.
- Taxonomy: Needs work -- I have no idea how the new relation to fields works now (maybe catch can help here)
- Trigger: uh oh - what about configurable actions?
- Upload: meh, why does this still exist?
- User: Basically, user role names would be translatable. Furthermore, user signatures could be translatable (though a bit tricky).
Interestingly, the menu_tree* functions and variables seem to be the only places where retrieved values are cached.
That said. Major problem. This patch also includes a change to the variables query. But we cannot translate variables on the fly, because variables are loaded before the language system is initialized. It's a race condition, because the language initialization requires variables... So that will not work. The only solution I can think of would be to make language.inc pluggable (like cache.inc, password.inc, and others) and overload $conf in there - which means that we wouldn't have the desired effect/pattern for variables, but would still be able to translate them.
Comment #14
andypostHere is patch for contact, is it ok to mark as duplicate? #251482: Contact categories can't be translated with i18n
Comment #15
sun@andypost: Thanks for trying to help here, but please read the issue before posting arbitrary patches. That Contact module issue basically won't fix, because of the reasons I provided over there.
Not sure why I didn't change the status... so let's have it a crack at it.
Comment #16
sunRemoved the variables query change. We should consider to open an issue about that plugging of language.inc *now*. ;)
Comment #17
catchOn taxonomy, I'm hoping field translation helps there, however that requires 'taxonomy term name and description as field', which is a Drupal 8 task. There's probably enough of an API in taxonomy.module to workaround it in Drupal 7, but it's not solved. Term loading does/should have a query tag though.
Comment #18
sunOTOH, why don't do the pluggable language.inc right in here. Benefit: Direct context for the already expected "WTF?" question of core maintainers. :)
Comment #19
nedjoThis is looking great. So simple. So powerful.
Two questions:
1. What would tests for this look like? I guess a sample query alter that translates one of the given relevant queries based on schema information?
2. Could we conceivably skip adding the query tag and instead do this at the db api level? We have what we need in the schema. But - and no doubt this is the reason why this won't work - we'd need to consult the schema for every query.
And I didn't quite follow why we need language.inc to be pluggable. Could you explain again? How does this solve the variables problem?
Comment #20
sun@all: Please also help over in #187398: Re-split locale module.
Re #19:
1) Tests, we don't need at this point. The QueryAlterableInterface of DBTNG is already covered by tests. We will just make use of it.
2) Yeah, that was the result of the discussion with Crell in #6 -- we won't touch Drupal core for this. I'm not yet 100% sure whether a single 'translatable' query tag may be sufficient, and whether we shouldn't pass the "type" of translatable query as query meta information instead. DBTNG supports tags and meta information, and while tags just work similarly to taxonomy, the meta information allows you to even pass complex data structures (even entire node objects) along with the query to hook_query_alter() implementations. Of course, our contrib module will parse the schema only once and store the translatable tables/columns in a more efficient variable (whenever the schema is rebuilt).
3) We need to make language.inc pluggable, because the system's variables are initialized very early in Drupal's bootstrap, when the language system is not yet initialized. That means, we cannot apply the concept of DDT to system variables. But what we can do is to replace language.inc with a customized version that, when the language sub-system initializes, retrieves translated variables and overloads the existing. At least, that's all I can think of currently. So the idea behind making language.inc pluggable is just to be able to do this at the appropriate point in time during Drupal's bootstrap.
Comment #21
Crell CreditAttribution: Crell commentedWait, how are you planning to make language.inc pluggable? Swappable include files are a poor approach. Lazy-load classes (a la handlers or the cache system or the stream wrappers system) are a much more stable solution. Then the physical location on disk is irrelevant.
Comment #22
sun@Crell: I fear that we won't have sufficient time for a handler-alike approach... unless you can provide us a really quick way to apply this to the language and localization systems. :-/ Since I doubt that a bit, and we (the i18n team) need a solution here to be able to work on this in contrib, a variable-based approach should be sufficient. If absolutely not doable, then we definitely need help here.
Comment #23
sunDiscussed with Crell...
We no longer want to make language.inc pluggable/swappable, but instead, we just squeeze a single module_invoke_all('language_init') onto the end of http://api.drupal.org/api/function/drupal_language_initialize/7
Help with rolling this patch into RTBC as quickly as possible would be highly appreciated.
Comment #24
sunAnd aforementioned solution translates into this.
What's left? Taxonomy, for sure. Any thoughts/comments on the list provided in #13 ?
Comment #26
tomsm CreditAttribution: tomsm commentedsubscribing
Comment #27
sunOf course we need to use bootstrap_invoke_all(), because the module system is not initialized yet.
Comment #29
sunchx was so kind to clarify that this is an install.php problem only.
Comment #30
sunRemoved the additional query tags that described the data being queried. The "translatable" tag is sufficient, because we will consult the schema to figure out all translatable table columns anyway.
Taxonomy module is still to be done.
Comment #31
sunNow also went through Taxonomy module.
Taxonomy module is kind of a special case scenario: It uses the entity controller for almost all load operations. That means, our tiny contrib module will have to figure out on which pages to apply the dynamic translation and on which not... ;) In general, I guess that we'll skip all queries that happen on admin/* anyway (because this is where we always have to load original values and the contrib module's translation UI needs to kick in in some way), so this is just FYI.
However, this entity controller issue leads to an interesting recursion issue:
To account for this edge-case, we may also have to add the global $language_interface to the entity_info cache key. But I'm not sure about that - and shouldn't hold up this patch.
On a related note, taxonomy terms are a very nice showcase for the new translation functionality in D7, as a quick discussion with plach revealed:
So this one should be near to being ready to fly.
Comment #33
plachThis patch is looking very good and we badly need what it would make possible: a unique, coherent, approach to user-entered string translation.
No remark to make besides the fact we have red light from the bot and we are not supporting some core elements.
Among the others:
{field_config_instance}.data
However if we miss something we'll have time to fix it later, as this does not seem to involve heavy changes.
Comment #34
plachdidn't mean to change the status
Comment #35
sunThis should pass again.
Comment #37
sunComment #39
sunNow for real.
I'm falling half asleep now, so I'm not sure whether I'll be able to re-roll with the further changes to Block module.
Field module seems to store its data in a serialized column, so I'm not sure whether we'll be able to do anything about that.
Comment #40
sunRe-rolled against HEAD. (Taxonomy synonyms are gone)
Comment #41
andypostConfigurable Actions/Triggers saves it's data in serializable {actions}.parameters
I think it's a same as field labels
Comment #42
sun- Actions: I don't think that's something we can handle cleanly. We'll have to use workarounds for that.
The only things we may want to tackle are:
- Blocks
- Field labels (looks equally undoable due to serialization)
- Upload
Comment #43
andypostSuppose only the block title should be translatable
Upload description... what for?
Comment #44
sunMerged that into the patch.
I'm marking this RTBC, because we horribly, badly need this to work on a contributed module during D7 (to possibly aim for core inclusion in D8).
It's clear that we won't get any further API/schema changes in for D7, so if we missed something, we have to workaround that...
Comment #45
Dries CreditAttribution: Dries commentedI'm happy to commit this patch to Drupal 7 still, but I feel we need to document
addTag('translatable')
somewhere. It doesn't have to be a long explanation, but without any context in core, it could be a bit of a WTF. Let's make sure to add the minimal documentation to core.Comment #46
sunThanks - I'm running a bit out of ideas where it would be sane to document this.
I think that Schema API properties are only covered in d.o handbooks, and likewise, detailed documentation about DBTNG select queries and query altering I've equally seen in the handbooks only so far. The concept really only boils down to:
1) DDT translatable data (usually strings) needs to be stored "normalized", i.e. in a separate column.
2) All translatable table fields use the additional property 'translatable' => TRUE in the Schema API definition, i.e. hook_schema().
3) All queries that load translatable data for display in the frontend should use db_select() along with the query tag "translatable".
I guess that only 2 + 3 would have to be documented somewhere in core, but we can certainly add some background info about 1 (the idea/pattern) as well.
Are those "node_access" and "term_access" query tags documented somewhere?
So any pointers to the proper location for documenting this would be helpful.
Comment #47
Crell CreditAttribution: Crell commentedhttp://drupal.org/node/310077 - The DB API documentation includes reference to some common tags. I vote we just add "translatable" there. If there's some better place to put all of them later (in code or otherwise) we can do that, but that's where we're documenting common tags for the time being.
Comment #48
andypostGood idea to summarize query tags, so new tag for issue QueryTags (another candidate #605630: Comment language administration UI with comment_filter tag)
documentation same as #46 is needed - is there i18n page docs?
Comment #49
nedjoA good place for documentation would be http://drupal.org/node/304002, Making your custom data translatable, in the module developer's guide.
As I suggested in #10, I still think we need a hook invocation in drupal_write_record(). The issue is that we don't just need to be able to switch in translations; we also need to know when we need to create them in the first place, and know when we need to update them because the original has changed. A hook invocation would allow a contrib solution to respond to data inserts and updates, storing source strings for future translation and marking existing translations as outdated. Of course, that could be done as a follow up patch.
Comment #50
andypost@nedjo why not use a common {entity}_save and not drupal_write_record() ? seems that data-level is much better then row-level to sync translations
Comment #51
Dries CreditAttribution: Dries commentedI don't see any major downsides of this patch at this point, so I decided to go ahead and commit it. I'm marking this code needs work, so (1) we can follow-up with documentation and (2) we create a CHANGELOG.txt entry.
Comment #52
webchickComment #53
sunRe-adding the critical tag, since this is one of the most important issues for the i18n team to be able to build the envisioned module in contrib during D7 (and make it core-worthy for inclusion in D8).
@nedjo: That's again a good point you're raising - sorry, I didn't really recognize it earlier. Not sure whether dwr() is the proper place, because not all insert/update queries are run through it. I guess that we'll still need an API that allows modules to integrate with this approach upon source data changes. Regarding data in core itself, I think that everything is sufficiently hook'ified, so the contrib module itself could implement the hooks on behalf of core modules. But yeah, let's brainstorm this some more.
Comment #54
andypostI think that hook_query_TAG_alter() is enough for presentation layer but for monitoring a data change events hook_{entity}_[insert|update|delete] is common practice in core.
insertion of drupal_alter() into dwr() or db_insert and others is bad idea because this approach needs tagging all queries in core.
Comment #55
yched CreditAttribution: yched commentedAwesome to see this committed.
It's a bit sad to have Field label and description out of this insanely neat mechanism just because they're stored in a serialized array (until this patch, there was just no use case for dedicated columns).
Splitting those in their own columns should be a simple patch (instance read and write are encapsulated in API functions), I can write this if there's a chance to have this accepted post freeze.
Comment #56
plach@yched: can't we file this as API cleanup?
Comment #57
yched CreditAttribution: yched commentedAPI cleanup I'm not sure, but IMO this stands as a simple followup keeping Field API to-date with a newly added core feature.
I'll post a patch over at #549698: Prepare field label and description for DDT (translatable queries).
Comment #58
plachany chance to adress also the allowed values translation?
Comment #59
sun@yched: That would be plain awesome. I didn't approach it in the patch, because I had no idea where these changes would have to be applied... ;)
Comment #60
sunFYI: Damien just pointed out that there are insane restrictions on the row size AND table definition: http://dev.mysql.com/doc/refman/5.0/en/column-count-limit.html
Hence, like he already suggested, our little module will better go with a JOIN-approach. Although that has limitations, too: http://dev.mysql.com/doc/refman/5.0/en/joins-limits.html - but I think it's less likely that we reach those ;)
Comment #61
sun.
Comment #62
yched CreditAttribution: yched commentedFYI, I posted an initial patch in #549698-16: Prepare field label and description for DDT (translatable queries), but it actually raises more questions than it solves :-/
Comment #63
andypostSo is there any docs about using queryTags for dynamic translation?
Should we put a someTag for every translatable query to identify them?
Comment #64
sun@andypost: Nope, not required for DDT. Only the 'translatable' tag is. Basically.
Comment #65
Dave ReidBTW, thanks for leaving an undocumented hook and not adding hook_language_init to bootstrap_hooks since its invoked before a full bootstrap... #642782: hook_language_init is a bootstrap hook and needs bootstrap_invoke_all()
Comment #66
andypostUser roles are still not marked as translatable #598862-12: Custom user roles are not translatable in core alone (requires a contributed module)
Comment #67
claudiu.cristeaIn #6, @sun:
I see in the patch that you added:
'translatable' => TRUE
in schema for translatable fields. Is only a question.... What is the meaning of this tagging? I cannot see any processing or storage operations with this key. No updates to the Schema API class. I don't understand for what this stands for.Comment #68
drifter CreditAttribution: drifter commentedsubscribing
Comment #69
klonossubscribing...
Comment #70
drifter CreditAttribution: drifter commentedWrote up some documentation at http://drupal.org/node/304002 - this is the first time I've edited a handbook page, so it could use some review...
Comment #71
sunThanks! I've slightly corrected that.
Should this be documented anywhere else, too?
Comment #72
sunLet's mark this fixed for now.
Comment #73
6aKa CreditAttribution: 6aKa commented#13: drupal.translatable.patch queued for re-testing.
Comment #74
sunComment #76
BerdirFYI, I made a proof of concept module that can create translation tables and does the necessary query mangling to display translations. There is no UI yet to enter translations.
I've tested it with roles at admin/permissions/roles, so it kinda works. I'm not sure that it's going to work in general, though. I'm pretty sure that not all necessary queries have been correctly converted, see for example http://api.drupal.org/api/drupal/modules--menu--menu.module/function/men....
Edit: Forgot the link to the project: http://drupal.org/sandbox/berdir/1122562
Comment #77
plach@berdir:
I cited your project in the Drupal 7 multilingual support matrix.