Closed Bug 7964 Opened 25 years ago Closed 23 years ago

[converter]Need ISO-2022-KR converters

Categories

(Core :: Internationalization, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla0.9.6

People

(Reporter: ftang, Assigned: jshin)

References

()

Details

(Keywords: helpwanted)

Attachments

(3 files, 4 obsolete files)

Status: NEW → ASSIGNED
Target Milestone: M10
Summary: Need ISO-2022-KR converters → [converter]Need ISO-2022-KR converters
Target Milestone: M10 → M14
change to M14 since it is post beta.
Whiteboard: Help Wanted
Target Milestone: M14 → M20
change to M20 since no one want to help yet. We probably only need To unicode conversion only.
Keywords: helpwanted
Whiteboard: Help Wanted
QA Contact: teruko → ftang
reassign to cata and add jbetak/nhotta/erik to the cc.
Assignee: ftang → cata
Status: ASSIGNED → NEW
Status: NEW → ASSIGNED
Target Milestone: M20 → Future
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.
move all cata's bug to ftang
Assignee: cata → ftang
Status: ASSIGNED → NEW
Status: NEW → ASSIGNED
*** Bug 80350 has been marked as a duplicate of this bug. ***
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?
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.
Jungshik Shin : can you review watanabe@komadori.planet.sci.kobe-u.ac.jp 's patch since you are korean guru? jshin- once you r= this one, please ask reviewers@mozilla.org to sr it and ask yokoyama to check in for you. thanks.
Assignee: ftang → yokoyama
Status: ASSIGNED → NEW
Target Milestone: Future → mozilla0.9.5
accepting for helping to checkin.
Status: NEW → ASSIGNED
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
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.
Attached patch ISO-2022-KR decoder only patch (obsolete) (deleted) — Splinter Review
Attachment #50052 - Attachment is obsolete: true
Attached patch ISO-2022-KR decoder only patch (new/fixed) (obsolete) (deleted) — Splinter Review
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.
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 watanabe@komadori.planet.sci.kobe-u.ac.jp again
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
Attachment #50194 - Flags: review+
Attachment #35400 - Attachment is obsolete: true
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.
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)
Jungshik Shin: can you attach a complete patch for decoder and properties file so that it's easier for super reviewers to review?
Attached patch a new patch including prop. file (obsolete) (deleted) — Splinter Review
Jungshik: Thanks. I'll try the patch and test. Once it's good, i'll give /r and ask for super review.
Attachment #50194 - Attachment is obsolete: true
> BTW, I took back what I wrote about your patch to > charsetTitles.properties I think it's good.
Comment on attachment 51747 [details] [diff] [review] a new patch including prop. file I posted a new patch
Attachment #51747 - Attachment is obsolete: true
Comment on attachment 52174 [details] [diff] [review] Same patch; just to fix the indentations /r=yokoyama. Verified that patch works.
Attachment #52174 - Flags: review+
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.
Attachment #52148 - Flags: review+
Target Milestone: mozilla0.9.5 → mozilla0.9.6
Moving the target milestone to 0.9.6.
Roy, thanks for catching what I missed (navigator.properties file) and confirming that it works. Shall I ask for sr on reviewers@ list?
>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 on attachment 52174 [details] [diff] [review] Same patch; just to fix the indentations rs=brendan@mozilla.org
Roy, now that I have r and sr, should I check in? My commit-accnt has been activated.
Jungshik Shin: Yes, please go ahead. (... your first check-in! :) .....) Let me know if you need my help.
reassign to jshin@pantheon.yale.edu to check in .
Assignee: yokoyama → jshin
Status: ASSIGNED → NEW
Roy and ftang, checked in this morning. Thanks for your help.
please mark it "FIXED" once you check in. That trigger QA process.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: