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.This patch adds string context all around, included an small display below source text.
It will be needed by i18n strings.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | context-support.patch | 8.38 KB | Gábor Hojtsy |
| #4 | DateContexts.png | 160.14 KB | Gábor Hojtsy |
| l10n_client_context.patch | 8.26 KB | Jose Reyero | |
Comments
Comment #1
Jerome F CreditAttribution: Jerome F commentedsubscribing - for testing purposes, what should work with this patch that didn't work before?
does "string context" mean that for example, on a page, with the localisation client enabled, i18n strings should now be available for translations as well as the built in interface strings?
Comment #2
Gábor HojtsyPatch generally looks good. Jerome F: you can test this simply by going to transalate the date settings page, it should output May and May in both the regular and long date format context, so it should appear twice with different contexts.
Comment #3
Jerome F CreditAttribution: Jerome F commentedI tested the patch according to what @Gábor Hojtsy suggested and it works as expected. +1 for RTBC
Comment #4
Gábor HojtsyOk, I've also tried the patch, and it seems to work well. I'd like to remove a couple TODO comments to implement this as well as as add escaping to the context value just to make sure it will not break, even though developers are not supposed to use HTML special chars in their contexts, they might. Then this looks good to commit.
Comment #5
Gábor HojtsyOk, here is the patch I'm committing. Thanks for the big help! (Not applicable to Drupal 6, so we'll need to diverge on D7 with this feature).
Comment #7
hansfn CreditAttribution: hansfn commentedDoesn't this issue/fix warrant a new release? It's a major issue IMHO.
Comment #8
hansfn CreditAttribution: hansfn commentedI'm sorry, but I just tested with the dev version (released 2011-Jun-21) and saving didn't work for me. I'm always sharing/sending my translation to localize.drupal.org - maybe that part wasn't tested properly? The client discovers the context, but if I try to save the translation is rejected (by localize.drupal.org) with the following reason:
However, if I check on localize.drupal.org the string is untranslated.
To reproduce try to translate "Orders" with context "a drupal commerce order" (from the Commerce module) - ref (missing) Norwegian Bokmål translation.
Comment #9
Gábor HojtsyIt might be just a suggestion. Are you sure it was a translation?
Comment #10
hansfn CreditAttribution: hansfn commentedI'm 110% sure. I just tested again - trying to translate "Orders". I got the same error (and there is still no suggestion or translation for "Orders").
I had a peak in the code. It seems that you haven't added context support to the function l10n_client_submit_translation (which is called by l10n_client_save_string). The reason probably is that l10n_remote_xmlrpc_string_submit (in l10n_remote/l10n_remote.module) doesn't have context support ...
Comment #11
Gábor HojtsyWell, well, right. Looks like we've only solved it for local editing so far. Right.
Comment #12
hansfn CreditAttribution: hansfn commentedWhich is a pity since some of the beauty of the l10n_client is that you can update translations on localize.drupal.org while fixing your own site - with hardly no extra effort. (It is annoying to approve the suggestions on localize.drupal.org afterwards, but it's very quickly done.) But you know everything about this.
Thanks for the quick response. I'm looking forward to a quick fix ;-) Seriously, I understand it can take some time since you have to fix l10n_server too.
Comment #13
Gábor HojtsyMaybe it can be appreciated that the local editing of context-specific strings were already fixed, unlike before this issue when we were even farther. Contributions are always welcome :)
Comment #14
hansfn CreditAttribution: hansfn commentedOf course, I'm happy that you have fixed the local editing, but telling me that contributions are welcome seems weird considering that I have translated almost 2300 strings to Norwegian Bokmål and that I spend a lot of time carefully reporting bugs I find. (I know that you contribute much more - thx!) I think my time is best spent doing that than trying to write a patch for both l10n_client and l10n_server ...
I guess what I'm trying to say is that the l10n_client is very important for my translation work - it's a brilliant tool for translators. That is why it is a pity that the context support wasn't complete.
Comment #15
Jose Reyero CreditAttribution: Jose Reyero commentedSo, back to the task, it seems we need context support on the client-server connector.
@Gabor,
How can we make the transition smooth between client/server versions with and without context? We need some idea about for which versions to work. Should we aim l10n_server 6.x? 7.x?
I remember there was some API version in the connector, let's see how we can use that for this case.
Comment #16
Gábor Hojtsy@Jose: let's look at the 6.x version for l10n_server (running on l.d.o), but if at all possible, port it to 7.x too. The code is *very* similar, there are purposefully not many changes there.
Comment #17
Jose Reyero CreditAttribution: Jose Reyero commentedThe question is are we upgrading LDO to 7.x anytime soon? If so we can target just that version of the server.
Whatever, to make the move smooth and not breaking thousands of client sites at a time we should code the client side making decisions based on the API version provided by the server.
So:
1. We upgrade the client to send strings with or without context, depending on server api version. Maybe we better skip sending strings with context is the API is the old one.
2. We upgrade the server to start accepting context and informing the client the API version has changed finally. Maybe we better use a different RPC call for that.
Sounds good?
Comment #18
Gábor HojtsyThere are lots of dependencies on l.d.o, so it will be 6.x for now. 7.x is not different, and let's not use special D7 stuff in l10n_server there for now to keep the two in sync, until we can update l.d.o. The string submission backward compatibility idea is great. IMHO skip sending context stuff if server not compatible, we can display an error similar to no permissions, to tell you the server does not support getting this string or something. Thanks for working on this!