Closed
Bug 100324
Opened 23 years ago
Closed 22 years ago
(patch) hack to detemine width of cyrillic characters
Categories
(Core :: Printing: Output, defect)
Tracking
()
VERIFIED
FIXED
mozilla0.9.9
People
(Reporter: ggromov, Assigned: dcone)
References
Details
Attachments
(2 files)
713 bytes,
patch
|
Details | Diff | Splinter Review | |
1.19 KB,
patch
|
dcone
:
review+
attinasi
:
superreview+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•23 years ago
|
||
Updated•23 years ago
|
Updated•23 years ago
|
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.8
Comment 3•23 years ago
|
||
HELP! Somebody is going to have to get me a new patch, thanks.
Reporter | ||
Comment 4•23 years ago
|
||
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? :)
Comment 5•23 years ago
|
||
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.
Comment 6•23 years ago
|
||
Comment 7•23 years ago
|
||
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)
Comment 8•23 years ago
|
||
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
Comment 9•23 years ago
|
||
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
Comment 10•23 years ago
|
||
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.
Updated•23 years ago
|
Target Milestone: mozilla0.9.8 → mozilla0.9.9
Reporter | ||
Comment 11•23 years ago
|
||
Frank, your version of the patch works fine with Cyrillic and western. Thank you very much for working on it!
Comment 12•23 years ago
|
||
Don, since this is PS I'll let you handle it.
Assignee: rods → dcone
Status: ASSIGNED → NEW
Comment 13•23 years ago
|
||
> 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?
Comment 15•22 years ago
|
||
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.
Assignee | ||
Comment 16•22 years ago
|
||
Comment on attachment 64326 [details] [diff] [review] better patch also may stop a crash r=dcone
Attachment #64326 -
Flags: review+
Comment 17•22 years ago
|
||
Comment on attachment 64326 [details] [diff] [review] better patch also may stop a crash sr=attinasi
Attachment #64326 -
Flags: superreview+
Comment 18•22 years ago
|
||
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.
Assignee | ||
Comment 19•22 years ago
|
||
Fixed
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Comment 20•22 years ago
|
||
Gena or someone on the cc: list, can you please verify this bug and mark it VERIFIED-FIXED please? thanks!
Comment 21•22 years ago
|
||
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 ...
Reporter | ||
Comment 22•22 years ago
|
||
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.
Description
•