Interface text translatability warnings

Any suggestion about what to do about most of these?

For example:

The first parameter to t() should be a literal string. There should be no variables, concatenation, constants or other non-literal strings there. At t($node->webform['preview_title']) in webform/webform.module on line 4075. Read more at http://drupal.org/node/322732

I am not sure what quicksketch had in mind when he used t() here. The usage is that the preview_title is used, if set by the user, in preference to t('Preview'). This is a common pattern in webform. Does perhaps the webform_localization module take advantage of this? I would be very surprised if this is a usage error, given the code author.

Some of the others might be fixable -- the watchdog ones for example.

I'm not sure what to do about the long concat. Certainly we're not going to put that on one line of code. I suppose it could be split into multiple calls to t(), although the context will be difficult.

I think the first one and the last one should be the only-ones using t(). The first is tricky because it's a very long text - I'm unsure what the policy is for that.

The variables shouldn't be passed through t() at all. i18n_node does take care of that if used (=There is one node per language). webform_localization seems to be the way to go if entity_translation is used. In other words: Those variables should be translated the same way as components (title, description, placeholders, …) are translated.

There are four different cases. Let's label them for convenience of discussion and resolution:

1. t($node->webform['some_constant']). Lots of these.
2. t('really'. 'long' .'text'). One of these, for token help.
3. watchdog('some description' . make_me_safe($scary_text)). One of these, for spam.
4. watchdog($some_already_translated_text). One of these, for an export message.

1) The tricky one. I *think* this is already handled by webform_localization by virtue of the localized webform node. But I don't use localization. @torotil suggests changing the way we handle this, but if it works today, can we simply remove the t()?
2) Let's just break this into a bunch of short calls to t(), with the html tags interspersed outside of the translatable text. A little more work for the translators, but solves the problem without creating a huge text field or requiring in-constant new lines.
3) The $scary_text should be moved to a !scary_text substitution pattern.
4) That's a bug. The message should be unfactored (defactored?) and watchdog called without translation.

Regarding the #1 situation (translating an entire variable), this behavior was intentional for the time in which the code was written (prior to Webform Localization). Although generally discouraged, calling t() on a variable is still very common in the D7 world. For example, all labels and descriptions provided by Field module are translated by running through t(). Doing a search on D7 core code base, there are 37 instances where t() is run directly on a variable.

So overall, the code was intentional for a time in which there were no other options for translating Webforms. Honestly, I think probably the majority of webforms probably continue to be translated in this manner, considering Webform Localization is relatively recent, still doesn't have an official release, and is used by a small fraction of the total number of Webform installs.

So although t() directly on labels, descriptions, etc, is discouraged, I don't think we should remove this approach from the Webform 4.x code base as it will likely cause substantial disruption on existing translated forms.

… I think probably the majority of webforms probably continue to be translated in this manner …

I disagree. I doubt that anyone has managed to get a sensible translation of a single webform using this method: The t() calls allow you to translate some labels on the webform. They don't allow you to translate anything on the components. That means no translations for: labels, descriptions, options, placeholders, … you name it. How useful is that? I think not at all.

I guess the way users did translate webforms is by using i18n_node (which was the way to translate content before entity_translation / webform_localization - and is still the only way that actually works). i18n_node creates one node per language. That means there is no need for webform to do any translation of the forms at all.

The only thing those t()-calls achieve now is that some already translated content shows up in the list of to-be-translated strings, thereby spamming the translation tables.

Here is what I'd propose as a solution to the problem.

The long string still is not perfect. Translating it in one multiline-piece would be a lot easier to translate instead of piece by piece. But at least now the parts that shouldn't be changed when translating are turned into arguments. Also I think the english version could use some improvements: I had to read it several times before I got what it actually means.

Fixup a syntax error.

Forgot to attach the patch. Sorry for the noise.

@roball: True. I forgot to include that too. I've added the tags now.

Ack. We're all working in parallel.

Here's my plan:

1) Commit items 2, 3, and 4 to the 7.x-4.x branch and 8.x. I have the code tags, removed the quotes, and removed the extra ? that crept in. I also reformatted the code a bit for readability at a tiny performance hit of extra concats. I also put the code tags in the translatable string, rather than the substitution because I think it is common to leave strong / em and other in-line formatting tags in the translatable string for flexibility. I can imagine not wanting them in an ideographic language, for example.

2) Remove the extra t($x) calls for the 8.x branch, relying on the (currently imaginary) d8 version of webform_localization.

I don't think the t($x) calls do much harm, and since there is disagreement about how disruptive it would be to remove them, and since I don't see and way to confirm one opinion or the other, the webform policy is to err on the side of continuity.

Step 1 (everything but t($x)) committed to 7.x-4.x and 8.x.

... and committed the t($x) fixes to 8.x.

Thanks for your help, guys. If in the future there is consensus about applying this patch to 7.x-4.x, we can always do so.

I don't think the t($x) calls do much harm

… that some already translated content shows up in the list of to-be-translated strings, thereby spamming the translation tables.

The later might be considered a bug. With ~10.000 strings in locales_source a dozen of additional strings per webform-node might be annoying but don't do harm directly.

I'd still argue that the t()-calls can't have been used for any translation - simply because they don't allow to translate all relevant strings of a webform (given it has at least one component). It still misleads users to think that that's how translating webform might be done which further complicates an already complicated matter.

If/when quicksketch agrees with you, we'll commit #22 to 7.x-4.x. The patch is all ready to go. Absent that, I'm not comfortable overruling webform's creator whose Drupal experience is several times mine.

Hmmm. Lost a comment in the wind. Yes, that's what I committed. Thanks.

I looked again at what we're calling t() on in Webform, and it looks like we already removed the t() calls around labels, descriptions, etc. We must have done that when we introduced the #translatable property.

It looks like what remains are items that are outside of the ability for Webform Localization to translate, e.g. the Submit button, page labels, the progress bar, etc. I believe Webform Localization can only translate the contents of components, so anything outside of a component is still dependent upon t().

So unless I'm mistaken, these calls to t() allow an entire form to translated within a single node (which is what Webform Localization aims to provide). If we did remove these remaining t() calls, there would be no available mechanism to translate these last pieces. Perhaps Webform Localization could expand to cover these remaining areas, but I don't think it's capable of translating these "extra" strings related to Webform configuration but not within a component itself.

I already commited what is in #29. Testbot go away.

@roball -- not your fault, and no biggie. Thanks for your help.