7964 - [converter]Need ISO-2022-KR converters

Status: RESOLVED FIXED

(Core :: Internationalization, defect, P3)

Description

[Frank Tang]

change to M14 since it is post beta.

Comment 1

[Frank Tang]

change to M20 since no one want to help yet. We probably only need To unicode
conversion only.

Comment 2

[Frank Tang]

reassign to cata and add jbetak/nhotta/erik to the cc.

Comment 3

[Katsuhiko Momoi]

Frank, we will have a problem in displaying legacy mail
messages in Korean. After all, prior to 4.5, we used to 
send out msgs in ISO-2022-KR.

Comment 4

[Frank Tang]

move all cata's bug to ftang

Comment 5

[Frank Tang]

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

Comment 6

[watanabe]

Though I am not so familiar with ISO-2022-KR encoding, I have
made a patch of ISO-2022-KR encoder/decoder from ISO-2022-JP encoder/decoder. 
Would you test it?

Comment 7

[watanabe]

Attachment: This is a patch for ISO-2022-KR encoding/decoding.

Comment 8

[Jungshik Shin]

  I also have a patch for this, but my patch is only for ISO-2022-KR
to Unicode direction, because there's NO need to generate ISO-2022-KR
any more while Mozilla needs to support ISO-2022-KR to Unicode conversion
to deal with messages sent with legacy clients that still generate ISO-2022-KR. 

  Once bug 75707 and bug 88944 are landed into trunk, I'll test my patch
and upload it here for comparison with Watanabe san's patch.

Comment 9

[Frank Tang]

 Jungshik Shin :
can you review  [e-mail] 's patch since you are
korean guru?

jshin- once you r= this one, please ask [e-mail] to sr it  and ask
yokoyama to check in for  you. thanks. 

Comment 10

[Roy Yokoyama]

accepting for helping to checkin.

Comment 11

[Roy Yokoyama]

Looks like Jungshik Shin was not in the CC list.  (CC'ing him and watanabe-san)

Jungshik Shin: can you review the patch and tell us what you think? Thanks

Comment 12

[Jungshik Shin]

Thank you, Roy. Indeed I didn't get an email when Frank
added his comment although I had added a comment to this bug before that.

There are two problems with Watanabe san's patch:

  1. Space(0x20) and tabs (0x09) embedded in KS X 1001 part
(KS C 5601 part) don't get a special treatment. They have to
be left alone even though they're between SO and SI.

  2. Whenever a new line character appear in the stream,
     US-ASCII mode has to kick in automatically, but it's not the
     case in the current patch.

 Another issue is that Mozilla doesn't need ISO 2022 KR encoder
because Mozilla need/should NOT produce ISO-2022-KR stream. Mozilla only
needs to convert ISO-2022-KR produced by some old email programs
to Unicode. Therefore, ISO-2022-KR should NOT be added to 
intl/uconv/src/charsetTitles.properties


  I've made my own nsISO2022KRToUnicode.cpp (similar to ISO2022JP2ToUnicodeV2
in intl/uconv/ucvja/nsJapaneseToUnicode.cpp). It got compiled
fine, but I couldn't test it with 'nsconv' for some reason (nsconv complained
that it couldn't get Unicode decoder for ISO-2022-KR). I'll update my
copy of the whole source tree and compile the whole thing to see if things
are different.

Comment 13

[Jungshik Shin]

Attachment: ISO-2022-KR decoder only patch

Comment 14

[Jungshik Shin]

Attachment: ISO-2022-KR decoder only patch (new/fixed)

Comment 15

[Jungshik Shin]

Attachment: a message sample with ISO-2022-KR body (EUC-KR in header)

Comment 16

[Jungshik Shin]

I put up a sample page in ISO-2022-KR at the URL above.
This is solely for the sake of testing ISO-2022-KR decoder.
ISO-2022-KR is NOT used for web publishing and only legacy
mail programs generate ISO-2022-KR encoded email messages.

I also tested various 'malformed' ISO-2022-KR with 'nsconv'
and it works fine given those malformed ISO-2022-KR input, let alone
well-formed ones.

Comment 17

[Roy Yokoyama]

jshin: the new patch looks good. 
watanabe-san: can you review the patch (09/20/01) and tell us what you think?
=== attempt to CC [e-mail] again

Comment 18

[Roy Yokoyama]

