88050 - 0x5C (backslash) MUST be kept as it is under Unix/Linux for Korean

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[Jungshik Shin]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux 2.2.19-5mdksecure i586; en-US; 0.8)
Gecko/20010215
BuildID:    20010607 : m0.9.1

0x5C (backslash) is converted to WON Sign
(U20A9) by  layout/html/base/src/nsTextTransformer.cpp
(see 
 nsTextTransformer::LanguageSpecificTransform()
 around line 693)
This is fine for MS-Windows where Windows-949 is used.
However, this should NOT be done for Unix/Linux.
 

Reproducible: Always
Steps to Reproduce:
Go to the URL above under Unix/Linux 


Actual Results:  '?' mark is used where 'backslash' should be (e.g.
C:?windows?system)

Expected Results:  'backslash' should be used instead of '?'.


'?' is used because the 
glyph for U20A9 (WON SIGN) is not available. 
Even if the glyph for U20A9 (WON SIGN) is available,
'backslash' should be preserved instead
of being converted to WON SIGN (at least in Unix/Linux).
KS X 2901 (where EUC-KR is defined) stipulates
that backslash be given a higher priority over WON 
SIGN. 

Even for MS-WIndows(where Windows-949 is
used), this conversion is not necessary
because even without this conversion, backslash
would be displayed as 'WON SIGN' under MS-Windows
since most Korean fonts for MS-Windows have
'won sign' at the position of 'backslash'.

  I suggest either of the following two fixes:

  A. Remove the following code for Korean 
     in nsTextTransformer.cpp     : 
     it will work correctly for both MS-WIndows
        and Unix(-like OS) and MacOS

717 else if (mLanguageSpecificTransformType ==
718 eLanguageSpecificTransformType_Korean) {
719 for (PRInt32 i = 0; i < aLen; i++) {
720 if (aText[i] == 0x5C) { // BACKSLASH
721 aText[i] = 0x20A9; // WON SIGN
722 SetHasMultibyte(PR_TRUE);
723 *aWasTransformed = PR_TRUE;
724 }
725 }


   B. Make the code above activated for only
      MS-Windows. Perhaps, conditional-compilation
      using #ifdef is better than adding
      another condition in if-statement for
      platform detection at run-time
      because Unix/Linux does NOT need the code
      at all. (as I said above, MS-Windows does
      NOT need it, either. Therefore option A is
      preferred to option B)


BTW, I think the same is true of Japanese
so that nsTextTransformer::LanguageSpecificTransform()
beginning at line 693 can be removed.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

->Internationalization

Comment 2

[nhottanscp]

Reassign to ftang.

http://lxr.mozilla.org/seamonkey/source/layout/html/base/src/nsTextTransformer.cpp#693

Comment 3

[Frank Tang]

>BTW, I think the same is true of Japanese so that
>nsTextTransformer::LanguageSpecificTransform() beginning at line 693 can
>be removed.
No, these lines are added by erik to address problem in Japanese. I can trust 
your judgetment about Korean, but don't speak for Japanese please. 

Comment 4

[Frank Tang]

I trust [e-mail] (Jungshik Shin) about any Korean iseue. 
Here is a patch

Comment 5

[Frank Tang]

Attachment: patch accofding to [e-mail] (Jungshik Shin)

Comment 6

[Frank Tang]

jshin: can you r= this ?

Comment 7

[Jungshik Shin]

Thank you for the patch.

The patch looks fine. 
r=jshin

Comment 8

[Blake Ross]

Missed 0.9.3.

Comment 9

[Frank Tang]

move to m0.9.5

Comment 10

[Frank Tang]

ask sr= today.

Comment 11

[msanz]

nsbranch- since Frank moved it to 0.9.5

Comment 12

[Frank Tang]

Comment on attachment 41328 [details] [diff] [review]
patch accofding to [e-mail] (Jungshik Shin)

r=ftang
The code change is simple. And we trust jshin's answer for all
Korean related issue since he is one of the most
knowledge person about Korean Software.

Comment 13

[Chris Waterson]

CVS will never forget this code; how likely are we to need it again?

Comment 14

[Frank Tang]

yea, we can remove the whole chunk instead of #if 0 it. 
I think no one have strong opinion about it. 

Comment 15

[Frank Tang]

Attachment: v2 of patch

Comment 16

[Frank Tang]

jshin- can you r= the v2 patch ?
waterson - can you sr= this patch ?

Comment 17

[Jungshik Shin]

r=jshin

Comment 18

[Chris Waterson]

looks good. sr=waterson

Comment 19

[Frank Tang]

fixed and check in