Bug 299049 - Cannot set a translation to the empty string
Summary: Cannot set a translation to the empty string
Status: NEW
Alias: None
Product: Babel
Classification: Technology
Component: Server (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Babel server inbox CLA
QA Contact:
URL: /babel/translate.php
Whiteboard: stalebug
Keywords: needinfo
Depends on:
Blocks: 295122
  Show dependency tree
 
Reported: 2010-01-07 10:42 EST by Markus Keller CLA
Modified: 2024-03-29 05:30 EDT (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Keller CLA 2010-01-07 10:42:42 EST
I cannot set a translation to the empty string in Babel. Concretely, I need this here:
http://babel.eclipse.org/babel/translate.php?project=eclipse&version=3.4&file=org.eclipse.ltk.ui.refactoring/src/org/eclipse/ltk/internal/ui/refactoring/RefactoringUIMessages.properties&string=RefactoringHistoryLabelProvider_label_country

... to fix bug 295122.

E.g. for the Arabic translation, the original "US" should be translated to "". When I submit that, it first looks as if it worked (just the message "Translated 0 strings across all Babel projects." is wrong). But when I go to another string and the back, the old value "US" is there again (only in the lower area, not in the "Strings In File", where it still looks like it worked).

Note that an empty string is different from an absent string. I guess the "Non-Translatable" checkbox removes a string from the translated properties file (at least that's what I see for key "RefactoringHistoryLabelProvider_label_variant").

When a string is absent in a language pack, the fallback is to use a string from another language pack, or the original string. But in this case, the string really needs to be the empty string, since it must *override* the string from the original file. I found no way to do this.

I found bug 217257, but it contains no justification why an empty string should not be saved. If that's what causes these bugs, then please revert that patch.
Comment 1 Kit Lo CLA 2010-01-07 11:54:26 EST
I'm not sure how the country string is being used in the code. But, shouldn't we "translate" "US" to "AR"?

Another workaround, enter "\ " (to force an escaped space) should probably work.
Comment 2 Markus Keller CLA 2010-01-07 13:07:23 EST
> I'm not sure how the country string is being used in the code. But, shouldn't
> we "translate" "US" to "AR"?

The original file contains comments that describe these properties, but unfortunately, that's not visible in Babel:
## The country, language and variant codes need to be changed to match the final locale of the translated property file

The strings are used as arguments for new java.util.Locale(lang, co, var), so for the language pack "ar", the country and the variant should be empty.

Your guess ("AR") is wrong, that would mean Argentina (and I don't think the size of the Arabic community in that country would warrant a separate language pack;-)

> Another workaround, enter "\ " (to force an escaped space) should probably
> work.

I don't know, and I found no documentation about the syntax supported by that field. I'm not a fan of guesswork.

I also don't know how I could test this. Does anybody know how to make a test build? Or at least where I can check out an up-to-date version of a translated properties file?
Comment 3 Denis Roy CLA 2010-01-07 15:28:43 EST
> > Another workaround, enter "\ " (to force an escaped space) should probably
> > work.
> 
> I don't know, and I found no documentation about the syntax supported by that
> field. I'm not a fan of guesswork.

Hence the reason a workaround was suggested to you, because even if there is documentation, you won't read it (see below)  :)  


> I also don't know how I could test this. Does anybody know how to make a test
> build? Or at least where I can check out an up-to-date version of a translated
> properties file?

You test this the same way you test code committed to CVS -- wait for the next nightly build.

From the Babel translation server, hit "Home" > Download a Language Pack > Developer and Nightly Builds > pick the Nightly build that will be created after your change.
Comment 4 Markus Keller CLA 2010-01-08 10:03:26 EST
(In reply to comment #3)
Thanks. In the Eclipse project, we usually test stuff *before* we release it (even into an N-build). Hence, I was narrow-mindedly looking for a way to check the result without waiting for an N-build. That shall teach me that translation is not only about 1:1 mappings but also about understanding foreign customs and traditions ;-)

I've now released "\ " for Arabic.
Comment 5 Markus Keller CLA 2010-01-14 09:14:57 EST
Entering "\ " in the Babel tool gives line:
"RefactoringHistoryLabelProvider_label_country=\ "
=> That's wrong (the property should be empty, not a space). However, if all else fails, we could still use that, since java.util.Locale is resilient to invalid country strings and just falls back to the default, so this works by chance.
(Tested in org.eclipse.ltk.ui.refactoring.nl_ar_3.5.0.v20100113043401.jar)

Entering "\" in the Babel tool gives line:
"RefactoringHistoryLabelProvider_label_country=\"
=> Fails horribly, since a "\" at the end of a line means that the property is continued on the next line, so the value of this property contains the next property key, and the next property in the file disappears completely!
(Tested in org.eclipse.ltk.ui.refactoring.nl_ar_3.5.0.v20100114043402.jar)

Summary: I have no way to fix this properly, so please revert bug 217257 or explain why you think it was a valid change.
Alternatively, you can also change the check so that it does not block empty translations, but translations that end with "\" (do not trim the string!).
Comment 6 Markus Keller CLA 2010-02-02 08:52:41 EST
Could you please fix this with priority?

The translation packs are currently broken and I'd like to fix them ASAP so that I can test the fix with the upcoming Galileo service release (3.5.2).
Comment 7 Kit Lo CLA 2010-02-02 14:08:03 EST
Mark the string as "non-translatable" using the check-box at the lower left corner of the translate page will tell the NL build process not to pull the translation into the language pack. Take a look at this language pack:

http://build.eclipse.org/technology/babel/babel_language_packs/N20100202-1400/BabelLanguagePack-eclipse-ar_3.5.0.v20100202015458.zip

I think that will solve your problem.
Comment 8 Markus Keller CLA 2010-02-03 06:17:41 EST
No, as I explained in comment 0, two of the strings [1] need to be set to the empty string, and that's *not* the same as omitting the entire key/value pair.

Furthermore, the "Non-Translatable" setting seems to be a global option, but there properties must be set for some of the translations (e.g. "Portuguese (Brazilian)" needs "...country=BR", but for most other translation packs, country must be empty.

I really don't understand why it's so hard to just fix this bug. If you don't want to just revert bug 217257 and need more context to understand the even better fix mentioned at the end of comment 5, please ask.


[1] For locale "ar", the values for RefactoringHistoryLabelProvider_label_country and RefactoringHistoryLabelProvider_label_variant must be the empty string.
Comment 9 Denis Roy CLA 2010-02-03 09:51:17 EST
(In reply to comment #8)
> I really don't understand why it's so hard to just fix this bug.

As a committer, I'm sure you've heard that before.

The reason bug 217257 was put in place is because we have mechanisms for finding binary matches of English strings and applying the same translation to all of them -- regardless of the project.  This helps complete the translation faster by requiring less effort on the translators, and ensures translation portability from one version of a project to the next even if the keys have been renamed, or moved to another properties file.

In this specific case, all untranslated instances of English string "US" in the Arabic language would be set to the empty string, which a) has a higher likelyhood of being incorrect and b) potentially adds unnecessary records to a very large database table, which slows down the UI.

Of course, we can add a bunch of conditionals to work around this, but that would require testing, and would introduce possible regressions which, in the end, would explain why it's so hard to just fix this bug.

If you have an easy and safe way of solving this that has no impact on database performance, by all means please attach a patch and I'll review it immediately.  

One possible solution I can think of would be to undo bug 217257 and add a "do not propagate" flag in the translations table (and a checkbox in the UI).  We could then alter the setStringTranslations.php and syncup.php routines to not propagate this translated value.
Comment 10 Markus Keller CLA 2010-02-03 12:11:16 EST
(In reply to comment #9)
Thanks. With that explanation, bug 217257 finally makes some sense. I don't speak PHP, so I'm afraid I can't help you find a better fix for the underlying issue.

Since I now know the cost of fixing this bug, I think the best tradeoff is to just live with it for now. I'll go and fix bug 295122 by setting the country to "US" for language packs where it should be empty. That's not correct, but it's at least a legal value (" " may work in some cases but is not legal according to the java.util.Local API).
Comment 11 Denis Roy CLA 2010-02-03 13:31:17 EST
Thanks for understanding.

Let's go with my approach in comment 9 (whenever we get to doing this).

In the meanwhile, I believe we will be spinning a new "release" of the language packs in alignment with Galileo SR2, so perhaps what we can do is edit those two keys manually for Arabic.  That would buy us some time to fix this bug.

Kit, does this work for you?
Comment 12 Kit Lo CLA 2010-02-04 01:32:56 EST
Good plan! +1
Comment 13 Markus Keller CLA 2010-02-04 08:54:53 EST
> edit those two keys manually for Arabic

So you want to put the empty string as value into the database directly? If so, please set the following for *all* languages (not just Arabic) in all versions:
RefactoringHistoryLabelProvider_label_country=
RefactoringHistoryLabelProvider_label_variant=

I can then set the right _country for the two packs that need it ("Chinese (Traditional)" and "Portuguese (Brazilian)"). And I'll correct RefactoringHistoryLabelProvider_label_language and the other properties.
Comment 14 Denis Roy CLA 2010-02-04 09:45:46 EST
> So you want to put the empty string as value into the database directly?

No, otherwise the sync mechanisms will find these translations and possibly apply them elsewhere.  I was thinking of unzipping the jars after a build, editing the files manually, then jarring them back up, but I was also assuming this only needed to be done for Arabic.  If all the languages need to be done, I can likely wave some bash magic.
Comment 15 Jacek Pospychala CLA 2010-02-04 10:00:50 EST
(In reply to comment #14)
> I can likely wave some bash magic.

Denis, what you suggest sounds crazy.
How about changing the server/html/callback/setStringTranslation.php

from
 
if (empty($translation) || (trim($translation) == '')) {

to

if (isset($_GET['ENABLE_EMPTY_STRINGS']) && (empty($translation) || (trim($translation) == ''))) {


Then possibly Markus could do his stuff, by appending "&ENABLE_EMPTY_STRINGS=1" to his Babel URL.

Note, that I haven't tried this, and all of above is guessed based on the original patch attached to bug 217257 which supposedly caused this bug.
Comment 16 Jacek Pospychala CLA 2010-02-04 10:03:43 EST
(In reply to comment #15)
> if (isset($_GET['ENABLE_EMPTY_STRINGS']) && (empty($translation) ||
> (trim($translation) == ''))) {

Sorry, actually it should be:

if ((!isset($_GET['ENABLE_EMPTY_STRINGS'])) && (empty($translation) || (trim($translation) == ''))) {
// do nothing
}


So do nothing if translation is empty and ENABLE_EMPTY_STRINGS flag is NOT set..
Comment 17 Markus Keller CLA 2010-02-04 11:49:11 EST
(In reply to comment #14)
Nah, I don't want the language packs to be out-of-sync with the database, and adding magic to the build process feels like overkill for a problem that can also be worked around differently. I'll rather use "US" in all language packs. I've verified that this works fine for all languages (same behavior as with an empty country string).

I don't need a workaround any more, and we'll keep this bug for full support as Denis suggested at the end of comment 9.
Comment 18 Eclipse Genie CLA 2014-05-16 13:26:34 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.
Comment 19 Eclipse Genie CLA 2016-05-06 06:44:46 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.
Comment 20 Eclipse Genie CLA 2018-04-27 07:31:16 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.
Comment 21 Eclipse Genie CLA 2020-04-17 00:56:46 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.
Comment 22 Eclipse Genie CLA 2022-04-08 02:31:32 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.
Comment 23 Eclipse Genie CLA 2024-03-29 05:30:10 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.