51327 - mozilla doesn't parse java properties files correctly

Status: VERIFIED FIXED

(Core :: Internationalization, defect, P3)

Description

[Henrik Lynggaard Hansen]

Mozilla fails to parse the escape character \ properly. It leaves it in the text 
string and thus they show up in the ui.

This is a problem since mozillaTranslator does use this character to write 
proper  java properties files.

The escpape char is valid in a java property file, and according to tao cheng 
mozilla should follow the java standard

Comment 1

[Frank Tang]

I think this is an invalid bug. We do properly escape unicode in .properties 
file. You have to use \u4e00 but not \4e00 ....
You are welcome to reopen this bug if you can provide test cases to show the 
problem. 
Mark it as invalid.

Comment 2

[Henrik Lynggaard Hansen]

the followingg was reported in bug 48753:

MT4 adds to every ":", "!" and "#" sign a preceeding backslash in .properties 
files, so that they read "\:", "\!" and "\#".
In original (en-US) files, those backslashes are not used, and Mozilla shows 
that backslashes (I saw at least the ones before ":" signs in de-AT version) in 
its UI.
That way, you have to manually clean out those backslashes if you want to 
release a working L10n package!

but Kairo, can you give a test-case (added him to the CC)

Comment 3

[nhottanscp]

Reassign to tao.

Comment 4

[Frank Tang]

In http://java.sun.com/products//jdk/1.2/docs/api/java/util/Properties.html
it said-
Every line other than a blank line or a comment line describes one property to 
be added to the table (except that if a line ends with \, then
     the following line, if it exists, is treated as a continuation line, as 
described below). The key consists of all the characters in the line starting
     with the first non-whitespace character and up to, but not including, the 
first ASCII =, :, or whitespace character. All of the key
     termination characters may be included in the key by preceding them with a 
\. Any whitespace after the key is skipped; if the first
     non-whitespace character after the key is = or :, then it is ignored and 
any whitespace characters after it are also skipped. All remaining
     characters on the line become part of the associated element string. Within 
the element string, the ASCII escape sequences \t, \n, \r,
     \\, \", \', \ (a backslash and a space), and \\uxxxx are recognized and 
converted to single characters. Moreover, if the last character
     on the line is \, then the next line is treated as a continuation of the 
current line; the \ and line terminator are simply discarded, and any
     leading whitespace characters on the continuation line are also discarded 
and are not part of the element string. 

also

Then every entry in this Properties table is written out, one per line. For each 
entry the key string is written, then an ASCII =, then the
     associated element string. Each character of the element string is examined 
to see whether it should be rendered as an escape sequence.
     The ASCII characters \, tab, newline, and carriage return are written as 
\\, \t, \n, and \r, respectively. Characters less than and
     characters greater than ~ are written as \\uxxxx for the appropriate 
hexadecimal value xxxx. Space characters, but not embedded or
     trailing space characters, are written with a preceding \. The key and 
value characters #, !, =, and : are written with a preceding slash to
     ensure that they are properly loaded. 


It seems we need to fix xpcom/ds/nsPersistentProperties.cpp
tao- can you check adding the following lines (with + will fix it or not
158                case 'r':
159                  value.AppendWithConversion('\r');
160                  break;
+                  case '#': 
+                  case ':': 
+                  case '!': 
+                  case '=': 
+                    value.AppendWithConversion(c);
+                    break;
161                default:
162                  value.AppendWithConversion('\\');
163                  value.Append((PRUnichar) c);

erik- can you code review it ?

Comment 5

[Frank Tang]

we need this to support properties file correctly. Without it, it will increase 
a lot of localization cost

Comment 6

[Frank Tang]

*** Bug 48008 has been marked as a duplicate of this bug. ***

Comment 7

[Kazu Yamamoto]

Hi. I'm the first this bug reporter to Henrik and Tao. I'm
looking forward to fix this problem. But I can't wait nsbeta3+.
So, I fixed this problem in MozillaTranslator side.

Please look at the following article.
news://news.mozilla.org/39C33889.4BADC24C%40tky2.3web.ne.jp 

Comment 8

[Frank Tang]

since we have work around now. We (tao and I) decide to minus this bug for now. 

Comment 9

[Henrik Lynggaard Hansen]

setting rtm flag, as this bug must be fixed ASAP although the nsbeta3- flag is 
understandarble.

Resons:

* People might use other l10n tools, 

* the workaround solves the problem by writeing NON-STANDARD java properties 
files.

Comment 10

[Kazu Yamamoto]

The following URL said that 
"You can use your favorite encoding during translation and convert it
 to escape-unicode via the native2ascii utility program available
 on most Unix systems. Java SDK also includes native2ascii."

http://www.mozilla.org/projects/l10n/mlp_what2.html#chrome_files

But native2ascii put backslash before a few character because
it's java properties style.

nsconv(.exe) including mozilla doesn't put backslash.
It's current mozilla properties style.

I think if you remove the sentence of native2ascii from above URL,
this problem will be fixed because mozilla use mozilla properties
style not java properties style.

Comment 11

[Frank Tang]

tao- what is your opinion?

Comment 12

[tao]

Sorry for the late reply.

I suggest we take your patch with extreme caution: get review from Erik who
wrote the original code. Then, test it with previous property files parsing bugs
(such as the '\n'.'\t',etc..) Also, try using nsconv for x-escape-unicode
conversion in both direction. If there is no regression, let's take the
patch.


how does this sound?

Comment 13

[tao]

Who don't we check this in into the trunk since the patch seems OK?

Comment 14

[tao]

r=tao.

Hi, Erik:

Could we get a super-review from you?

Comment 15

[Erik van der Poel]

I reviewed the current code and the patch above, and I disapprove. If the
intention is to be compatible with Java property files, then the line that adds
a backslash to the value in the following code is bad, and should be removed:

161                default:
162                  value.AppendWithConversion('\\');
163                  value.Append((PRUnichar) c);

I.e. line 162 is bad, and should be deleted. When that is done, then the patch
above won't even be necessary.

However, it may not be a good idea to check this into the branch at this point,
since I don't know how many property files depend on this bug.

Comment 16

[msanz]

Frank, should this be rtm-?

Comment 17

[Frank Tang]

*** Bug 7605 has been marked as a duplicate of this bug. ***

Comment 18

[selmer (gone)]

rtm-, suggested by msanz and no activity.

Comment 19

[tao]

I tested Erik's suggestion (taking out line 162) and things starts to work. The
property file parser does remove the extra '\\' before inserting the value to
resource bundle.

I also tested it values that contain Japanese text. It appears to work properly.
I am attaching the proposed patch below.

Comment 20

[tao]

Attachment: new patch suggested by erik.

Comment 21

[tao]

Hi, Erik:


Mind to review the patch again.

thx

Comment 22

[Erik van der Poel]

Tao, I'm the author of that fix (since I suggested it). So someone else should
review it. Maybe you'd like to review it?

Comment 23

[tao]

Yup, r=tao. And assign to you :-) You'd need to get a sr.

Comment 24

[Erik van der Poel]

Tao, since I will be going on vacation soon, I won't have enough time to
respond to any property file problems that might arise from this change.
Please check in the fix after doing some testing. Thanks!

Comment 25

[Alec Flett]

sr=alecf, seems like the right thing, but you guys would know for sure.

Comment 26

[tao]

fixed in the trunk

Comment 27

[tao]

.