Closed Bug 100324 Opened 23 years ago Closed 22 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 patchSplinter 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: 22 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: