125132 - foreign characters not displayed properly in align=justify

Status: VERIFIED FIXED

(Core :: Internationalization: Localization, defect)

Description

[Paul]

Foreign characters are not being displayed properly when used within a tag
with the align=justify parameter under BeOS.  The current code, iterates 
through the string, one character at a time, drawing it, and putting the needed
space in between the characters.  The code does not, however, take into account
utf8 characters, and therefor displays the wrong text.

Comment 1

[Paul]

Attachment: patch to nsRenderingContextBeOS to iterate through utf8 chars

Patch originally created by Sergei Dolgov.  I brought it into the latest
version in the tree, and cleaned it up a bit.  Seems to work.

Comment 2

[Daniel Switkin]

A few obvious problems:

- utf8substr is leaked on every character
- new'ing utf8substr on every character is incredibly wasteful
- using memcpy() for utf8substr is overkill

What is the maximum size that utf8_char_len() can return? I thought UTF-8 could only be two 
or three, but I don't remember? If that's the case, you should just have:

uint8 utf8substr[max_size + 1];

on the stack, then a for loop to copy ch_len characters into it by hand.

Really we need to get spacing working correctly with only one call to DrawString(). There 
is definitely overhead to calling it once per character/glyph, but cleaning up this patch 
should hold us for the mean time.

Comment 3

[Paul]

Attachment: Updated patch

Updated patch.	Much better.  Thanks go to Sergei for both patches.

Comment 4

[Paul]

Attachment: Test html page, if you need it (justified russion text)

Test page, containing some russian text in a justified area, as well as 
non-justified

Comment 5

[Daniel Switkin]

The new patch is much better. Doing it in place is even better than how I suggested on the 
stack, I forgot you didn't need to pass a null-terminated string if you had the length in 
bytes.

Out of curiosity, can someone who has built with the patch tell me if this fixes the 
strange garbled text here:

http://www.dpreview.com/news/0202/02021101foveonx3.asp

Comment 6

[Paul]

Yes.  That page you mentioned is almost entirely justified.  Looks "ok" now.

Comment 7

[Paul]

I have checked in the updated patch.  Marking resolved/fixed.

Comment 8

[Yuying Long]

I can not reproduce it here, mark it as verified.

Please reopen if still existing on BeOS.

Comment 9

[Sergei Dolgov]

Hi people. Because this case (justified drawing) heavily affects also text input (e.g. in web forms) i decided to revise code and made another version of patch. It may be still not perfect - but number when it calls BeOS DrawString is in TIMES decreased! 
It does it now on word-by-word  basis - counts number of words, calcuclates each word length, pointer and nscoordinate - and puts those whole words in DrawString.
What should i do? - Create new attachment here or send you code individually to preview?
Regards, SD

Comment 10

[Paul]

Sergei, do a "cvs diff -u file [file ...] > patchfile", and then do a "Create a New
Attachment" on this bug, and upload the patch file.

Comment 11

[Sergei Dolgov]

Attachment: justified string drawing: word-by-word

I decided for shorter code (code i've send to Paul and Daniel private was more
*clean*). Though, it seems working. Also two implicit casts made explicit -
because my (and not only mine) compiler refused to compile those places
permanently.

Comment 12

[Paul]

Comment on attachment 69375 [details] [diff] [review]
Updated patch

a new patch that works on a word by word basis was attached.  Marking this one
obsolete

Comment 13

[Sergei Dolgov]

Attachment: fix for little bug in previous patch

In previous patch version '\0' characters sometimes passed into BeOS
DrawString.
It resulted in boxes around some word first letter.
Fixed

Comment 14

[Paul]

Something is VERY WRONG with the last patch posted.  Text is drawn all over the screen, 
overlapping, with differnt text overlapping each other.  I have not looked into the reasons 
yet, but if I get a chance next week, I will.

Comment 15

[Paul]

Sergei, I am marking this bug assigned to you.

Comment 16

[Sergei Dolgov]

Attachment: Restored Bold Emulation

Restored bold emulation.
Btw, it seems that another DrwString maybe be reworked too - to process string
directly instead loading stacj with this DrawString call. As it is done for
some other OS-es.
At all, if we can do something for faster text processing, we should do it,
because e.g. tect input in forms in BeOS build is still irritating slow.

I also made other changes in nsRenderingContextBeOS to improve speed a
responisbility a bit, but it is subject of other future "bugs".

Comment 17

[Paul]

Comment on attachment 84489 [details] [diff] [review]
Restored Bold Emulation

Everything seems ok with this patch, Sergei.
r=arougthopher

before I check it in, did you have any other changes you want to make?

Comment 18

[Sergei Dolgov]

let's checkin it as is, Paul.
It helps me to do following changes.
I will send you new diffs privately for testing after you update tree

Comment 19

[Paul]

this has been checked in.

Comment 20

[Yuying Long]

I don't see it on 06-12 branch build on WinXP, verified as fixed.  Please
re-open if still see the problem on BeOS.

Comment 21

[Paul]

Question:  How does one verify a bug for one platform, but running a build on another?  
This seems VERY wrong to me!  Even though I agree it's fixed, but, I'm the one that checked 
the thing in.

Comment 22

[Yuying Long]

You are right.  Please verify it on BeOS.

Comment 23

[Paul]

timeless, could you verify this?