Closed Bug 100324 Opened 23 years ago Closed 23 years ago

(patch) hack to detemine width of cyrillic characters

Categories

(Core :: Printing: Output, defect)

x86
Linux
defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla0.9.9

People

(Reporter: ggromov, Assigned: dcone)

References

Details

Attachments

(2 files)

Here is a patch (hack) to define a width of cyrillic characters. It's much better than nothing. --- The patch is created by ALTlinux team (www.altlinux.ru) and is used in their package of mozilla. Their SRPMs with this patch included are publically distributed since spring 2001, from ftp://ftp.altlinux.ru/pub/distributions/ALTLinux/Sisyphus/SRPMS/ (NOTE: their ftp supports passive mode only!) Please give credit to them for their wonderfull patches somewhere in release notes and/or documentation, if possible.
Attached patch patch (deleted) — Splinter Review
Severity: critical → major
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: patch, review
taking
Assignee: dcone → rods
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.8
HELP! Somebody is going to have to get me a new patch, thanks.
Hi! What do you mean? I just checked the latest version of their patch, it seems to be the same: diff -ur mozilla.orig/gfx/src/ps/nsAFMObject.cpp mozilla/gfx/src/ps/nsAFMObject.cpp --- mozilla.orig/gfx/src/ps/nsAFMObject.cpp Sat Dec 16 17:59:47 2000 +++ mozilla/gfx/src/ps/nsAFMObject.cpp Sat Dec 16 18:08:36 2000 @@ -776,8 +776,10 @@ fwidth = (PRInt32)(mPSFontInfo->mAFMCharMetrics[idx].mW0x); // if ( (*cptr == 0x0020) || (*cptr == 0x002c) ) // printf("fwidth = %d\n", fwidth); - if (*cptr & 0xff00) - fwidth = 1056; + if (*cptr & 0xff00) + fwidth = 1056; + if (*cptr & 0x0400) //cyrillic + fwidth = 600; // hack!!! if ( (*cptr == 0x0020) || (*cptr == 0x002c) ) fwidth = 1056; // space and comma are half size of a CJK width totallen += fwidth; Or did you mean anything else? BTW, why target you set is 0.9.8, why not 0.9.7? :)
I don't like that exact patch. The patch will not only impact Cyrillic. I think we can work out a better patch here. I think the whole block is wrong. We should not use if (*cptr & 0x0400) //cyrillic for cyrillic it should be if ((*cptr & 0xff00) == 0x0400) //cyrillic if (*cptr & 0x0400) will match more than 0x0400 - 0x04ff it will ALSO match any other unicode that bit is on. if ((*cptr & 0xff00) == 0x0400) will only match cyrillic. Also the origional code may cause crash ( idx may < 0) I will propose a better patch, But I need someone to help me to test the patch is coming.
the hack could be better if we can take http://www.unicode.org/Public/3.1-Update1/EastAsianWidth-5.txt and cacular width depend on the unicode property ( half width for non CJK, full width for CJK , zero width for combining mark. read http://www.unicode.org/unicode/reports/tr11/ for details. But I think we can do that improvement later. file bug 119265 for that. Let's not blocking this patch untill that got fix. (that will take longer time)
rods, jshin, katakai and Gena, can you test my patch ? I think jshin and katakai cover the testing of Japaense, Chinese and Korean and Gena can cover Cyrillic and western. Rods can cover western. Thanks
I think my patch should also STOP a crashing when (*cptr) is euqal to 0x0100 - 0x011f, 0x0200 - 0x021f,0x0300 - 0x031f.. 0xff00 - 0xff1f where idx is 0xFFFFFFE0-0xFFFFFFF
I've tried the patch and I could not see the crash issue. However, Japanese EUC 0xa7a0-0xa7f2 are converted to \u04?? so the behavior will be changed by this patch. Not sure which is correct.
Target Milestone: mozilla0.9.8 → mozilla0.9.9
Frank, your version of the patch works fine with Cyrillic and western. Thank you very much for working on it!
Don, since this is PS I'll let you handle it.
Assignee: rods → dcone
Status: ASSIGNED → NEW
Blocks: 123569
> However, Japanese EUC 0xa7a0-0xa7f2 are converted to \u04?? > so the behavior will be changed by this patch. Not sure which > is correct. The same is true of any encodings based on KS X 1001 (KS C 5601). I also think that's the case of SC and TC encodings that include Cyrillic letters. However, I tend to believe that this is an acceptable fallback until EastAsianWidth is handled by Mozilla as specified in UTR #11. BTW, Greek alphabets in 0x370-0x37F should be treated the same way as Cyrillic alphabets, shouldn't they?
Marking nsbeta1+
Keywords: nsbeta1+
dcone: will you land this in m0.9.9? if you want me to land it, then please r= it and reassign it to me.
Comment on attachment 64326 [details] [diff] [review] better patch also may stop a crash r=dcone
Attachment #64326 - Flags: review+
Comment on attachment 64326 [details] [diff] [review] better patch also may stop a crash sr=attinasi
Attachment #64326 - Flags: superreview+
if (0x0400 == (*cptr & 0xff00)) { // Cyrillic Although this patch is for Cyrillic, wouldn't it better to take care of Greek as well by replacing the above line with the following? if (0x370 <= *cptr && *cptr <= 0x04ff) { // Greek and Cyrillic Or, am I missing something here (i.e. Greek alphabets are taken care of somewhere else)? In that case, here goes my apology.
Fixed
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Gena or someone on the cc: list, can you please verify this bug and mark it VERIFIED-FIXED please? thanks!
Just for the log: I've cleaned-up the postscript module prefs which involves an name change of the prefs. All postscript-module specific prefs start now with "print.postscript.". Example: "print.psnativefont.ja" --> "print.postscript.nativefont.ja" "print.psnativecode.ja" --> "print.postscript.nativecode.ja" Some (the others will follow, but I did not have enougth time to do more cleanup&&testing) pref items like "print_command" can now be set in a per printer-specific manner, e.g. |pref("print.postscript.printer_foobar.print_command", "psview /dev/stdin");| sets the print command for the printer "foobar" only to "psview /dev/stdin", overriding the default in unix.js ...
marking verified-fixed. Thanks for fixing this issue!
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: