33498 - add transliterator to Windows version of font engine

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[Erik van der Poel]

The Unix version of the font engine already calls the transliteration API that
maps e.g. the euro to "EUR". We need to call this API from the Windows version
of the font engine too, since even Windows doesn't always have glyphs for all
Unicodes. This bug is a spin-off from bug 17962.

Comment 1

[Frank Tang]

Mark it as M25

Comment 2

[Frank Tang]

mark this as future for now.

Comment 3

[Frank Tang]

erik resign. reassign all his bug to ftang for now.

Comment 4

[Frank Tang]

mark all future new as assigned after move from erik to ftang

Comment 5

[Frank Tang]

shanjian- low priority bug but I think you are the right person to look at it if
we really need it. 

Comment 6

[rbs]

I have been "manually" doing some transliteration for bug 79928, It would be
much cleaner if SubstituteChars() could do the transliteration in the usual way 
(i.e., cache the properties on first use, an re-use later).

Comment 7

[rbs]

Attachment: patch in intl

Comment 8

[rbs]

Attachment: patch in gfx

Comment 9

[rbs]

Grabbing the bug as I have a fix.

Tiny little patch like people like :-) Could I get some hot r/sr, thanks. 
This way I can land this little gem presto and keep my other changes in sync...

I was having some misgivings about manually transliterating the ZWSP (bug 79928) 
because that hardcoded approach doesn't scale. So I thought I will just bite the 
bullet and do it the proper way. The fix uses the transliterator and this brings 
GfxWin in parity with what is happening on other platforms.

In the process, I noted that other platforms are using 
nsISaveAsCharset::attr_EntityBeforeCharsetConv. I think what people want is
nsISaveAsCharset::attr_EntityAfterCharsetConv. 

The former will convert to entities _before_ doing the transliteration, thereby 
possibly converting some glyphs that could be transliterated. The latter will 
transliterate, and then the glyphs that couldn't be transliterated will be 
converted to their entity representation if possible or to question marks if not 
possible. Finally, just in case, I also made sure that the Win32 routines that 
expect the converted result are 0-length safe. Not sure about other platforms 
here.

Comment 10

[Shanjian Li]

Thanks roger for taking this. I read through your code and have one suggestion. 
You probably want to use "NS_ConvertASCIItoUCS2" to do conversion from latin1 to 
unicode. 

Comment 11

[rbs]

Wouldn't that allocate another area? Or is there another flavor that does the 
conversion in a caller provided buffer?

Comment 12

[Shanjian Li]

I don't realize there is another flavor. You are right, in this case 
we don't want to bother.  r=shanjian

Comment 13

[rbs]

Attachment: updated patch in gfxwin

Comment 14

[rbs]

Update at the wake of the font changes.
Ready for r/sr.

Comment 15

[rbs]

*** Bug 79928 has been marked as a duplicate of this bug. ***

Comment 16

[rbs]

Guys, could you clear me on this one. I have well tested by simulating different 
error conditions to exercise the code-paths, and repeated the process with the 
'A' functions.

Comment 17

[rbs]

... and stepped in the debugger to see that things were going on as expected.

Comment 18

[Chris Waterson]

Why ISO-8859-1? Will this work properly on non-US systems?

+      res = gFontSubstituteConverter->Init("ISO-8859-1",

Comment 19

[rbs]

Yes. This causes the result of the substitution to be in ASCII. Then the 
resulting ASCII string is copied to Unicode (since ASCII code points are 
invariant in Unicode). And from there on, the result is fed to other functions
which expects Unicode.

Comment 20

[Chris Waterson]

Comment on attachment 51176 [details] [diff] [review]
updated patch in gfxwin

r/sr=waterson, whichever helps.

Comment 21

[rbs]

Will checkin shortly with the assumption that the earlier r=shanjian still 
applies after transposing the patch at the wake of font changes. Also noted a 
couple of possible leaks on the error code-paths that shanjian pointed in 
bug 99010 and somehow I missed them in the heat of that bug.

Comment 22

[rbs]

Fix checked in.