190283 - Unicode escaped characters don't work in .properties

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P1)

Description

[Robert Kaiser]

In localized language packs, we escape charaters all over using 4-character
unicode values, and this seems not to work with current builds, I guess it
regessed with the checkin to bug 121341.

This basically means that lots of strings in my localized build don't work
currently, e.g. in Page Info / Security I get
"Die Website bugzilla.mozilla.org unterst00Ftzt keine Verschlsselung fr die
Seite, die Sie ansehen."
instead of 
"Die Website bugzilla.mozilla.org unterstützt keine Verschlüsselung für die
Seite, die Sie ansehen."

The line in pippki.properties is as following (just to help to detect the error):
pageInfo_Privacy_None1=Die Website %S unterst\u00FCtzt keine
Verschl\u00FCsselung f\u00FCr die Seite, die Sie ansehen.

It's three times the same character here, I have other strings with other
escaped characters as well, and most times they're just left out, as here in two
of the three cases.

Comment 1

[Alec Flett]

hmm.. they worked in my testing.. are you sure you aren't seeing bug 190256?

Comment 2

[Alec Flett]

Attachment: actually append the unicode value

argh, I spoke to soon. My 'testing' was following it in the debugger, to make
sure the unicode value was parsed correctly.. which it was.. the only problem
was that it never did the final step of appending it to the string.
here's the fix.

Comment 3

[Alec Flett]

Comment on attachment 112418 [details] [diff] [review]
actually append the unicode value

Looking for reviews. Basically I moved the detection of the 4th quartet to the
point after it has been appended. at that point, we append the value.

the trick was recovering from errors.. we don't actually try that hard, but
append the unicode character fragment that we do encounter.

jag/jst do you mind reviewing?

Comment 4

[Johnny Stenback (:jst)]

Comment on attachment 112418 [details] [diff] [review]
actually append the unicode value

r/sr=jst

Comment 5

[Alec Flett]

Comment on attachment 112418 [details] [diff] [review]
actually append the unicode value

asking for 1.3b approval now

Comment 6

[Asa Dotzler [:asa]]

Comment on attachment 112418 [details] [diff] [review]
actually append the unicode value

a=asa (on behalf of drivers) for checkin to 1.3beta.

Comment 7

[Owen Marshall (Not reading bugmail)]

Checkins to module MozillaTinderboxAll by alecf%netscape.com between 01/25/2003
14:12 and 01/25/2003 14:16 :

fix for bug 190283 - save unicode values stored like \u4ea2 after decoding them.
r=jst, sr=bzbarsky, a=asa

Shouldn't this be marked fixed?

Comment 8

[Boris Zbarsky [:bzbarsky]]

Sure, once the bug owner decides to. Notice that it's not even cycled on some of
the tinderboxes yet (eg AIX).  And that alecf may want confirmation from people
actually testing the builds with the patch.  Or that he may just be out to lunch
on a Saturday afternoon.

Comment 9

[Robert Kaiser]

The basic thing here works now, thanks.

I still see a problem here (only in this one string though):

pageInfo_Privacy_None1=Die Website %S unterst\u00FCtzt keine
Verschl\u00FCsselung f\u00FCr die Seite, die Sie ansehen.

is now displayed as

"Die Website bugzilla.mozilla.org unterst00ützt keine Verschlüsselung für die
Seite, die Sie ansehen."

You might notice that when I posted the bug, I got raw "00F", I got only raw
"00" in the last days, now I get this raw "00" plus the (correct) character "ü"
displayed here - looks quite strange to me...

Comment 10

[Alec Flett]

that is odd! you said you're only seeing it with that string - does that mean
other strings are working? is it only ones that begin with '\u00..'? i'll
investigate this more as well, but I'd love to hear more details

Comment 11

[Robert Kaiser]

alecf:
I haven't found any other strings yet where this is happening for me, but it's
very reproducible with this one. A very strange thing, that's true...
I'm only using '\u00..' characters, German doesn't have any other characters
that have to be encoded...

Comment 12

[Asa Dotzler [:asa]]

removing from 1.3beta blocker list.

Comment 13

[Alec Flett]

I'm going to be poking at this soon.. hopefully in time for 1.3beta... I'd like
to hear if anyone else has special strings that wont decode.

Comment 14

[rbs]

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

Comment 15

[rbs]

Unfortunately, this regression breaks the processing of unicode characters in
unexpected ways. See duplicate bug 191693 which clearly shows that things go
awry in the handling of unicode sequences.

Re-requesting the evaluation of drivers, even for the opportunity of a
last-minute back-out. I see that alecf has already given P1 to this bug. While
awaiting a fix, it would be good if drivers could allow room for a last-minute
fallback option, pending further investigation of the remaining issues.

Comment 16

[Alec Flett]

yeah, I'm wondering if I should just backount my entire .properties optimization
and make another run at it in 1.4. This patch has already landed in the current
1.3b, but it sounds like we're STILL seeing problems... 

Comment 17

[rbs]

Clearing flags since alecf has backed out his optimization.

Comment 18

[Alec Flett]

ok, I backed out the original optimization, so this bug is working now.. but I'm
going to re-land it in 1.4alpha so I can test this string again.

Comment 19

[rbs]

I wonder if it is worth the trouble. I didn't see much changes in the Tinderbox
timings after the back-out. It was as if the optimization wasn't perceptible.

Comment 20

[Alec Flett]

argh, so this is actually fixed, since I backed out my whole checkin... we'll
reopen if I reland, and this problem is still appearent.