Comment on attachment 50194 [details] [diff] [review]
ISO-2022-KR decoder only patch (new/fixed)

I can't contact watanabe-san; but patch looks good.
/r=yokoyama

Comment 19

[watanabe]

Sorry, I was too busy to reply.  Now, I have reviewed the patch.
It looks good.

Comment: spaces and tabs embedded in KSC5601 part may be invalid for RFC-1557. 
However, some softwares generate such a code.  Is that right?  If so, I can
understand this patch.

Comment 20

[Jungshik Shin]

Watanabe-san,
Thank you for reviewing my patch.
Yes, you're right. Some programs generate space and tab embedded
in KSC 5601 part so that Mozilla has to be generous. Moreover,
some programs don't end each line with Shift-In so that 
whenever EOL is encountered, Mozilla has to come back to US-ASCII mode.

BTW, I took back what I wrote about your patch to charsetTitles.properties
(as shown below). The patch still has to be applied (although
Mozilla would not support *outgoing* ISO-2022-KR messages).  

Index: uconv/src/charsetTitles.properties
===================================================================
RCS file: /cvsroot/mozilla/intl/uconv/src/charsetTitles.properties,v
retrieving revision 1.20
diff -u -r1.20 charsetTitles.properties
--- uconv/src/charsetTitles.properties	2001/05/11 13:41:20	1.20
+++ uconv/src/charsetTitles.properties	2001/05/21 13:51:40
@@ -57,6 +57,7 @@
 x-gbk.title = Chinese Simplified (GBK)
 iso-2022-cn.title = Chinese Simplified (ISO-2022-CN)
 euc-kr.title = Korean (EUC-KR)
+iso-2022-kr.title = Korean (ISO-2022-KR)
 utf-7.title = Unicode (UTF-7)
 utf-8.title = Unicode (UTF-8)
 iso-8859-5.title = Cyrillic (ISO-8859-5)


 

Comment 21

[Roy Yokoyama]

Jungshik Shin: can you attach a complete patch for decoder and 
properties file so that it's easier for super reviewers to review?

Comment 22

[Jungshik Shin]

Attachment: a new patch including prop. file

Comment 23

[Roy Yokoyama]

Jungshik: Thanks.  I'll try the patch and test. Once it's good, i'll give /r 
and ask for super review.

Comment 24

[watanabe]

> BTW, I took back what I wrote about your patch to
> charsetTitles.properties

I think it's good.

Comment 25

[Roy Yokoyama]

Attachment: Need to update navigator.properties to show the KO converter under East Asian

Comment 26

[Roy Yokoyama]

Attachment: Same patch; just to fix the indentations

Comment 27

[Roy Yokoyama]

Comment on attachment 51747 [details] [diff] [review]
a new patch including prop. file

I posted a new patch

Comment 28

[Roy Yokoyama]

Comment on attachment 52174 [details] [diff] [review]
Same patch; just to fix the indentations

/r=yokoyama.  Verified that patch works.

Comment 29

[Roy Yokoyama]

Comment on attachment 52148 [details] [diff] [review]
Need to update navigator.properties to show the KO converter under East Asian

/r=yokoyama.  Verified that patch works.

Comment 30

[Roy Yokoyama]

Moving the target milestone to 0.9.6.

Comment 31

[Jungshik Shin]

Roy, thanks for catching what I missed (navigator.properties file)
and confirming that it works.
Shall I ask for sr on reviewers@ list? 

Comment 32

[Roy Yokoyama]

>Shall I ask for sr on reviewers@ list? 
Sure.  Thanks. 
If you don't hear from /sr, then I can go upstairs and knock some doors. :)

Comment 33

[Brendan Eich [:brendan]]

Comment on attachment 52174 [details] [diff] [review]
Same patch; just to fix the indentations

rs=[e-mail]

Comment 34

[Jungshik Shin]

Roy, now that I have r and sr, should I check in? 
My commit-accnt has been activated.

Comment 35

[Roy Yokoyama]

Jungshik Shin: Yes, please go ahead.  (... your first check-in! :) .....)
Let me know if you need my help.  

Comment 36

[Frank Tang]

reassign to [e-mail] to check in .

Comment 37

[Jungshik Shin]

Roy and ftang,
checked in this morning. Thanks for your help.

Comment 38

[Frank Tang]

please mark it "FIXED" once you check in. That trigger QA process.